[win][clang] Do not inject static_assert macro definition (#147030)

In ms-compatibility mode we inject static_assert macro definition if
assert macro is defined. This is done by
8da0903
for the sake of better diagnosing, in particular to emit a compatibility
warning when static_assert keyword is used without inclusion of
<assert.h>. Unfortunately it doesn't do a good job in c99 mode adding
that macro unexpectedly for the users, so this patch removes macro
injection and the diagnostics.

---------

Co-authored-by: Corentin Jabot <[email protected]>

Author: Fznamznon

clang/docs/ReleaseNotes.rst

@@ -756,6 +756,11 @@ Bug Fixes in This Version
 - Fixed an infinite recursion when checking constexpr destructors. (#GH141789)
 - Fixed a crash when a malformed using declaration appears in a ``constexpr`` function. (#GH144264)
 - Fixed a bug when use unicode character name in macro concatenation. (#GH145240)
+- Clang doesn't erroneously inject a ``static_assert`` macro in ms-compatibility and
+  -std=c99 mode. This resulted in deletion of ``-W/Wno-microsoft-static-assert``
+  flag and diagnostic because the macro injection was used to emit this warning.
+  Unfortunately there is no other good way to diagnose usage of ``static_assert``
+  macro without inclusion of ``<assert.h>``.
 
 Bug Fixes to Compiler Builtins
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

clang/include/clang/Basic/DiagnosticGroups.td

@@ -1450,7 +1450,6 @@ def MicrosoftAnonTag : DiagGroup<"microsoft-anon-tag">;
 def MicrosoftCommentPaste : DiagGroup<"microsoft-comment-paste">;
 def MicrosoftEndOfFile : DiagGroup<"microsoft-end-of-file">;
 def MicrosoftInaccessibleBase : DiagGroup<"microsoft-inaccessible-base">;
-def MicrosoftStaticAssert : DiagGroup<"microsoft-static-assert">;
 def MicrosoftInitFromPredefined : DiagGroup<"microsoft-init-from-predefined">;
 def MicrosoftStringLiteralFromPredefined : DiagGroup<
     "microsoft-string-literal-from-predefined">;
@@ -1472,7 +1471,7 @@ def Microsoft : DiagGroup<"microsoft",
      MicrosoftRedeclareStatic, MicrosoftEnumForwardReference, MicrosoftGoto,
      MicrosoftFlexibleArray, MicrosoftExtraQualification, MicrosoftCast,
      MicrosoftConstInit, MicrosoftVoidPseudoDtor, MicrosoftAnonTag,
-     MicrosoftCommentPaste, MicrosoftEndOfFile, MicrosoftStaticAssert,
+     MicrosoftCommentPaste, MicrosoftEndOfFile,
      MicrosoftInitFromPredefined, MicrosoftStringLiteralFromPredefined,
      MicrosoftInconsistentDllImport, MicrosoftInlineOnNonFunction]>;
 

clang/include/clang/Basic/DiagnosticParseKinds.td

@@ -474,9 +474,6 @@ def err_bool_redeclaration : Error<
 def warn_cxx98_compat_static_assert : Warning<
   "'static_assert' declarations are incompatible with C++98">,
   InGroup<CXX98Compat>, DefaultIgnore;
-def ext_ms_static_assert : ExtWarn<
-  "use of 'static_assert' without inclusion of <assert.h> is a Microsoft "
-  "extension">, InGroup<MicrosoftStaticAssert>;
 def ext_cxx_static_assert_no_message : ExtWarn<
   "'static_assert' with no message is a C++17 extension">, InGroup<CXX17>;
 def ext_c_static_assert_no_message : ExtWarn<

clang/lib/Lex/PPDirectives.cpp

@@ -3304,23 +3304,6 @@ void Preprocessor::HandleDefineDirective(
   // If the callbacks want to know, tell them about the macro definition.
   if (Callbacks)
     Callbacks->MacroDefined(MacroNameTok, MD);
-
-  // If we're in MS compatibility mode and the macro being defined is the
-  // assert macro, implicitly add a macro definition for static_assert to work
-  // around their broken assert.h header file in C. Only do so if there isn't
-  // already a static_assert macro defined.
-  if (!getLangOpts().CPlusPlus && getLangOpts().MSVCCompat &&
-      MacroNameTok.getIdentifierInfo()->isStr("assert") &&
-      !isMacroDefined("static_assert")) {
-    MacroInfo *MI = AllocateMacroInfo(SourceLocation());
-
-    Token Tok;
-    Tok.startToken();
-    Tok.setKind(tok::kw__Static_assert);
-    Tok.setIdentifierInfo(getIdentifierInfo("_Static_assert"));
-    MI->setTokens({Tok}, BP);
-    (void)appendDefMacroDirective(getIdentifierInfo("static_assert"), MI);
-  }
 }
 
 /// HandleUndefDirective - Implements \#undef.

clang/lib/Parse/ParseDeclCXX.cpp

@@ -937,9 +937,6 @@ Decl *Parser::ParseStaticAssertDeclaration(SourceLocation &DeclEnd) {
     if (!getLangOpts().CPlusPlus) {
       if (getLangOpts().C23)
         Diag(Tok, diag::warn_c23_compat_keyword) << Tok.getName();
-      else
-        Diag(Tok, diag::ext_ms_static_assert) << FixItHint::CreateReplacement(
-            Tok.getLocation(), "_Static_assert");
     } else
       Diag(Tok, diag::warn_cxx98_compat_static_assert);
   }

clang/test/Parser/static_assert.c

@@ -20,7 +20,6 @@ static_assert(1, ""); // c17-warning {{'static_assert' is a keyword in C23}} \
                       // c17-error {{expected ')'}} \
                       // c17-note {{to match this '('}} \
                       // c17-error {{type specifier missing, defaults to 'int'; ISO C99 and later do not support implicit int}} \
-                      // c17-ms-warning {{use of 'static_assert' without inclusion of <assert.h> is a Microsoft extension}}
 
 #endif
 

clang/test/Preprocessor/static_assert.c

@@ -1,5 +1,6 @@
 // RUN: %clang_cc1 -std=c11 -Wgnu-folding-constant -fsyntax-only -verify %s
 // RUN: %clang_cc1 -fms-compatibility -Wgnu-folding-constant -DMS -fsyntax-only -verify=expected,ms %s
+// RUN: %clang_cc1 -std=c99 -fms-compatibility -Wgnu-folding-constant -DMS -fsyntax-only -verify=expected,ms %s
 // RUN: %clang_cc1 -std=c99 -pedantic -Wgnu-folding-constant -fsyntax-only -verify=expected,ext %s
 // RUN: %clang_cc1 -xc++ -std=c++11 -pedantic -fsyntax-only -verify=expected,ext,cxx %s
 
@@ -13,15 +14,15 @@ _Static_assert(0, "0 is nonzero"); // expected-error {{static assertion failed:
                                    // ext-warning {{'_Static_assert' is a C11 extension}}
 
 #ifdef MS
-static_assert(1, "1 is nonzero"); // ms-warning {{use of 'static_assert' without inclusion of <assert.h> is a Microsoft extension}}
+static_assert(1, "1 is nonzero");
 #endif
 
 void foo(void) {
   _Static_assert(1, "1 is nonzero"); // ext-warning {{'_Static_assert' is a C11 extension}}
   _Static_assert(0, "0 is nonzero"); // expected-error {{static assertion failed: 0 is nonzero}} \
                                      // ext-warning {{'_Static_assert' is a C11 extension}}
 #ifdef MS
-  static_assert(1, "1 is nonzero"); // ms-warning {{use of 'static_assert' without}}
+  static_assert(1, "1 is nonzero");
 #endif
 }
 
@@ -38,7 +39,7 @@ struct A {
   _Static_assert(0, "0 is nonzero"); // expected-error {{static assertion failed: 0 is nonzero}} \
                                      // ext-warning {{'_Static_assert' is a C11 extension}}
 #ifdef MS
-  static_assert(1, "1 is nonzero"); // ms-warning {{use of 'static_assert' without}}
+  static_assert(1, "1 is nonzero");
 #endif
 };
 
@@ -64,16 +65,27 @@ typedef UNION(char, short) U3; // expected-error {{static assertion failed due t
 typedef UNION(float, 0.5f) U4; // expected-error {{expected a type}} \
                                // ext-warning 3 {{'_Static_assert' is a C11 extension}}
 
-// After defining the assert macro in MS-compatibility mode, we should
-// no longer warn about including <assert.h> under the assumption the
-// user already did that.
+// MSVC accepts static_assert in all language modes without including <assert.h>
+// and so do we in ms-compatibility mode. Unfortunately, there is no good way
+// to diagnose that with a pedantic warning. We'd have to track inclusion of
+// <assert.h> which is difficult when modules and PCH are involved. Adding
+// implicit definition of the macro causes unexpected results in c99 mode.
 #ifdef MS
+
+#if __STDC_VERSION__ < 201112L
+#if defined(static_assert)
+static_assert(0, "0 is nonzero"); // ok because we should not define static_assert
+                                  // macro in c99.
+#endif
+
+static_assert(0, "0 is nonzero"); // ms-error {{static assertion failed: 0 is nonzero}}
+#endif
+
 #define assert(expr)
 static_assert(1, "1 is nonzero"); // ok
 
-// But we should still warn if the user did something bonkers.
 #undef static_assert
-static_assert(1, "1 is nonzero"); // ms-warning {{use of 'static_assert' without}}
+static_assert(1, "1 is nonzero"); // yes, still ok.
 #endif
 
 _Static_assert(1 , "") // expected-error {{expected ';' after '_Static_assert'}} \