[PATCH] Make Preprocessor::Lex non-recursive

[Richard Smith]

On Wed, Sep 18, 2013 at 3:44 PM, Richard Smith <richard at metafoo.co.uk>wrote:

> A modules-related change seems to have slipped into
> lib/Frontend/CompilerInstance.cpp
>
> The IsAtStartOfLine FIXME concerns me a little. Token.h says:
>
>     StartOfLine   = 0x01,  // At start of line or only after whitespace.
>
> ... 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.
>

Um, users of the flag.


> What happens if the Lex call in isNextPPTokenLParen returns false?
>
>
> On Mon, Sep 16, 2013 at 4:40 PM, Eli Friedman <eli.friedman at gmail.com>wrote:
>
>> On Mon, Sep 16, 2013 at 3:44 PM, Richard Smith <richard at metafoo.co.uk>wrote:
>>
>>> On Mon, Sep 2, 2013 at 5:26 PM, Eli Friedman <eli.friedman at gmail.com>wrote:
>>>
>>>> Per subject, patch attached which makes Preprocessor::Lex non-recursive.
>>>>
>>>> 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).
>>>>
>>>> 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.
>>>>
>>>> I still need to do performance measurements, but I'm not anticipating
>>>> any measurable changes.
>>>>
>>>
>>> This seems reasonable to me. Have you had a chance to do any performance
>>> measurements yet?
>>>
>>
>> I did some basic performance measurements; I can't find any performance
>> difference lexing Cocoa.h
>>
>>
>>> Preprocessor::Lex now looks extremely unlikely to be inlined; is there
>>> any benefit in keeping it in the header file?
>>>
>>>
>>> No; moved to Preprocessor.cpp.
>>
>> -Eli
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130918/8921d258/attachment.html>