illumos / cpp Goto Github PK

Joyent never quite got around to completing review and testing on TritonDataCenter/illumos-extra#12 (submitted by @richlowe), which forms the basis of this repository. We should do that. Rich's comments reproduced below:

richlowe commented on Sep 21, 2013:

This contains a chunk of fixes to improve compatible with Sun cpp
(or correctness in general). It started out as a fix for
TritonDataCenter/smartos-live#171 but grew as more problems became apparent.

There are still at least 2 issues outstanding:

1. As documented in the first non-HACK commit, our treatment of
   spaces in macros is, while considerably better, still not
   strictly compatible in a couple of ways that might be
   important

2. #if conditions are not macro-expanded prior to being
   processed, such that #if SOME_MACRO(5) is a syntax error.
   This is proving particularly frustrating to fix.

This will definitely require more testing on the SmartOS side to
be really sure it's safe, I'd recommend a complete platform build,
testing of any SDC bits that use dtrace -C, at least. I'd also
probably suggest running test.sh | squash.py over a SmartOS
/usr/include and checking for anything that would affect the
parseability of the result. (this is why I left them in the
source).

richlowe commented on Oct 7, 2013:

I'm more concerned with review than testing. Making sure SDC isn't
broken, etc, is important, but the scripts I attached and their
results (and just how plainly wrong some of the prior behaviour
was) suggest it's pretty unlikely. The major concern I have is
that the code is so interestingly structured (because here on the
11/34, we don't have much memory to play with!), that I could have
screwed it entirely and have it still just about work right :\

I think that fixing cpp like this is probably the right thing to
do short-term, but I pretty firmly believe that -- given just how
broken this cpp is and was -- that switching to an ANSI cpp while
perhaps wrong is actually not any worse. I think the damage to
dtrace -C would be real, but minimal (and probably defensible).
The real problem would be how much pain it caused as.

Take the following:

$ cat wtf.cpp
#define WTF     "    this    is     a    string     with      spaces"

"    this    is     a    string     with      spaces"
WTF

This is the kind of output we would expect:

$ cat wtf.cpp | /opt/gcc-12/bin/cpp
# 0 "<stdin>"
# 0 "<built-in>"
# 0 "<command-line>"
# 1 "<stdin>"


"    this    is     a    string     with      spaces"
"    this    is     a    string     with      spaces"

In antiquarian CPP, however:

$ cat wtf.cpp | ./cpp/src/cpp
# 1 "" 


"    this    is     a    string     with      spaces"
" this is a string with spaces"

This is due to fixing TritonDataCenter/smartos-live#171, and in particular, this code:

Unforunately, this behavior is causing a DTrace test suite failure, as macros in https://github.com/illumos/illumos-gate/blob/master/usr/src/cmd/dtrace/test/tst/common/json/tst.general.d contain spaces that are being incorrectly elided.

It's unclear how hard this would be to fix or if the cure is worse than the disease on this one. If DTrace is the only consumer (but it sounds like as is also still a consumer?), it would probabably be be worth moving to an ANSI-compatible CPP in /usr/lib.