r200787 - Fix whitespace handling in empty macro expansions

[Richard Smith]

On Tue Feb 04 2014 at 2:34:43 PM, Harald van Dijk <harald at gigawatt.nl>
wrote:

> On 04/02/14 22:48, Justin Bogner wrote:
> > Richard Smith <richard at metafoo.co.uk> writes:
> >> On Tue, Feb 4, 2014 at 11:18 AM, Justin Bogner <mail at justinbogner.com>
> wrote:
> >>     Fix whitespace handling in empty macro expansions
> >>
> >>     When a macro expansion does not result in any tokens, and the macro
> name
> >>     is preceded by whitespace, the whitespace should be passed to the
> first
> >>     token that follows the macro expansion. Similarly when a macro
> expansion
> >>     ends with a placemarker token, and that placemarker token is
> preceded by
> >>     whitespace. This worked already for top-level macro expansions, but
> is
> >>     now extended to also work for nested macro expansions.
> >>
> >>     Patch by Harald van Dijk!
> >>
> >>     Modified:
> >>         cfe/trunk/lib/Lex/TokenLexer.cpp
> >>         cfe/trunk/test/Preprocessor/macro_space.c
> >>
> >>     Modified: cfe/trunk/lib/Lex/TokenLexer.cpp
> >>     URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/
> TokenLexer.cpp?
> >>     rev=200787&r1=200786&r2=200787&view=diff
> >>     ============================================================
> ==============
> >>     ====
> >>     --- cfe/trunk/lib/Lex/TokenLexer.cpp (original)
> >>     +++ cfe/trunk/lib/Lex/TokenLexer.cpp Tue Feb  4 13:18:35 2014
> >>     @@ -477,9 +477,13 @@ bool TokenLexer::Lex(Token &Tok) {
> >>        if (isFirstToken) {
> >>          Tok.setFlagValue(Token::StartOfLine , AtStartOfLine);
> >>          Tok.setFlagValue(Token::LeadingSpace, HasLeadingSpace);
> >>     -    AtStartOfLine = false;
> >>     -    HasLeadingSpace = false;
> >>     +  } else {
> >>     +    // If this is not the first token, we may still need to pass
> through
> >>     +    // leading whitespace if we've expanded a macro.
> >>     +    if (HasLeadingSpace) Tok.setFlag(Token::LeadingSpace);
> >>
> >> Do we need to do this for AtStartOfLine too? What happens if we have a
> macro
> >> at the start of a line that expands to no tokens, within a TokenLexer?
> (I
> >> guess this can't happen within a macro expansion, since no tokens there
> are
> >> ever at the start of a line after line-splicing, but we use TokenLexers
> in
> >> other cases too).
> >
> > CCing Harald.
> >
> > Handling AtStartOfLine as well does sound pretty reasonable to me, but
> > it's difficult to come up with a case where this actually applies.
> > Without a concrete example, I don't have a good way to judge this.
>
> It can happen even in a macro expansion:
>
> #define foo() bar() .
> #define bar()
> foo()
>
> The question is whether the resulting . should be treated as at the
> start of the line. I was initially unable to come up with a test case
> where it mattered, but am able to come up with one now, because error
> handling is different depending on whether certain tokens start on the
> next line:
>
> int a, int b; // expected-error {{expected identifier or '('}}
> // expected-error at -1 {{expected ';' after top level declarator}}
>
> int c, // expected-error {{expected ';' at end of declaration}}
> int d;
>
> #define foo() bar() int f;
> #define bar()
> int e, // expected-error {{expected ';' at end of declaration}}
> foo()
>
> This fails, should pass, and setting Token::StartOfLine is indeed enough
> to make it pass, like so:
>
> diff --git a/lib/Lex/TokenLexer.cpp b/lib/Lex/TokenLexer.cpp
> index 9913f30..543ca92 100644
> --- a/lib/Lex/TokenLexer.cpp
> +++ b/lib/Lex/TokenLexer.cpp
> @@ -474,6 +474,7 @@ bool TokenLexer::Lex(Token &Tok) {
>    } else {
>      // If this is not the first token, we may still need to pass through
>      // leading whitespace if we've expanded a macro.
> +    if (AtStartOfLine) Tok.setFlag(Token::StartOfLine);
>      if (HasLeadingSpace) Tok.setFlag(Token::LeadingSpace);
>    }
>    AtStartOfLine = false;
>
>
> I am not happy with my test case, though. Is there an easier way to
> verify that this works as intended?
>

Here's an easy test:

#define foo() bar() b
#define bar()
a
foo()
c

This should produce:

# 1 "..."


a
 b
c

... but currently produces:

#1 "..."


a b

c
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140204/9840bdf0/attachment.html>