[cfe-dev] Fix for preprocessor CommentHandler bug
andyg1001 at hotmail.co.uk
Mon Jun 18 09:50:01 PDT 2012
On Monday, June 18, 2012 2:20 AM, Richard Smith wrote:
> Apologies for the delay!
No problem! Its only because I'm hoping to get my other patch accepted ;o)
(By the way, I am also splitting this up, as requested...)
> There are two ways in which you might consider testing
> 1) Add a gunit test for it (you should be able to mirror
> the basic structure of unittests/Tooling/RecursiveASTVisitorTest.cpp).
> Your FrontendAction can add a CommentHandler in an
> override of BeginInvocation.
This is the route I've taken. I should have a new patch together tomorrow
(I've run out of time for today, unfortunately).
> For the patch itself, would it make sense to move the
> LexingRawMode alteration into CheckEndOfDirective?
Yes, this might make more sense. I'll have a look into it tomorrow.
> It also looks like you've changed the behavior of this
> #if 0
> #if 1
> #endif junk
Well caught! But it is also the case that other #directives inside an #if 0
will also not provide these warnings. Take the following snippet:
#if 0 junk
#if 1 junk
Even without my patch, only the first #if and both #endifs produce
diagnostics. The way the preprocessor seems to be designed, in most cases
it simply discards the whole line (comments included) following the
preprocessor directive when inside a skipped block. With the change I made,
one might argue, the diagnostics from the preprocessor are moving towards
more consistenty: i.e. inside a skipped #if block, there are (or should be)
no diagnostics generated. This matches nicely with the fact that we ignore
comment handling inside skipped #if blocks (thinking ahead to the -verify
But on the other hand, of course, I can easily revert the regression.
What do you think?
More information about the cfe-dev