[PATCH] Make Preprocessor::Lex non-recursive

[Richard Smith]

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.

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/ddbe9119/attachment.html>