[Clang][Parser] Accept P2741R3 (static_assert with user-generated message) in C++11 as an extension

[MitalAshok]

Added a new -Wpre-c++26-compat warning for when this feature is used in C++26 and a -Wc++26-extensions warning for when this is used in C++11 through C++23.

[llvmbot]

@llvm/pr-subscribers-debuginfo

@llvm/pr-subscribers-clang

Author: Mital Ashok (MitalAshok)

Changes

Added a new -Wpre-c++26-compat warning for when this feature is used in C++26 and a -Wc++26-extensions warning for when this is used in C++11 through C++23.

---

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

6 Files Affected:

• (modified) clang/docs/LanguageExtensions.rst (+2)
• (modified) clang/include/clang/Basic/DiagnosticParseKinds.td (+6)
• (modified) clang/lib/Frontend/InitPreprocessor.cpp (+2-4)
• (modified) clang/lib/Parse/ParseDeclCXX.cpp (+7-3)
• (modified) clang/test/Lexer/cxx-features.cpp (+1-1)
• (added) clang/test/SemaCXX/static-assert-ext.cpp (+28)

diff --git a/clang/docs/LanguageExtensions.rst b/clang/docs/LanguageExtensions.rst
index be07f81cc41b0..5953ca5259da2 100644
--- a/clang/docs/LanguageExtensions.rst
+++ b/clang/docs/LanguageExtensions.rst
@@ -1483,6 +1483,7 @@ Generic lambda expressions                   __cpp_generic_lambdas            C+
 variable templates                           __cpp_variable_templates         C++14         C++03
 Binary literals                              __cpp_binary_literals            C++14         C++03
 Relaxed constexpr                            __cpp_constexpr                  C++14         C++11
+Static assert with no message                __cpp_static_assert>=201411L     C++17         C++11
 Pack expansion in generalized lambda-capture __cpp_init_captures              C++17         C++03
 ``if constexpr``                             __cpp_if_constexpr               C++17         C++11
 fold expressions                             __cpp_fold_expressions           C++17         C++03
@@ -1503,6 +1504,7 @@ Conditional ``explicit``                     __cpp_conditional_explicit       C+
 ``static operator()``                        __cpp_static_call_operator       C++23         C++03
 Attributes on Lambda-Expressions                                              C++23         C++11
 Attributes on Structured Bindings            __cpp_structured_bindings        C++26         C++03
+Static assert with user-generated message    __cpp_static_assert>=202306L     C++26         C++11
 Pack Indexing                                __cpp_pack_indexing              C++26         C++03
 ``= delete ("should have a reason");``       __cpp_deleted_function           C++26         C++03
 -------------------------------------------- -------------------------------- ------------- -------------
diff --git a/clang/include/clang/Basic/DiagnosticParseKinds.td b/clang/include/clang/Basic/DiagnosticParseKinds.td
index f8d50d12bb935..eaad99451eb7f 100644
--- a/clang/include/clang/Basic/DiagnosticParseKinds.td
+++ b/clang/include/clang/Basic/DiagnosticParseKinds.td
@@ -470,6 +470,12 @@ def warn_c17_compat_static_assert_no_message : Warning<
   "'_Static_assert' with no message is incompatible with C standards before "
   "C23">,
   DefaultIgnore, InGroup<CPre23Compat>;
+def ext_cxx_static_assert_user_generated_message : ExtWarn<
+  "'static_assert' with a user-generated message is a C++26 extension">,
+  InGroup<CXX26>;
+def warn_cxx20_compat_static_assert_user_generated_message : Warning<
+  "'static_assert' with a user-generated message is incompatible with "
+  "C++ standards before C++26">, DefaultIgnore, InGroup<CXXPre26Compat>;
 def err_function_definition_not_allowed : Error<
   "function definition is not allowed here">;
 def err_expected_end_of_enumerator : Error<
