Explicit error for comments, with README link

[cellularmitosis]

@eliben thanks so much for your blog and for pycparser!

This PR adds an explicit exception for comments, with a hint to the user that clang's cpp does not remove comments, and that simply using gcc's cpp instead will likely solve their problem.

$ cat foo.c
int foo() {
    // this is a comment.
    return 42;
}

$ python3 examples/c_json.py foo.c
Traceback (most recent call last):
pycparser.plyparser.ParseError: foo.c:2:5: Comments are not supported (note: GCC's cpp removes comments, Clang's cpp does not).

See also my comment on a related github issue.

Hopefully this will curb the endless flood of "comments don't work" github issues! 😭

[eliben]

Thanks. I'm still thinking about this, but here are some initial comments

Also: add tests

[eliben]

I don't understand the c89/c99 distinction here. Aren't // just C++ style comments and /* the original C style?

[cellularmitosis]

I thought it made more sense to describe the comments from a C perspective, rather than a C++ perspective. I'll update this.

Also, I accidentally swapped c89 / c99.

[eliben]

This error message is way too specific; you have a particular issue you're encountering now, but there are a million other reasons people could try to run pycparser on files with comments. It could point to the repository's README, for example (which has a section on the preprocessor)

[cellularmitosis]

Thanks for the context, I'll point to the README, but I'd also like to add more context to the README.

Is his diagram an accurate understanding of the situation?

[cellularmitosis]

A bit more context on my findings: invoking clang's preprocessor as cpp appears to have different behavior than invoking it as clang -E. In particular, macOS system headers appear to rely on clang extensions, but apparently cpp turns support for them off.

In other words, clang's/usr/bin/cpp chokes on any macOS system header, while clang -E works (but does not remove comments).

[cellularmitosis]

PR updated, tests are still TODO.

(Edit: added a second commit rather than amend / force-push, to preserve PR context)

[cellularmitosis]

Ok, tests added to PR.

This is ready for a re-review.

[eliben]

I'm not sure what Clang's cpp is... not clang -E, right? Regardless, such a comment will likely go out of date, and I prefer excluding it. The sentence above says enough about what cpp should do

[eliben]

A bit more context on my findings: invoking clang's preprocessor as cpp appears to have different behavior than invoking it as clang -E. In particular, macOS system headers appear to rely on clang extensions, but apparently cpp turns support for them off.

In other words, clang's/usr/bin/cpp chokes on any macOS system header, while clang -E works (but does not remove comments).

OK, thanks I understand the context now.

Still, I wouldn't say too much about this in the README. I understand you ran into a frustrating situation, but we shouldn't over-index on it. pycparser has been around for ages now, and this very rarely comes up. The errors you added should improve the situation significantly, but I don't want to maintain additional statements and flowcharts on the README that may go stale at any point

[cellularmitosis]

Sorry for the confusion: the flowchart was just for communication during this PR and will not be introduced into the repo.

I picked up on and am trying hard to satisfy the desire to avoid over-indexing on error specificity, but I'm going to be frank: I feel the current PR's user help on this topic is already too minimal.

The current PR:

• adds the phrase "A compatible"
• adds the sentence "Note that clang's cpp does not remove comments."

To communicate my thinking on this topic, here's a possible spectrum of context we could offer the user:

The minimum context the user needs in order to understand (but not fix) the problem is:

• parse_file() and preprocess_file() require a cpp which strips comments
• your cpp is not stripping comments

The minimum context to fix the problem would also include:

• gcc is an example of a cpp which strips comments

However, I also feel that the majority of developers are on Macs these days. A pleasant experience would also include direct, specific advice for that majority:

• brew install gcc
• add cpp_path='/opt/homebrew/bin/cpp-14' as a parameter to any parse_file() or preprocess_file() calls

Please understand that the user help in the current PR is already less that what I'd prefer.

[cellularmitosis]

Thinking about the staleness issue a bit more, I could change "Note that clang's cpp does not remove comments" to something like "Gcc includes a cpp which meets these requirements".

That would be less likely to go stale, as it is independent of clang's behavior, and I would guess it is very unlikely that gcc would ever change their implementation to stop stripping comments.

[eliben]

Thanks for sharing your thoughts. The PR's largest change is adding an explicit error "comments are not supported" with a README link. A user following this link will read about the requirements of cpp and will notice their cpp doesn't do the job. The text also says one may use clang -E instead of cpp, and even points to an example.

I really think this is sufficient for now.

By the way, it's entirely possible that Mac's cpp not removing comments is a new thing, because this issue would have come up more much frequently otherwise. These things change from time to time, and I want to keep our documentation as resilient as possible to these changes (it already talks quite a bit about the cpp issue!!)

[cellularmitosis]

Oh my gosh, I am so sorry, I somehow missed that while clang's cpp does not remove comments, clang -E does remove comments.

[cellularmitosis]

Ok, PR updated to remove "Note that clang's cpp does not remove comments".