<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Sun, Feb 9, 2014 at 1:01 AM, Harald van Dijk <span dir="ltr"><<a href="mailto:harald@gigawatt.nl" target="_blank">harald@gigawatt.nl</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div class="">On 05/02/14 23:16, Richard Smith wrote:<br>
> On Wed, Feb 5, 2014 at 1:08 PM, Harald van Dijk <<a href="mailto:harald@gigawatt.nl">harald@gigawatt.nl</a><br>
</div><div><div class="h5">> <mailto:<a href="mailto:harald@gigawatt.nl">harald@gigawatt.nl</a>>> wrote:<br>
><br>
>     On 05/02/14 00:42, Richard Smith wrote:<br>
>     > Here's an easy test:<br>
>     ><br>
>     > #define foo() bar() b<br>
>     > #define bar()<br>
>     > a<br>
>     > foo()<br>
>     > c<br>
>     ><br>
>     > This should produce:<br>
>     ><br>
>     > # 1 "..."<br>
>     ><br>
>     ><br>
>     > a<br>
>     >  b<br>
>     > c<br>
>     ><br>
>     > ... but currently produces:<br>
>     ><br>
>     > #1 "..."<br>
>     ><br>
>     ><br>
>     > a b<br>
>     ><br>
>     > c<br>
><br>
>     Thanks Richard, I don't know how I managed to miss that.<br>
><br>
>     Setting Token::StartOfLine as mentioned gets this right in the lexer.<br>
>     However, even though the b token in the output has both<br>
>     Token::StartOfLine and Token::LeadingSpace set,<br>
>     PrintPPOutputPPCallbacks::HandleFirstTokOnLine uses the column number to<br>
>     determine whether to print spaces, not the flag. As a result, the output<br>
>     becomes not<br>
><br>
>     a<br>
>      b<br>
>     c<br>
><br>
>     but<br>
><br>
>     a<br>
>     b<br>
>     c<br>
><br>
>     Given that the lexer behaviour is now right, would it be okay to leave<br>
>     this as it is, and only add a test to check that b is on a separate<br>
>     line, like attached? (I used first/second/third instead of a/b/c in an<br>
>     attempt to get FileCheck to print a better suggestion when the test<br>
>     fails, but it neither helps nor hurts.)<br>
><br>
><br>
> I'm not sure whether we have consumers of -E that care about this<br>
> detail, but I think this is an unrelated bug. Consider:<br>
><br>
> #define foo(x) x y<br>
> foo()<br>
><br>
> This also misses the leading space from the output. So:<br>
><br>
> 1) I think your patch is fine as-is, and<br>
> 2) I think we should fix PrintPreprocessedTokens to correctly handle the<br>
> first token on the line having leading space but being expanded from a<br>
> token in the first column.<br>
<br>
</div></div>All right. Assuming that leading white space in column 1 is the only<br>
case that gets mishandled, it seems like a special exception would<br>
suffice. The attached (on top of my previous patch) passes testing. But<br>
I am not all that familiar with this part of the code.<br></blockquote><div><br></div><div>Patch looks good, committed as r202070.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
Note that the code below checks for # as a special exception: if that<br>
appears at the start of a line, a space is output without changing<br>
ColNo. While I do like consistency, using that approach would cause two<br>
spaces to be printed instead of one if # should appear in column 1, with<br>
leading white space.</blockquote><div><br></div><div>It seems somewhat reasonable to insert the space and stay in column 1 in that case -- that way, downstream tools that support the "preprocessed source" input format can just strip the leading space in a line that starts with " #" and not worry about later column numbers getting messed up.</div>
</div></div></div>