diff --git a/clang/lib/Frontend/InitPreprocessor.cpp b/clang/lib/Frontend/InitPreprocessor.cpp
index 8e62461d8a181..038fad5272be7 100644
--- a/clang/lib/Frontend/InitPreprocessor.cpp
+++ b/clang/lib/Frontend/InitPreprocessor.cpp
@@ -671,10 +671,8 @@ static void InitializeCPlusPlusFeatureTestMacros(const LangOptions &LangOpts,
                         LangOpts.CPlusPlus23   ? "202211L"
                         : LangOpts.CPlusPlus17 ? "201603L"
                                                : "200907");
-    Builder.defineMacro("__cpp_static_assert", LangOpts.CPlusPlus26 ? "202306L"
-                                               : LangOpts.CPlusPlus17
-                                                   ? "201411L"
-                                                   : "200410");
+    // C++17 / C++26 static_assert backported
+    Builder.defineMacro("__cpp_static_assert", "202306L");
     Builder.defineMacro("__cpp_decltype", "200707L");
     Builder.defineMacro("__cpp_attributes", "200809L");
     Builder.defineMacro("__cpp_rvalue_references", "200610L");
diff --git a/clang/lib/Parse/ParseDeclCXX.cpp b/clang/lib/Parse/ParseDeclCXX.cpp
index aac89d910bbc8..87ba660c35b55 100644
--- a/clang/lib/Parse/ParseDeclCXX.cpp
+++ b/clang/lib/Parse/ParseDeclCXX.cpp
@@ -1073,7 +1073,7 @@ Decl *Parser::ParseStaticAssertDeclaration(SourceLocation &DeclEnd) {
     }
 
     bool ParseAsExpression = false;
