Argyrios,

Thanks for your continued help.

>The source range would be the range of the parens right ? Not sure why you'd 
>need it for modularize tool (I thought the location of the identifier is the 
>important one) but in general I'm buying the "consistency with other 
>callbacks" argument so LGTM.

It would be the whole "defined(SYMBOL)" range.  It's just a small tweak that 
lets me call out the whole "defined(SYMBOL)" expression in the error messages, 
i.e.:

InconsistentSubHeader.h:17:5:
#if defined(SYMBOL1)
    ^
error: Macro instance 'defined(SYMBOL1)' has different values in this header, 
depending on how it was included.
  'defined(SYMBOL1)' expanded to: 'true' with respect to these inclusion paths:
    InconsistentHeader1.h
      InconsistentSubHeader.h
InconsistentHeader1.h:3:9:
#define SYMBOL1 1
        ^
Macro defined here.
  'defined(SYMBOL1)' expanded to: 'false' with respect to these inclusion paths:
    InconsistentHeader2.h
      InconsistentSubHeader.h
(no macro definition)

>It would be great if you could also add some unit tests for the recent changes 
>in unittests/Lex/PPCallbacksTest.cpp but this can be done post-commit.

Oh, I didn't know about this test.  Thanks for pointing it out. Will do.

-John


From: Argyrios Kyrtzidis [mailto:[email protected]]
Sent: Friday, July 19, 2013 8:45 AM
To: Thompson, John
Cc: [email protected]; Sean Silva
Subject: Re: [PATCH] Enhance PPCallbacks::Defined callback to provide 
SourceRange


On Jul 18, 2013, at 3:55 PM, "Thompson, John" 
<[email protected]<mailto:[email protected]>> 
wrote:


Thanks for looking at it, Argyrios.

> Why do you need a SourceRange, can't you just use the location of the 
> identifier from the token parameter ?

I thought it would be cleaner to have the preprocessor pass it along, rather 
than crawling the source or approximating it.  But if avoiding changes to the 
preprocessor is preferred, I'll go that route, since I don't need the exact 
range, just something unique.

The source range would be the range of the parens right ? Not sure why you'd 
need it for modularize tool (I thought the location of the identifier is the 
important one) but in general I'm buying the "consistency with other callbacks" 
argument so LGTM.

It would be great if you could also add some unit tests for the recent changes 
in unittests/Lex/PPCallbacksTest.cpp but this can be done post-commit.



-John

From: Argyrios Kyrtzidis [mailto:[email protected]<http://gmail.com>]
Sent: Thursday, July 18, 2013 3:16 PM
To: Thompson, John
Cc: cfe-commits@; Sean Silva
Subject: Re: [PATCH] Enhance PPCallbacks::Defined callback to provide 
SourceRange

On Jul 18, 2013, at 1:09 PM, Thompson, John 
<[email protected]<mailto:[email protected]>> 
wrote:



In modularize, it turns out I need the SourceRange for the "defined()" 
expression.

Why do you need a SourceRange, can't you just use the location of the 
identifier from the token parameter ?




The enclosed patch addresses this, making it more like the MacroExpands 
callback.

MacroExpands needs a source range due to function macro expansions.


Thanks.

-John

<ppcallbacks2.patch>

_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to