[PATCH] Preprocessor: preserve whitespace in -traditional-cpp mode.

Jordan Rose jordan_rose at apple.com
Wed Feb 13 10:11:43 PST 2013



================
Comment at: test/Preprocessor/traditional-cpp.c:16-17
@@ +15,4 @@
+/* The following lines contain tab characters; do not change them! */
+/* CHECK:	{{i}}ndented!
+ * CHECK: {{tab}}	{{separated}}	{{values}}
+ */
----------------
Dmitri Gribenko wrote:
> Richard Smith wrote:
> > What are the {{ }} for here? Is this some oddity of -strict-whitespace?
> It might have been cleaner to use the {{^}}...{{$}} pattern for this test.
Ah, yes. Richard: these are needed because otherwise the CHECK line matches itself. Dmitri's change would allow them to match the whole line instead.

================
Comment at: lib/Lex/Lexer.cpp:1879-1882
@@ -1877,4 +1878,6 @@
   if (isKeepWhitespaceMode()) {
     FormTokenWithChars(Result, CurPtr, tok::unknown);
+    if (SawNewline)
+      IsAtStartOfLine = true;
     return true;
   }
----------------
Richard Smith wrote:
> Could we handle all the changes to this function by just setting IsAtStartOfLine to Result.hasFlag(Token::StartOfLine) and clearing the Token::StartOfLine flag here?
> 
> I'm also not sure whether the current approach does the right thing if (for instance) a file starts with <space>#define <...>. Will we set the StartOfLine flag for the '#' token?
That almost works, but then we'd lose the validity of StartOfLine for whitespace that //does// start a line, but also starts the next line. We might not care, though.

As for the start-of-file case, we do get this correct because we don't set IsAtStartOfLine to false in this method.

================
Comment at: lib/Lex/Lexer.cpp:2661-2666
@@ -2649,8 +2660,8 @@
       << makeCharRange(*this, BufferPtr, CurPtr);
 
-    Result.setFlag(Token::LeadingSpace);
     if (SkipWhitespace(Result, CurPtr))
       return; // KeepWhitespaceMode
 
+    Result.setFlag(Token::LeadingSpace);
     return LexTokenInternal(Result);
   }
----------------
Richard Smith wrote:
> If the sequence of whitespace ends in a newline, we would previously not set the LeadingSpace flag, now we always set it. Is that what we want? (Likewise for all the similar changes below.)
Hm. I was probably overzealous in moving around the LeadingSpace flag—it's often paired with StartOfLine, but it doesn't have the same behavior when there are whitespace tokens involved. I'll go check back on all of these.


http://llvm-reviews.chandlerc.com/D399



More information about the cfe-commits mailing list