FW: [PATCH] Extend PPCallbacks #if and #elif callbacks

[Thompson, John]

(The return addresses got garbled, so I'm resending.)

From: Thompson, John
Sent: Tuesday, July 16, 2013 3:37 PM
To: 'Argyrios Kyrtzidis'
Cc: cfe-commits@; silvas@
Subject: RE: [PATCH] Extend PPCallbacks #if and #elif callbacks

Thanks for the feedback, Argyrios.

I guess you're right.  I was thinking that it would be nice to show an #if directive after expansion, but it does overlap what I'm doing with the macros already.  If no one else expresses an interest in having the expression tokens, I'll cut it back to just the flag.

-John

From: Argyrios Kyrtzidis [mailto:akyrtzi at gmail.com]
Sent: Tuesday, July 16, 2013 2:32 PM
To: Thompson, John
Cc: cfe-commits@; silvas@
Subject: Re: [PATCH] Extend PPCallbacks #if and #elif callbacks

Hi John,

I like the Boolean flag but I'm not sure about the token buffer.

My issue is that this increases somewhat the preprocessor complexity and maintenance (there is an extra buffer to worry about) and not sure if it is really needed anyway.
All PPCallbacks clients so far did not need it and even for the modularize tool, I have doubts about the user friendliness of just showing two strings that differ (the concatenation of the tokens).
IMHO it would be better if you could point at the macros that have different definitions and for that you only need the locations to point to the different definitions.

What do you think ?

On Jul 16, 2013, at 8:38 AM, Thompson, John <John_Thompson at playstation.sony.com<mailto:John_Thompson at playstation.sony.com>> wrote:

Hi,

To facilitate the modularize tool's tracking of #if and #elif preprocessor conditional directives, it would be useful for the PPCallbacks interface to include two new arguments in the callbacks for these directives, a vector of tokens for the conditional expression (after substitution) and a Boolean flag indicating the evaluated value for the condition expression.  There was a FIXME in the original header for the former, and the later was discussed on cfe-dev.

The enclosed patch represents my attempt to implement this.

Some design considerations were to minimize performance impact, and to avoid making the existing code more complicated than necessary.  Most of the logic for handling the conditional expression was in one or two files, so it seemed a reasonable approach was to handle the token collection in some new member functions in Preprocessor used for lexing the next tokens, such that only function call name changes were needed in the conditional expression code.

The vector for storing the collected tokens is lazily created or cleared in the common EvaluateDirectiveExpression function.  It's defined as an OwningPtr to have it released when the Preprocessor object is destructed.

One concern is there might be user code outside of the repository that would need to be changed to accommodate this API change.

I would appreciate your review feedback and/or permission to check in.

Thanks.

-John

<tokencollection1.patch>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130716/643d8dca/attachment.html>