[MSVC Compat] Warn when suppressing a trailing comma in macro args

[• ehsan]

Summary

MSVC suppresses a trailing comma if no arguments are passed in place of
a variadic argument[1]. This patch adds a -Wmicrosoft warning when
accepting such code.

[1] https://msdn.microsoft.com/en-us/library/ms177415.aspx

[thakis]

Sorry for the slow response. I was reading http://reviews.llvm.org/D15670 to understand this patch, and I couldn't find anything in that patch that enables this extension only in Microsoft mode. Trying suppressed-comma-msextension.cpp locally, it seems to pass without -fms-compatibility too. Am I missing something, or does http://reviews.llvm.org/D15670 lack a check for MicrosoftExt?

[• ehsan]

In D16503#342116, @thakis wrote:

Sorry for the slow response. I was reading http://reviews.llvm.org/D15670 to understand this patch, and I couldn't find anything in that patch that enables this extension only in Microsoft mode. Trying suppressed-comma-msextension.cpp locally, it seems to pass without -fms-compatibility too. Am I missing something, or does http://reviews.llvm.org/D15670 lack a check for MicrosoftExt?

Hmm, is the PP.getLangOpts().MSVCCompat check earlier in MaybeRemoveCommaBeforeVaArgs() insufficient?

(Note that this whole thing mostly builds on top of r167613 which was done by Andy, so it's possible that I needed to do something extra and I forgot that...)

include/clang/Basic/DiagnosticGroups.td

 def MicrosoftFlexibleArray : DiagGroup<"microsoft-flexible-array">;
 def MicrosoftExtraQualification : DiagGroup<"microsoft-extra-qualification">;
 def MicrosoftCast : DiagGroup<"microsoft-cast">;
 def MicrosoftConstInit : DiagGroup<"microsoft-const-init">;
 def MicrosoftVoidPseudoDtor : DiagGroup<"microsoft-void-pseudo-dtor">;
 def MicrosoftAnonTag : DiagGroup<"microsoft-anon-tag">;
 def MicrosoftCommentPaste : DiagGroup<"microsoft-comment-paste">;
 def MicrosoftEndOfFile : DiagGroup<"microsoft-end-of-file">;
+def MicrosoftElidedMacroArgument : DiagGroup<"microsoft-elided-macro-argument">;
 // Aliases.
 def : DiagGroup<"msvc-include", [MicrosoftInclude]>;
                 // -Wmsvc-include = -Wmicrosoft-include
 
 // Warnings group for warnings about Microsoft extensions.
 def Microsoft : DiagGroup<"microsoft",
     [MicrosoftCharize, MicrosoftInclude, MicrosoftCppMacro, MicrosoftFixedEnum,
      MicrosoftSealed, MicrosoftUnqualifiedFriend, MicrosoftExceptionSpec,
      MicrosoftUsingDecl, MicrosoftMutableReference, MicrosoftPureDefinition,
      MicrosoftUnionMemberReference, MicrosoftExplicitConstructorCall,
      MicrosoftEnumValue, MicrosoftDefaultArgRedefinition, MicrosoftTemplate,
      MicrosoftRedeclareStatic, MicrosoftEnumForwardReference, MicrosoftGoto,
      MicrosoftFlexibleArray, MicrosoftExtraQualification, MicrosoftCast,
      MicrosoftConstInit, MicrosoftVoidPseudoDtor, MicrosoftAnonTag,
-     MicrosoftCommentPaste, MicrosoftEndOfFile]>;
+     MicrosoftCommentPaste, MicrosoftEndOfFile, MicrosoftElidedMacroArgument]>;
 
 def ObjCNonUnifiedException : DiagGroup<"objc-nonunified-exceptions">;
 
 def ObjCProtocolMethodImpl : DiagGroup<"objc-protocol-method-implementation">;
 
 def ObjCNoPropertyAutoSynthesis : DiagGroup<"objc-property-synthesis">;
 
 // ObjC API warning groups.

include/clang/Basic/DiagnosticLexKinds.td

def ext_charize_microsoft : Extension<
   "charizing operator #@ is a Microsoft extension">,
   InGroup<MicrosoftCharize>;
 def ext_comment_paste_microsoft : Extension<
   "pasting two '/' tokens into a '//' comment is a Microsoft extension">,
   InGroup<MicrosoftCommentPaste>;
 def ext_ctrl_z_eof_microsoft : Extension<
   "treating Ctrl-Z as end-of-file is a Microsoft extension">,
   InGroup<MicrosoftEndOfFile>;
+def ext_elided_macro_argument_microsoft: Extension<
+  "suppressing trailing comma when not specifying any macro variadic "
+  "arguments is a Microsoft extension">,
+  InGroup<MicrosoftElidedMacroArgument>;
 
 def ext_token_used : Extension<"extension used">,
   InGroup<DiagGroup<"language-extension-token">>;
 
 def warn_cxx11_keyword : Warning<"'%0' is a keyword in C++11">,
   InGroup<CXX11Compat>, DefaultIgnore;
 
 def ext_unterminated_char_or_string : ExtWarn<

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);
+  else
+    // Issue an extension diagnostic for the suppressed trailing comma.
+    PP.Diag(ResultToks.back().getLocation(), diag::ext_elided_macro_argument_microsoft);
 
   // 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

test/Preprocessor/microsoft-ext.c

 
 #define MAKER(_arg1, _comma, _arg2)                                            \
   void func(_arg1 _comma _arg2) {}
 #define MAKE_FUNC(_makerP1, _makerP2, _arg1, _comma, _arg2)                    \
   _makerP1##_makerP2(_arg1, _comma, _arg2)
 
 MAKE_FUNC(MAK, ER, int a, _COMMA, int b);
 // CHECK: void func(int a , int b) {}
-
-#define macro(a, b) (a - b)
-void function(int a);
-#define COMMA_ELIDER(...) \
-  macro(x, __VA_ARGS__); \
-  function(x, __VA_ARGS__);
-COMMA_ELIDER();
-// CHECK: (x - );
-// CHECK: function(x);

test/Preprocessor/suppressed-comma-msextension.cpp

+// RUN: %clang_cc1 -verify -fms-compatibility -Wmicrosoft %s
+// RUN: %clang_cc1 -P -E -fms-compatibility %s | FileCheck %s
+
+void function(int a);
+#define macro(a, b) function(b - a)
+#define COMMA_ELIDER(...) \
+  macro(x, __VA_ARGS__); \
+  function(x, __VA_ARGS__);
+// expected-warning@-2 {{suppressing trailing comma when not specifying any macro variadic arguments is a Microsoft extension}}
+// expected-warning@-2 {{suppressing trailing comma when not specifying any macro variadic arguments is a Microsoft extension}}
+void func() {
+int x;
+COMMA_ELIDER()
+// CHECK: function( - x); function(x);
+}