[PATCH] D30427: Fix whitespace before token-paste of an argument.

2017-05-04 Thread James Y Knight via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL302195: Fix whitespace before token-paste of an argument. 
(authored by jyknight).

Changed prior to commit:
  https://reviews.llvm.org/D30427?vs=89918=97881#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D30427

Files:
  cfe/trunk/lib/Lex/TokenLexer.cpp
  cfe/trunk/test/Preprocessor/macro_paste_commaext.c
  cfe/trunk/test/Preprocessor/stringize_space.c

Index: cfe/trunk/test/Preprocessor/macro_paste_commaext.c
===
--- cfe/trunk/test/Preprocessor/macro_paste_commaext.c
+++ cfe/trunk/test/Preprocessor/macro_paste_commaext.c
@@ -1,13 +1,20 @@
-// RUN: %clang_cc1 %s -E | grep 'V);'
-// RUN: %clang_cc1 %s -E | grep 'W, 1, 2);'
-// RUN: %clang_cc1 %s -E | grep 'X, 1, 2);'
-// RUN: %clang_cc1 %s -E | grep 'Y,);'
-// RUN: %clang_cc1 %s -E | grep 'Z,);'
+// RUN: %clang_cc1 %s -E | FileCheck --strict-whitespace --match-full-lines %s
+
+// In the following tests, note that the output is sensitive to the
+// whitespace *preceeding* the varargs argument, as well as to
+// interior whitespace. AFAIK, this is the only case where whitespace
+// preceeding an argument matters, and might be considered a bug in
+// GCC. Nevertheless, since this feature is a GCC extension in the
+// first place, we'll follow along.
 
 #define debug(format, ...) format, ## __VA_ARGS__)
+// CHECK:V);
 debug(V);
-debug(W, 1, 2);
+// CHECK:W,1, 2);
+debug(W,1, 2);
+// CHECK:X, 1, 2);
 debug(X, 1, 2 );
+// CHECK:Y,);
 debug(Y, );
+// CHECK:Z,);
 debug(Z,);
-
Index: cfe/trunk/test/Preprocessor/stringize_space.c
===
--- cfe/trunk/test/Preprocessor/stringize_space.c
+++ cfe/trunk/test/Preprocessor/stringize_space.c
@@ -18,3 +18,14 @@
 1)
 
 // CHECK: {{^}}"-1"
+
+#define paste(a,b) str(a ".foo" not ". foo".
 if (i != 0 && !Tokens[i-1].is(tok::hashhash) && CurTok.hasLeadingSpace())
   NextTokGetsSpace = true;
 
@@ -317,14 +323,16 @@
 const Token *ArgToks = ActualArgs->getUnexpArgument(ArgNo);
 unsigned NumToks = MacroArgs::getArgLength(ArgToks);
 if (NumToks) {  // Not an empty argument?
+  bool VaArgsPseudoPaste = false;
   // If this is the GNU ", ## __VA_ARGS__" extension, and we just learned
   // that __VA_ARGS__ expands to multiple tokens, avoid a pasting error when
   // the expander trys to paste ',' with the first token of the __VA_ARGS__
   // expansion.
   if (NonEmptyPasteBefore && ResultToks.size() >= 2 &&
   ResultToks[ResultToks.size()-2].is(tok::comma) &&
   (unsigned)ArgNo == Macro->getNumArgs()-1 &&
   Macro->isVariadic()) {
+VaArgsPseudoPaste = true;
 // Remove the paste operator, report use of the extension.
 PP.Diag(ResultToks.pop_back_val().getLocation(), diag::ext_paste_comma);
   }
@@ -344,18 +352,16 @@
ResultToks.end()-NumToks, ResultToks.end());
   }
 
