[PATCH] D69626: Fix Microsoft compatibility handling of commas in nested macro expansions.

[Eric Astor via Phabricator via cfe-commits]

epastor created this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
epastor added reviewers: rnk, thakis.

In Microsoft-compatibility mode, single commas from nested macro expansions
should not be considered as argument separators; we emulate this by marking
them to be ignored. However, in MSVC's preprocessor, subsequent expansions
DO treat these commas as argument separators... so we now ignore each comma
at most once.

Includes a small unit test that validates we match MSVC's behavior as shown in https://gcc.godbolt.org/z/y0twaq


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D69626

Files:
  clang/lib/Lex/PPMacroExpansion.cpp
  clang/test/Preprocessor/microsoft-ext.c


Index: clang/test/Preprocessor/microsoft-ext.c
===================================================================
--- clang/test/Preprocessor/microsoft-ext.c
+++ clang/test/Preprocessor/microsoft-ext.c
@@ -23,6 +23,19 @@
                 HAS_1_TEMPLATE_PARAMS(int, k),
                 AND_2_VALUE_PARAMS(p0, p1));
 
+// Regression test for PR43282; check that we match MSVC's failure to unpack
+// __VA_ARGS__ unless forwarded through another macro.
+#define THIRD_ARGUMENT(A, B, C, ...) C
+#define TEST(...) THIRD_ARGUMENT(__VA_ARGS__, 1, 2)
+#define COMBINE(...) __VA_ARGS__
+#define WRAPPED_TEST(...) COMBINE(THIRD_ARGUMENT(__VA_ARGS__, 1, 2))
+// Check that we match MSVC's failure to unpack __VA_ARGS__, unless forwarded
+// through another macro
+auto packed = TEST(,);
+auto unpacked = WRAPPED_TEST(,);
+// CHECK: auto packed = 2;
+// CHECK: auto unpacked = 1;
+
 // This tests compatibility with behaviour needed for type_traits in VS2012
 // Test based on _VARIADIC_EXPAND_0X macros in xstddef of VS2012
 #define _COMMA ,
Index: clang/lib/Lex/PPMacroExpansion.cpp
===================================================================
--- clang/lib/Lex/PPMacroExpansion.cpp
+++ clang/lib/Lex/PPMacroExpansion.cpp
@@ -820,18 +820,26 @@
         }
       } else if (Tok.is(tok::l_paren)) {
         ++NumParens;
-      } else if (Tok.is(tok::comma) && NumParens == 0 &&
-                 !(Tok.getFlags() & Token::IgnoredComma)) {
+      } else if (Tok.is(tok::comma)) {
         // In Microsoft-compatibility mode, single commas from nested macro
         // expansions should not be considered as argument separators. We test
-        // for this with the IgnoredComma token flag above.
-
-        // Comma ends this argument if there are more fixed arguments expected.
-        // However, if this is a variadic macro, and this is part of the
-        // variadic part, then the comma is just an argument token.
-        if (!isVariadic) break;
-        if (NumFixedArgsLeft > 1)
-          break;
+        // for this with the IgnoredComma token flag.
+        if (Tok.getFlags() & Token::IgnoredComma) {
+          // However, in MSVC's preprocessor, subsequent expansions do treat
+          // these commas as argument separators. This leads to a common
+          // workaround used in macros that need to work in both MSVC and
+          // compliant preprocessors. Therefore, the IgnoredComma flag can only
+          // apply once to any given token.
+          Tok.clearFlag(Token::IgnoredComma);
+        } else if (NumParens == 0) {
+          // Comma ends this argument if there are more fixed arguments
+          // expected. However, if this is a variadic macro, and this is part of
+          // the variadic part, then the comma is just an argument token.
+          if (!isVariadic)
+            break;
+          if (NumFixedArgsLeft > 1)
+            break;
+        }
       } else if (Tok.is(tok::comment) && !KeepMacroComments) {
         // If this is a comment token in the argument list and we're just in
         // -C mode (not -CC mode), discard the comment.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D69626.227132.patch
Type: text/x-patch
Size: 3098 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20191030/0ee2db40/attachment.bin>