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

Fznamznon · 2025-07-04T10:01:57Z

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.

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 disagnostic.

llvmbot · 2025-07-04T10:02:30Z

@llvm/pr-subscribers-clang

Author: Mariya Podchishchaeva (Fznamznon)

Changes

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.

Full diff: https://github.com/llvm/llvm-project/pull/147030.diff

8 Files Affected:

(modified) clang/docs/ReleaseNotes.rst (+2)
(modified) clang/include/clang/Basic/DiagnosticGroups.td (+1-2)
(modified) clang/include/clang/Basic/DiagnosticParseKinds.td (-3)
(modified) clang/lib/Lex/PPDirectives.cpp (-17)
(modified) clang/lib/Parse/ParseDeclCXX.cpp (-3)
(modified) clang/test/Parser/static_assert.c (-1)
(removed) clang/test/Preprocessor/static_assert.c (-12)
(modified) clang/test/Sema/static-assert.c (+20-8)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 023f8ff7951d3..b66b2c0b7b707 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -750,6 +750,8 @@ 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 static_assert macro in ms-compatibility and
+  -std=c99 mode.
 
 Bug Fixes to Compiler Builtins
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td
index 36fa3227fd6a6..0603716942e0e 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/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]>;
 
diff --git a/clang/include/clang/Basic/DiagnosticParseKinds.td b/clang/include/clang/Basic/DiagnosticParseKinds.td
index 6c30da376dafb..1d86d1f548734 100644
--- a/clang/include/clang/Basic/DiagnosticParseKinds.td
+++ b/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<
diff --git a/clang/lib/Lex/PPDirectives.cpp b/clang/lib/Lex/PPDirectives.cpp
index e6da19d24f1c5..3fa060f7ec1bd 100644
--- a/clang/lib/Lex/PPDirectives.cpp
+++ b/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.
diff --git a/clang/lib/Parse/ParseDeclCXX.cpp b/clang/lib/Parse/ParseDeclCXX.cpp
index bd5c28b1992c4..a8832209d32b7 100644
--- a/clang/lib/Parse/ParseDeclCXX.cpp
+++ b/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);
   }
diff --git a/clang/test/Parser/static_assert.c b/clang/test/Parser/static_assert.c
index e9d7d3f0833f3..1b9b6b676e3e6 100644
--- a/clang/test/Parser/static_assert.c
+++ b/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
 
diff --git a/clang/test/Preprocessor/static_assert.c b/clang/test/Preprocessor/static_assert.c
deleted file mode 100644
index 7fa9975e1d468..0000000000000
--- a/clang/test/Preprocessor/static_assert.c
+++ /dev/null
@@ -1,12 +0,0 @@
-// RUN: %clang_cc1 -E -dM %s | FileCheck --strict-whitespace --check-prefix=NOMS %s
-// RUN: %clang_cc1 -fms-compatibility -E -dM %s | FileCheck --strict-whitespace --check-prefix=MS %s
-
-// If the assert macro is defined in MS compatibility mode in C, we
-// automatically inject a macro definition for static_assert. Test that the
-// macro is properly added to the preprocessed output. This allows us to
-// diagonse use of the static_assert keyword when <assert.h> has not been
-// included while still being able to compile preprocessed code.
-#define assert
-
-MS: #define static_assert _Static_assert
-NOMS-NOT: #define static_assert _Static_assert
diff --git a/clang/test/Sema/static-assert.c b/clang/test/Sema/static-assert.c
index d603bc19bb824..dd83cd7684d2c 100644
--- a/clang/test/Sema/static-assert.c
+++ b/clang/test/Sema/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,7 +14,7 @@ _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) {
@@ -21,7 +22,7 @@ void foo(void) {
   _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'}} \

cor3ntin

LGTM but please give @AaronBallman a chance to comment, as this reverts a previous patch of his (which I think was too clever)!

clang/docs/ReleaseNotes.rst

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

AaronBallman

LGTM! My cleverness didn't work out, alas. :-D

AaronBallman · 2025-07-07T11:52:24Z

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">;


I was curious whether we should leave the diagnostic group so it's not disruptive, but: https://sourcegraph.com/search?q=context:global+Wno-microsoft-static-assert&patternType=keyword&sm=0

Given there's so little use in the wild, I think it's better to just pull the group entirely so we don't keep it around as tech debt.

Does it make sense to mention in the release note that there is no flag anymore?

Might as well! Better to over-communicate than under.

Fznamznon added 2 commits July 4, 2025 02:50

Remove the diag

6b9dc4a

Fznamznon requested review from rnk and AaronBallman July 4, 2025 10:01

llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Jul 4, 2025

Fznamznon requested a review from zygoloid July 4, 2025 10:04

cor3ntin reviewed Jul 4, 2025

View reviewed changes

clang/docs/ReleaseNotes.rst Outdated Show resolved Hide resolved

cor3ntin approved these changes Jul 4, 2025

View reviewed changes

Apply suggestions from code review

1902654

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

AaronBallman approved these changes Jul 7, 2025

View reviewed changes

Fznamznon added 2 commits July 7, 2025 06:29

Merge branch 'main' into win-static-assert

7bd1216

Mention the downside

bd40def

Fznamznon merged commit 2837557 into llvm:main Jul 8, 2025
10 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

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

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

Uh oh!

Fznamznon commented Jul 4, 2025

Uh oh!

llvmbot commented Jul 4, 2025

Uh oh!

cor3ntin left a comment

Uh oh!

Uh oh!

AaronBallman left a comment

Uh oh!

AaronBallman Jul 7, 2025

Uh oh!

Fznamznon Jul 7, 2025

Uh oh!

AaronBallman Jul 7, 2025

Uh oh!

Fznamznon Jul 7, 2025

Uh oh!

Uh oh!

Uh oh!

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

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

Uh oh!

Conversation

Fznamznon commented Jul 4, 2025

Uh oh!

llvmbot commented Jul 4, 2025

Uh oh!

cor3ntin left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

AaronBallman left a comment

Choose a reason for hiding this comment

Uh oh!

AaronBallman Jul 7, 2025

Choose a reason for hiding this comment

Uh oh!

Fznamznon Jul 7, 2025

Choose a reason for hiding this comment

Uh oh!

AaronBallman Jul 7, 2025

Choose a reason for hiding this comment

Uh oh!

Fznamznon Jul 7, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!