Fix whitespace before token-paste of an argument.

[jyknight]

Summary

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.

[jyknight]

Ping

[jyknight]

Ping.

[rsmith]

LGTM

test/Preprocessor/macro_paste_commaext.c

4

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

lib/Lex/TokenLexer.cpp

void TokenLexer::ExpandFunctionArguments() {
   // we install the newly expanded sequence as the new 'Tokens' list.
   bool MadeChange = false;
 
   for (unsigned i = 0, e = NumTokens; i != e; ++i) {
     // If we found the stringify operator, get the argument stringified.  The
     // preprocessor already verified that the following token is a macro name
     // when the #define was parsed.
     const Token &CurTok = Tokens[i];
+    // We don't want a space for the next token after a paste
+    // operator.  In valid code, the token will get smooshed onto the
+    // preceding one anyway. 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 (i != 0 && !Tokens[i-1].is(tok::hashhash) && CurTok.hasLeadingSpace())
       NextTokGetsSpace = true;
 
     if (CurTok.isOneOf(tok::hash, tok::hashat)) {
       int ArgNo = Macro->getArgumentNum(Tokens[i+1].getIdentifierInfo());
       assert(ArgNo != -1 && "Token following # is not an argument?");
 
       SourceLocation ExpansionLocStart =
if (!PasteBefore && !PasteAfter) {
       continue;
     }
 
     // Okay, we have a token that is either the LHS or RHS of a paste (##)
     // argument.  It gets substituted as its non-pre-expanded tokens.
     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);
       }
 
       ResultToks.append(ArgToks, ArgToks+NumToks);
 
       // If the '##' came from expanding an argument, turn it into 'unknown'
       // to avoid pasting.
       for (Token &Tok : llvm::make_range(ResultToks.end() - NumToks,
                                          ResultToks.end())) {
         if (Tok.is(tok::hashhash))
           Tok.setKind(tok::unknown);
       }
 
       if (ExpandLocStart.isValid()) {
         updateLocForMacroArgTokens(CurTok.getLocation(),
                                    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,
+                                                           false);
+        ResultToks[ResultToks.size()-NumToks].setFlagValue(Token::LeadingSpace,
+                                                           NextTokGetsSpace);
+      }
 
       NextTokGetsSpace = false;
       continue;
     }
 
     // If an empty argument is on the LHS or RHS of a paste, the standard (C99
     // 6.10.3.3p2,3) calls for a bunch of placemarker stuff to occur.  We
     // implement this by eating ## operators when a LHS or RHS expands to

test/Preprocessor/macro_paste_commaext.c

-// 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,);
-

test/Preprocessor/stringize_space.c

 // CHECK: {{^}}"a c"{{$}}
 
 #define str(x) #x
 #define f(x) str(-x)
 f(
     1)
 
 // CHECK: {{^}}"-1"
+
+#define paste(a,b) str(a<b##ld)
+paste(hello1, wor)
+paste(hello2,
+      wor)
+paste(hello3,
+wor)
+
+// CHECK: {{^}}"hello1<world"
+// CHECK: {{^}}"hello2<world"
+// CHECK: {{^}}"hello3<world"