<div dir="ltr"><br><div class="gmail_extra">I recommend looking at the code and trying to break it down into a bunch of small, "obvious", incremental changes to the existing code. It's really hard to review such a huge patch.<br>
<br><div class="gmail_quote">On Wed, Jun 12, 2013 at 1:41 PM, Thompson, John <span dir="ltr"><<a href="mailto:John_Thompson@playstation.sony.com" target="_blank">John_Thompson@playstation.sony.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">





<div lang="EN-US" link="blue" vlink="purple">
<div>
<p class=""><span style="font-size:11pt;font-family:Calibri,sans-serif;color:rgb(31,73,125)">Thanks a bunch, Sean.<u></u><u></u></span></p>
<p class=""><span style="font-size:11pt;font-family:Calibri,sans-serif;color:rgb(31,73,125)"><u></u> <u></u></span></p>
<p class=""><span style="font-size:11pt;font-family:Calibri,sans-serif;color:rgb(31,73,125)">I’ve cleaned up the sources a lot per your comments, but I still need to replace the macro substitution function, which will mean I can do away with the symbol
 storing too.  I couldn’t find a function such as I need, i.e. a function in Preprocessor that will either take a string or source range as input and produce either a string or token vector, with macro substitutions done using the current preprocessor state,
 all without adversely affecting the preprocessor state.  But it’s a huge class, so I might be missing it.  Do you or anyone know of such a function?  Otherwise, I’ll work on a separate patch to add one to Preprocessor.</span></p>
</div></div></blockquote><div style>Not off the top of my head. You may want to ask on cfe-dev for the best way to approach this.</div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div lang="EN-US" link="blue" vlink="purple">
<p class=""><span style="font-size:11pt;font-family:Calibri,sans-serif;color:rgb(31,73,125)">Also, in addition to the macro substitution, I think I can relatively easily add a similar message to the “header (file) has different contents depending on
 how it was included” error, relating the error to the differing preprocessor conditional.  Likewise, an option for showing the include hierarchy would help.<u></u><u></u></span></p>
<p class=""><span style="font-size:11pt;font-family:Calibri,sans-serif;color:rgb(31,73,125)"><u></u> <u></u></span></p>
<p class=""><span style="font-size:11pt;font-family:Calibri,sans-serif;color:rgb(31,73,125)">Anyway, may I check this in as a working intermediate step, in case folks find it useful?<u></u><u></u></span></p>
<p class=""><span style="font-size:11pt;font-family:Calibri,sans-serif;color:rgb(31,73,125)"><u></u> </span></p></div></blockquote><div style>Did you send the right patch? You don't seem to have addressed many of my review comments. Please revisit my last message and make sure that you have addressed those issues. There are numerous other changes I would like to suggest but the ones I pointed out earlier need to be fixed before the other changes can be meaningfully discussed.</div>
<div style><br></div><div style><br></div><div style>-- Sean Silva</div></div></div></div>