-  // If this token (the macro argument) was supposed to get leading
-  // whitespace, transfer this information onto the first token of the
-  // expansion.
-  //
-  // Do not do this if the paste operator occurs before the macro argument,
-  // as in "A ## MACROARG".  In valid code, the first token will get
-  // smooshed onto the preceding one anyway (forming AMACROARG).  In
-  // assembler-with-cpp mode, invalid pastes are allowed through: in this
-  // case, we do not want the extra whitespace to be added.  For example,
-  // we want ". ## foo" -> ".foo" not ". foo".
-  if (NextTokGetsSpace)
-ResultToks[ResultToks.size()-NumToks].setFlag(Token::LeadingSpace);
+  // Transfer the leading whitespace information from the token
+  // (the macro argument) onto the first token of the
+  // expansion. Note that we don't do this for the GNU
+  // pseudo-paste extension ", ## __VA_ARGS__".
+  if (!VaArgsPseudoPaste) {
+ResultToks[ResultToks.size() - 

[PATCH] D30427: Fix whitespace before token-paste of an argument.

2017-04-27 Thread Richard Smith via Phabricator via cfe-commits
rsmith accepted this revision.
rsmith added a comment.
This revision is now accepted and ready to land.

LGTM




Comment at: test/Preprocessor/macro_paste_commaext.c:4
+// In the following tests, note that the output is sensitive to the
+// whitespace *preceeding* the varargs argument, as well as to
+// interior whitespace. AFAIK, this is the only case where whitespace

Typo (too many 'e's in "preceding").


https://reviews.llvm.org/D30427



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D30427: Fix whitespace before token-paste of an argument.

2017-03-27 Thread James Y Knight via Phabricator via cfe-commits
jyknight added a comment.

Ping.


https://reviews.llvm.org/D30427



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D30427: Fix whitespace before token-paste of an argument.

2017-03-06 Thread James Y Knight via Phabricator via cfe-commits
jyknight added a comment.

Ping


https://reviews.llvm.org/D30427



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D30427: Fix whitespace before token-paste of an argument.

2017-02-27 Thread James Y Knight via Phabricator via cfe-commits
jyknight created this revision.

The whitespace should come from the argument name in the macro
expansion, rather than from the token passed to the macro (same as it
does when not pasting).

Added a new test case for the change in behavior to stringize_space.c.

FileCheck'ized macro_paste_commaext.c, tweaked the test case, and
added a comment; no behavioral change to this test.


https://reviews.llvm.org/D30427

Files:
  lib/Lex/TokenLexer.cpp
  test/Preprocessor/macro_paste_commaext.c
  test/Preprocessor/stringize_space.c

Index: test/Preprocessor/stringize_space.c
===
--- test/Preprocessor/stringize_space.c
+++ test/Preprocessor/stringize_space.c
@@ -18,3 +18,14 @@
 1)
 
 // CHECK: {{^}}"-1"
+
+#define paste(a,b) str(a ".foo" not ". foo".
 if (i != 0 && !Tokens[i-1].is(tok::hashhash) && CurTok.hasLeadingSpace())
   NextTokGetsSpace = true;
 
@@ -317,14 +323,16 @@
 const Token *ArgToks = ActualArgs->getUnexpArgument(ArgNo);
 unsigned NumToks = MacroArgs::getArgLength(ArgToks);
 if (NumToks) {  // Not an empty argument?
+  bool VaArgsPseudoPaste = false;
   // If this is the GNU ", ## __VA_ARGS__" extension, and we just learned
   // that __VA_ARGS__ expands to multiple tokens, avoid a pasting error when
   // the expander trys to paste ',' with the first token of the __VA_ARGS__
   // expansion.
   if (NonEmptyPasteBefore && ResultToks.size() >= 2 &&
   ResultToks[ResultToks.size()-2].is(tok::comma) &&
   (unsigned)ArgNo == Macro->getNumArgs()-1 &&
   Macro->isVariadic()) {
+VaArgsPseudoPaste = true;
 // Remove the paste operator, report use of the extension.
 PP.Diag(ResultToks.pop_back_val().getLocation(), diag::ext_paste_comma);
   }
@@ -344,18 +352,16 @@
ResultToks.end()-NumToks, ResultToks.end());
   }
 
-  // If this token (the macro argument) was supposed to get leading
-  // whitespace, transfer this information onto the first token of the
-  // expansion.
-  //
-  // Do not do this if the paste operator occurs before the macro argument,
-  // as in "A ## MACROARG".  In valid code, the first token will get
-  // smooshed onto the preceding one anyway (forming AMACROARG).  In
-  // assembler-with-cpp mode, invalid pastes are allowed through: in this
-  // case, we do not want the extra whitespace to be added.  For example,
-  // we want ". ## foo" -> ".foo" not ". foo".
-  if (NextTokGetsSpace)
-ResultToks[ResultToks.size()-NumToks].setFlag(Token::LeadingSpace);
+  // Transfer the leading whitespace information from the token
+  // (the macro argument) onto the first token of the
+  // expansion. Note that we don't do this for the GNU
+  // pseudo-paste extension ", ## __VA_ARGS__".
+  if (!VaArgsPseudoPaste) {
+ResultToks[ResultToks.size()-NumToks].setFlagValue(Token::StartOfLine,
+