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