On 05/02/14 23:16, Richard Smith wrote:
> On Wed, Feb 5, 2014 at 1:08 PM, Harald van Dijk <[email protected]
> <mailto:[email protected]>> wrote:
>
> On 05/02/14 00:42, Richard Smith wrote:
> > 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
>
> Thanks Richard, I don't know how I managed to miss that.
>
> Setting Token::StartOfLine as mentioned gets this right in the lexer.
> However, even though the b token in the output has both
> Token::StartOfLine and Token::LeadingSpace set,
> PrintPPOutputPPCallbacks::HandleFirstTokOnLine uses the column number to
> determine whether to print spaces, not the flag. As a result, the output
> becomes not
>
> a
> b
> c
>
> but
>
> a
> b
> c
>
> Given that the lexer behaviour is now right, would it be okay to leave
> this as it is, and only add a test to check that b is on a separate
> line, like attached? (I used first/second/third instead of a/b/c in an
> attempt to get FileCheck to print a better suggestion when the test
> fails, but it neither helps nor hurts.)
>
>
> I'm not sure whether we have consumers of -E that care about this
> detail, but I think this is an unrelated bug. Consider:
>
> #define foo(x) x y
> foo()
>
> This also misses the leading space from the output. So:
>
> 1) I think your patch is fine as-is, and
> 2) I think we should fix PrintPreprocessedTokens to correctly handle the
> first token on the line having leading space but being expanded from a
> token in the first column.
All right. Assuming that leading white space in column 1 is the only
case that gets mishandled, it seems like a special exception would
suffice. The attached (on top of my previous patch) passes testing. But
I am not all that familiar with this part of the code.
Note that the code below checks for # as a special exception: if that
appears at the start of a line, a space is output without changing
ColNo. While I do like consistency, using that approach would cause two
spaces to be printed instead of one if # should appear in column 1, with
leading white space.
Cheers,
Harald van Dijk
Leading whitespace at start of line
When a line starts with a token in column 1 that should have a space
before it, move it to column 2.
---
lib/Frontend/PrintPreprocessedOutput.cpp | 7 +++++++
test/Preprocessor/macro_expand_empty.c | 2 +-
2 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/lib/Frontend/PrintPreprocessedOutput.cpp b/lib/Frontend/PrintPreprocessedOutput.cpp
index f3393bf..d76a12e 100644
--- a/lib/Frontend/PrintPreprocessedOutput.cpp
+++ b/lib/Frontend/PrintPreprocessedOutput.cpp
@@ -556,6 +556,13 @@ bool PrintPPOutputPPCallbacks::HandleFirstTokOnLine(Token &Tok) {
// indented for easy reading.
unsigned ColNo = SM.getExpansionColumnNumber(Tok.getLocation());
+ // The first token on a line can have a column number of 1, yet still expect
+ // leading white space, if a macro expansion in column 1 starts with an empty
+ // macro argument, or an empty nested macro expansion. In this case, move the
+ // token to column 2.
+ if (ColNo == 1 && Tok.hasLeadingSpace())
+ ColNo = 2;
+
// This hack prevents stuff like:
// #define HASH #
// HASH define foo bar
diff --git a/test/Preprocessor/macro_expand_empty.c b/test/Preprocessor/macro_expand_empty.c
index c3fc4f2..5507728 100644
--- a/test/Preprocessor/macro_expand_empty.c
+++ b/test/Preprocessor/macro_expand_empty.c
@@ -17,5 +17,5 @@ IDENTITY0()
#define FOO() BAR() second
#define BAR()
first // CHECK: {{^}}first{{$}}
-FOO() // CHECK: second
+FOO() // CHECK: {{^}} second{{$}}
third // CHECK: {{^}}third{{$}}
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
