<div dir="ltr">On Wed, Sep 18, 2013 at 3:44 PM, Richard Smith <span dir="ltr"><<a href="mailto:richard@metafoo.co.uk" target="_blank">richard@metafoo.co.uk</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote">
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div>A modules-related change seems to have slipped into lib/Frontend/CompilerInstance.cpp<br></div><div>
<br></div><div>The IsAtStartOfLine FIXME concerns me a little. Token.h says:</div><div><br></div><div>
<div> StartOfLine = 0x01, // At start of line or only after whitespace.</div></div><div><br></div><div>... which implies to me that the intent was that it be false if there's a preceding EMPTY macro. I'm not sure if that's what the existing users of the macro want, though.</div>
</div></blockquote><div><br></div><div>Um, users of the flag.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div>What happens if the Lex call in isNextPPTokenLParen returns false?</div>
</div><div class="HOEnZb"><div class="h5"><div class="gmail_extra"><br><br><div class="gmail_quote">On Mon, Sep 16, 2013 at 4:40 PM, Eli Friedman <span dir="ltr"><<a href="mailto:eli.friedman@gmail.com" target="_blank">eli.friedman@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div>On Mon, Sep 16, 2013 at 3:44 PM, Richard Smith <span dir="ltr"><<a href="mailto:richard@metafoo.co.uk" target="_blank">richard@metafoo.co.uk</a>></span> wrote:<br>
</div><div class="gmail_extra"><div class="gmail_quote"><div>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div>On Mon, Sep 2, 2013 at 5:26 PM, Eli Friedman <span dir="ltr"><<a href="mailto:eli.friedman@gmail.com" target="_blank">eli.friedman@gmail.com</a>></span> wrote:<br>
</div><div class="gmail_extra"><div class="gmail_quote"><div><div>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Per subject, patch attached which makes Preprocessor::Lex non-recursive.<div><br></div><div>Before this patch, Lex() would recurse whenever the current lexer changed (e.g. upon entry into a macro). This patch turns the recursion into a loop: the various lex routines now don't return a token when the current lexer changes, and at the top level Preprocessor::Lex() now loops until it finds a token. Normally, the recursion doesn't end up being very deep, but the recursion depth can explode in edge cases like a bunch of consecutive macros which expand to nothing (like in the testcase test/Preprocessor/macro_expand_empty.c in this patch).</div>
<div><br></div><div>To make this work, I made some substantial changes to the way the whitespace and empty macro flags are propagated. It had to be rewritten alongside this patch because the previous code to handle this wasn't tail-recursive. The code for this is pretty straightforward: it just adds a couple flags to the Lexer class, then updates them whenever we leave a macro expansion. I also fixed a minor bug while I was in the area: the previous code didn't work correctly for macros which had tokens in the definitions which expanded to nothing.</div>
<div><br></div><div>I still need to do performance measurements, but I'm not anticipating any measurable changes.</div></div></blockquote><div><br></div></div></div><div>This seems reasonable to me. Have you had a chance to do any performance measurements yet?</div>
</div></div></div></blockquote><div><br></div></div><div>I did some basic performance measurements; I can't find any performance difference lexing Cocoa.h</div><div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><br></div><div>Preprocessor::Lex now looks extremely unlikely to be inlined; is there any benefit in keeping it in the header file?</div><div> </div><div>
<br></div></div></div></div></blockquote></div><div>No; moved to Preprocessor.cpp.</div><span><font color="#888888"><div><br></div><div>-Eli </div></font></span></div><br></div></div>
</blockquote></div><br></div>
</div></div></blockquote></div><br></div></div>