[Preprocessor] Dont omit comma after __VA_ARGS__ in MSVC mode if it is an argument to a macro function that takes only a single parameter.

[erichkeane]

Summary

One fairly annoying exception to the comma elision rule is that if it is in an expansion that appears to expand to a macro-function-call that takes only a variadic macro.

[erichkeane]

This has likely gone stale, and my employer has decided to tell customers to modify their code.

lib/Lex/TokenLexer.cpp

bool TokenLexer::MaybeRemoveCommaBeforeVaArgs(
   // Is a comma available to be removed?
   if (ResultToks.empty() || !ResultToks.back().is(tok::comma))
     return false;
 
   // Issue an extension diagnostic for the paste operator.
   if (HasPasteOperator)
     PP.Diag(ResultToks.back().getLocation(), diag::ext_paste_comma);
 
+  // Don't remove the comma if it is going to be part of the sole parameter to a
+  // single-arg variadic macro function.
+  if (PP.getLangOpts().MSVCCompat) {
+    // Search backwards for an unclosed left-paren that looks like a function
+    // call.
+    unsigned RParenCount = 0;
+    int TokenIndex;
+    for (TokenIndex = ResultToks.size() - 1; TokenIndex >= 0; --TokenIndex) {
+      // Match up right to left parens.  Open left parens could be a cast or
+      // a grouping, so wait until we find an unmatched l-paren with an
+      // identifier
+      // before it, since that looks like a function.
+      if (ResultToks[TokenIndex].is(tok::r_paren))
+        ++RParenCount;
+      else if (ResultToks[TokenIndex].is(tok::l_paren) && RParenCount > 0)
+        --RParenCount;
+      else if (ResultToks[TokenIndex].is(tok::l_paren) && TokenIndex > 0 &&
+               ResultToks[TokenIndex - 1].is(tok::identifier)) {
+        MacroDefinition MD = PP.getMacroDefinition(
+            ResultToks[TokenIndex - 1].getIdentifierInfo());
+        auto *MI = MD ? MD.getMacroInfo() : nullptr;
+        if (MI && MI->isFunctionLike() && MI->isVariadic() &&
+            MI->getNumArgs() == 1)
+          return false;
+        break;
+      }
+    }
+  }
+
   // Remove the comma.
   ResultToks.pop_back();
 
   if (!ResultToks.empty()) {
     // If the comma was right after another paste (e.g. "X##,##__VA_ARGS__"),
     // then removal of the comma should produce a placemarker token (in C99
     // terms) which we model by popping off the previous ##, giving us a plain
     // "X" when __VA_ARGS__ is empty.
for (unsigned i = 0, e = NumTokens; i != e; ++i) {
     MadeChange = true;
 
     // Otherwise, this is a use of the argument.
 
     // In Microsoft mode, remove the comma before __VA_ARGS__ to ensure there
     // are no trailing commas if __VA_ARGS__ is empty.
     if (!PasteBefore && ActualArgs->isVarargsElidedUse() &&
         MaybeRemoveCommaBeforeVaArgs(ResultToks,
-                                     /*HasPasteOperator=*/false,
-                                     Macro, ArgNo, PP))
+                                     /*HasPasteOperator=*/false, Macro, ArgNo,
+                                     PP))
       continue;
 
     // If it is not the LHS/RHS of a ## operator, we must pre-expand the
     // argument and substitute the expanded tokens into the result.  This is
     // C99 6.10.3.1p1.
     if (!PasteBefore && !PasteAfter) {
       if (!ExpandSingleFunctionArgument(CurTok, ArgNo, ResultToks))
         MaybeRemoveCommaBeforeVaArgs(ResultToks,

test/Preprocessor/microsoft-ext.c

 int d,X()
 // CHECK: int d,,b;
 #undef Q
 #undef Z
 #undef Y
 #undef X
 #undef M
 
+// One exception to the __VA_ARGS__ comma expansion elision is when it is in an
+//  argument to a macro function that takes only a single parameter.
+#define MAC(...) __VA_ARGS__
+#define MF(x,...) fun(x, __VA_ARGS__,<-) 
+#define MM(x,...) MAC(x, __VA_ARGS__,<-)
+;MF(->);
+// Comma suppressed
+// CHECK: ;fun(->,<-);
+;MM(->);
+// comma not suppressed
+// CHECK: ;->, ,<-;
+#undef MM
+#undef MF
+#undef MAC
+
+// And a more real-life example
+#define VA_ARGS(...) __VA_ARGS__
+#define VA_GET(ARG, ...) ARG
+#define FOO(X, ...) VA_ARGS(VA_GET(__VA_ARGS__)(VA_ARGS(__VA_ARGS__)))  
+#define BAR(X, ...) VA_ARGS(VA_GET(X)(X, __VA_ARGS__, D))
+#define BAR2(X, ...) VA_ARGS(VA_GET(X)(X, VA_ARGS(__VA_ARGS__), D))
+FOO(A, BAR, C);
+// CHECK: BAR(BAR, C, , D);
+FOO(A, BAR2, C);
+// MSVC actually DOES suppress the comma if this empty expansion is the result 
+// of a function instead of just a __VA_ARGS__, which is inconsistent with 
+// normal comma removal.
+// CHECK: BAR2(BAR2, C, D);
+#undef BAR2
+#undef BAR
+#undef FOO
+#undef VA_GET
+#undef VA_ARGS