-    if (getLangOpts().CPlusPlus26) {
+    if (getLangOpts().CPlusPlus11) {
       for (unsigned I = 0;; ++I) {
         const Token &T = GetLookAheadToken(I);
         if (T.is(tok::r_paren))
@@ -1085,9 +1085,13 @@ Decl *Parser::ParseStaticAssertDeclaration(SourceLocation &DeclEnd) {
       }
     }
 
-    if (ParseAsExpression)
+    if (ParseAsExpression) {
+      Diag(Tok,
+           getLangOpts().CPlusPlus26
+               ? diag::warn_cxx20_compat_static_assert_user_generated_message
+               : diag::ext_cxx_static_assert_user_generated_message);
       AssertMessage = ParseConstantExpressionInExprEvalContext();
-    else if (tokenIsLikeStringLiteral(Tok, getLangOpts()))
+    } else if (tokenIsLikeStringLiteral(Tok, getLangOpts()))
       AssertMessage = ParseUnevaluatedStringLiteralExpression();
     else {
       Diag(Tok, diag::err_expected_string_literal)
diff --git a/clang/test/Lexer/cxx-features.cpp b/clang/test/Lexer/cxx-features.cpp
index 08b732132228b..47e9ae9f5d2c5 100644
--- a/clang/test/Lexer/cxx-features.cpp
+++ b/clang/test/Lexer/cxx-features.cpp
@@ -321,7 +321,7 @@
 #error "wrong value for __cpp_range_based_for"
 #endif
 
-#if check(static_assert, 0, 200410, 200410, 201411, 201411, 201411, 202306)
+#if check(static_assert, 0, 202306, 202306, 202306, 202306, 202306, 202306)
 #error "wrong value for __cpp_static_assert"
 #endif
 
diff --git a/clang/test/SemaCXX/static-assert-ext.cpp b/clang/test/SemaCXX/static-assert-ext.cpp
new file mode 100644
index 0000000000000..05f7a0e96974a
--- /dev/null
+++ b/clang/test/SemaCXX/static-assert-ext.cpp
@@ -0,0 +1,28 @@
+// RUN: %clang_cc1 -std=c++98 -fsyntax-only -pedantic %s -verify=precxx11,precxx17,precxx26
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -pedantic %s -verify=since-cxx11,precxx17,precxx26 -Wc++98-compat
+// RUN: %clang_cc1 -std=c++17 -fsyntax-only -pedantic %s -verify=since-cxx11,since-cxx17,precxx26 -Wc++98-compat -Wpre-c++17-compat
+// RUN: %clang_cc1 -std=c++26 -fsyntax-only -pedantic %s -verify=since-cxx11,since-cxx17,since-cxx26 -Wc++98-compat -Wpre-c++17-compat -Wpre-c++26-compat
+
+static_assert(false, "a");
+// precxx11-error@-1 {{a type specifier is required for all declarations}}
+// since-cxx11-warning@-2 {{'static_assert' declarations are incompatible with C++98}}
+// since-cxx11-error@-3 {{static assertion failed: a}}
+
+#if __cplusplus >= 201103L
+static_assert(false);
+// since-cxx11-warning@-1 {{'static_assert' declarations are incompatible with C++98}}
+// precxx17-warning@-2 {{'static_assert' with no message is a C++17 extension}}
+// since-cxx17-warning@-3 {{'static_assert' with no message is incompatible with C++ standards before C++17}}
+// since-cxx11-error@-4 {{static assertion failed}}
+
+struct X {
+    static constexpr int size() { return 1; } // since-cxx11-warning {{'constexpr'}}
+    static constexpr const char* data() { return "b"; } // since-cxx11-warning {{'constexpr'}}
+};
+
+static_assert(false, X());
+// since-cxx11-warning@-1 {{'static_assert' declarations are incompatible with C++98}}
+// precxx26-warning@-2 {{'static_assert' with a user-generated message is a C++26 extension}}
+// since-cxx26-warning@-3 {{'static_assert' with a user-generated message is incompatible with C++ standards before C++26}}
+// since-cxx11-error@-4 {{static assertion failed: b}}
+#endif

[MitalAshok]

This doesn't make sense to backport to C++98's _Static_assert extension (no constexpr member functions)

CC @cor3ntin @AaronBallman

Re: affects on libc++/any downstream:
Other than the feature test macro, this extension should not affect any valid C++11/20 program, since previously it just didn't parse.

[cor3ntin]

That seems reasonable to me.
We need a changelog entry for that change tough

[MitalAshok]

I guess this is one of the potentially-breaking changes that comes with changing the feature test macro. I think this is incorrect code and should have been __cpp_static_assert >= 202306L && __cpp_constexpr >= 201907L.

For DR testing, I don't think we check extensions, which is why I changed this to check __cplusplus. @Endilll is that right?

[Endilll]

Yeah, checking for __cplusplus makes more sense in DR test suite.
But I wonder if we can leverage feature test macros for a future conformance testing outside of DRs.

[philnik777]

AFAICT this is non-conforming. C++20 requires __cpp_static_assert to be 201411L. Personally I think that's a defect in the standard, but that's not my decision to make. Also note that other feature test macros reflect the standards wording as well and not whether the feature has been backported.

[MitalAshok]

I see that other features that have been backported only define the feature test macro if it didn't exist in the older versions. This sounds reasonable, I'll revert this change.

[AaronBallman]

I'm not certain I agree that this is non-conforming, but the standard could be made more clear. The name of the feature test macro has to be predefined, but the standard says nothing about the value beyond what the value is for that particular release of the standard. The whole point to feature test macros is to be able to use the resulting date to tell you which feature you're working with, so not being able to specify an updated value for backported functionality... kind of defeats the purpose.

[philnik777]

I'm not exactly sure what's ambiguous here. You say yourself that it specifies what the value should be for a given standards version. I also can't find anything suggesting that implementations are allowed to define then to something else. Anyways, since we seem to agree that implementations should be allowed to define them to some higher number, I'm not sure it makes a ton of sense to do anything other than filing an issue. But where should it be filed? Is that core wording or library? Both? Neither?

[philnik777]

Oh, also note that this could potentially be a huge breaking change, since users may rely on them guarding for a version.

[philnik777]

https://eel.is/c++draft/cpp.predefined has two styles of predefined macros. Ones where there is normative text explaining what the macro has to expand to, and others where it simply lists a value in a table. Nothing in that subclause says "shall be defined as" for the values in the table. It just says the names shall be defined.

Hmm, true. The library version seems less ambiguous, since there it's in the synopsis as #define __cpp_lib_whatever <some_number>L.

Probably one Core issue and one Library issue.

Sounds good. Who should file them?

Huh? The idiomatic use is #if defined(__cplusplus) && __cpp_whatever >= 201907L, so bumping to a later revision is not a breaking change. It's of course possible for a user to test using == or some other approach, but they're using the feature testing macro incorrectly at that point.

I'm more thinking of stuff like this: https://godbolt.org/z/qzaMaP4sW.

[MitalAshok]

I'm more thinking of stuff like this: https://godbolt.org/z/qzaMaP4sW.

That problem already exists for these macros:

llvm-project/clang/lib/Frontend/InitPreprocessor.cpp

Lines 758 to 767 in 4f067dc

	// We provide those C++23 features as extensions in earlier language modes, so
	// we also define their feature test macros.
	if (LangOpts.CPlusPlus11)
	Builder.defineMacro("__cpp_static_call_operator", "202207L");
	Builder.defineMacro("__cpp_named_character_escapes", "202207L");
	Builder.defineMacro("__cpp_placeholder_variables", "202306L");
	// C++26 features supported in earlier language modes.
	Builder.defineMacro("__cpp_pack_indexing", "202311L");
	Builder.defineMacro("__cpp_deleted_function", "202403L");

https://godbolt.org/z/nGqPb4Pvr

[cor3ntin]

We have had that discussion before. #77262

I think we should

• unconditionally set the value of __cpp_static_assert to 202306L in this patch
• Explore through an rfc or more detailed issue an holistic change of there is consensus that we should improve how pedantic errors and feature test macro interact.

[philnik777]

@MitalAshok That doesn't mean that it won't break people though. Maybe not in this specific case, but almost certainly in general.

[AaronBallman]

I agree with @cor3ntin -- pedantic errors and feature testing are a wider concern; in addition to #77262 we also have #84372 which is a similar problem but with __has_extension and pedantic errors.

In your example, the user is getting precisely what they asked for and I don't see the issue. They explicitly opt in to diagnostics about code that uses extensions in C++14 mode, so the diagnostic warning is expected. And they explicitly opt to make all warnings be errors, so upgrading the warning to an error is expected.

[cor3ntin]

This seems wrong!

[MitalAshok]

Fixed, thanks!

[AaronBallman]

I think this LGTM now, thank you!

[yashssh]

Reigniting the discussion around __cpp_static_assert's value set to 202306L . We are seeing some failures in perennial c++20 tests who assume the value of macro to be 201411L which is consistent with how GCC defines value of these macros (see https://godbolt.org/z/xE9bWsdKT)

From the discussions earlier I'm getting the impression that this has more to do with c++ standard not properly defining the values of these macros for older revisions. What can be the best way forward to fix this? Have it consistent with GCC?

[yashssh]

https://isocpp.org/files/papers/N4860.pdf according to this, cpp_static_assert should be 201411L for c++20

[erichkeane]

@yashssh : We can take this offline (see slack), but Feature Test Macros are intended to change. The values listed in the standard are the 'minimum' value for a compiler that implemented every feature in the standard.

The expectation is that an implementation will use a number greater than that when implementing/enabling features from future standards. Since Clang is permitting this version of static_assert as an extension, it should be expected that the FTM is a value that would match.

In this case, the Perennial test has an incorrect assumption, and, comparing a FTM with an equality comparison is actually contrary to how it is intended to be used(it should be >=, since the point is, "did you implement this one component of this feature that I want, or better?").