<html><head><meta http-equiv="Content-Type" content="text/html charset=windows-1252"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><div>Hi John,</div><div><br></div><div>I like the Boolean flag but I'm not sure about the token buffer.</div><div><br></div><div>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.</div><div>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).</div><div>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.</div><div><br></div><div>What do you think ?</div><br><div><div>On Jul 16, 2013, at 8:38 AM, Thompson, John <<a href="mailto:John_Thompson@playstation.sony.com">John_Thompson@playstation.sony.com</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div lang="EN-US" link="blue" vlink="purple" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><div class="WordSection1" style="page: WordSection1;"><div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;">Hi,<o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;"><o:p> </o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;">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.<o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;"><o:p> </o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;">The enclosed patch represents my attempt to implement this.<o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;"><o:p> </o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;">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.<o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;"><o:p> </o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;">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.<o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;"><o:p> </o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;">One concern is there might be user code outside of the repository that would need to be changed to accommodate this API change.<o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;"><o:p> </o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;">I would appreciate your review feedback and/or permission to check in.<o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;"><o:p> </o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;">Thanks.<o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;"><o:p> </o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;">-John<o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;"><o:p> </o:p></div></div><span><tokencollection1.patch></span></div></blockquote></div><br></body></html>