[clang] Remove separate evaluation step for static class member init. #142713

efriedma-quic · 2025-06-04T02:32:48Z

We already evaluate the initializers for all global variables, as required by the standard. Leverage that evaluation instead of trying to separately validate static class members.

This has a few benefits:

Improved diagnostics; we now get notes explaining what failed to evaluate.
Improved correctness: is_constant_evaluated is handled correctly.

The behavior follows the proposed resolution for CWG1721.

Fixes #88462. Fixes #99680.

We already evaluate the initializers for all global variables, as required by the standard. Leverage that evaluation instead of trying to separately validate static class members. This has a few benefits: - Improved diagnostics; we now get notes explaining what failed to evaluate. - Improved correctness: is_constant_evaluated is handled correctly. Fixes llvm#88462. Fixes llvm#99680.

llvmbot · 2025-06-04T02:33:26Z

@llvm/pr-subscribers-clang

Author: Eli Friedman (efriedma-quic)

Changes

We already evaluate the initializers for all global variables, as required by the standard. Leverage that evaluation instead of trying to separately validate static class members.

This has a few benefits:

Improved diagnostics; we now get notes explaining what failed to evaluate.
Improved correctness: is_constant_evaluated is handled correctly.

Fixes #88462. Fixes #99680.

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

6 Files Affected:

(modified) clang/lib/Sema/SemaDecl.cpp (+18-21)
(modified) clang/test/SemaCXX/builtin-is-constant-evaluated.cpp (+5)
(modified) clang/test/SemaCXX/class.cpp (+18-10)
(modified) clang/test/SemaCXX/cxx0x-class.cpp (+7-4)
(modified) clang/test/SemaCXX/cxx2a-consteval.cpp (+4-4)
(modified) clang/test/SemaTemplate/instantiate-static-var.cpp (+6-4)

diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 86b871396ec90..ab779f6d3c177 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -13980,31 +13980,10 @@ void Sema::AddInitializerToDecl(Decl *RealDecl, Expr *Init, bool DirectInit) {
 
     // We allow integer constant expressions in all cases.
     } else if (DclT->isIntegralOrEnumerationType()) {
-      // Check whether the expression is a constant expression.
-      SourceLocation Loc;
       if (getLangOpts().CPlusPlus11 && DclT.isVolatileQualified())
         // In C++11, a non-constexpr const static data member with an
         // in-class initializer cannot be volatile.
         Diag(VDecl->getLocation(), diag::err_in_class_initializer_volatile);
-      else if (Init->isValueDependent())
-        ; // Nothing to check.
-      else if (Init->isIntegerConstantExpr(Context, &Loc))
-        ; // Ok, it's an ICE!
-      else if (Init->getType()->isScopedEnumeralType() &&
-               Init->isCXX11ConstantExpr(Context))
-        ; // Ok, it is a scoped-enum constant expression.
-      else if (Init->isEvaluatable(Context)) {
-        // If we can constant fold the initializer through heroics, accept it,
-        // but report this as a use of an extension for -pedantic.
-        Diag(Loc, diag::ext_in_class_initializer_non_constant)
-          << Init->getSourceRange();
-      } else {
-        // Otherwise, this is some crazy unknown case.  Report the issue at the
-        // location provided by the isIntegerConstantExpr failed check.
-        Diag(Loc, diag::err_in_class_initializer_non_constant)
-          << Init->getSourceRange();
-        VDecl->setInvalidDecl();
-      }
 
     // We allow foldable floating-point constants as an extension.
     } else if (DclT->isFloatingType()) { // also permits complex, which is ok
@@ -14726,6 +14705,17 @@ void Sema::CheckCompleteVariableDeclaration(VarDecl *var) {
       // Compute and cache the constant value, and remember that we have a
       // constant initializer.
       if (HasConstInit) {
+        if (var->isStaticDataMember() && !var->isInline() &&
+            var->getLexicalDeclContext()->isRecord() &&
+            type->isIntegralOrEnumerationType()) {
+          // In C++98, in-class initialization for a static data member must
+          // be an integer constant expression.
+          SourceLocation Loc;
+          if (!Init->isIntegerConstantExpr(Context, &Loc)) {
+            Diag(Loc, diag::ext_in_class_initializer_non_constant)
+              << Init->getSourceRange();
+          }
+        }
         (void)var->checkForConstantInitialization(Notes);
         Notes.clear();
       } else if (CacheCulprit) {
@@ -14761,6 +14751,13 @@ void Sema::CheckCompleteVariableDeclaration(VarDecl *var) {
           << Attr->getRange() << Attr->isConstinit();
       for (auto &it : Notes)
         Diag(it.first, it.second);
+    } else if (var->isStaticDataMember() && !var->isInline() &&
+               var->getLexicalDeclContext()->isRecord()) {
+      Diag(var->getLocation(), diag::err_in_class_initializer_non_constant)
+        << Init->getSourceRange();
+      for (auto &it : Notes)
+        Diag(it.first, it.second);
+      var->setInvalidDecl();
     } else if (IsGlobal &&
                !getDiagnostics().isIgnored(diag::warn_global_constructor,
                                            var->getLocation())) {
diff --git a/clang/test/SemaCXX/builtin-is-constant-evaluated.cpp b/clang/test/SemaCXX/builtin-is-constant-evaluated.cpp
index a1c003c85f732..2ecbef3c1558e 100644
--- a/clang/test/SemaCXX/builtin-is-constant-evaluated.cpp
+++ b/clang/test/SemaCXX/builtin-is-constant-evaluated.cpp
@@ -143,3 +143,8 @@ namespace fold_initializer {
   const float A::f = __builtin_is_constant_evaluated();
   static_assert(fold(A::f == 1.0f));
 }
+
+struct GH99680 {
+  static const int x = 1/(1-__builtin_is_constant_evaluated()); // expected-error {{in-class initializer for static data member is not a constant expression}} \
+    // expected-note {{division by zero}}
+};
diff --git a/clang/test/SemaCXX/class.cpp b/clang/test/SemaCXX/class.cpp
index 2f59544e7f36c..f1e02d5158aac 100644
--- a/clang/test/SemaCXX/class.cpp
+++ b/clang/test/SemaCXX/class.cpp
@@ -1,5 +1,5 @@
 // RUN: %clang_cc1 -fsyntax-only -verify=expected,cxx11 -Wc++11-compat %s
-// RUN: %clang_cc1 -fsyntax-only -verify -Wc++11-compat %s -std=c++98
+// RUN: %clang_cc1 -fsyntax-only -verify=expected,cxx98 -Wc++11-compat %s -std=c++98
 class C {
 public:
   auto int errx; // expected-error {{storage class specified for a member declaration}}
@@ -32,7 +32,7 @@ class C {
   int : 1, : 2;
   typedef int E : 1; // expected-error {{typedef member 'E' cannot be a bit-field}}
   static int sb : 1; // expected-error {{static member 'sb' cannot be a bit-field}}
-  static int vs;
+  static int vs; // cxx11-note {{declared here}}
 
   typedef int func();
   func tm;
@@ -48,20 +48,28 @@ class C {
 #endif
   static int si = 0; // expected-error {{non-const static data member must be initialized out of line}}
   static const NestedC ci = 0; // expected-error {{static data member of type 'const NestedC' must be initialized out of line}}
-  static const int nci = vs; // expected-error {{in-class initializer for static data member is not a constant expression}}
+  static const int nci = vs; // expected-error {{in-class initializer for static data member is not a constant expression}} \
+  // cxx11-note {{read of non-const variable 'vs' is not allowed in a constant expression}} \
+  // cxx98-note {{subexpression not valid in a constant expression}}
   static const int vi = 0;
   static const volatile int cvi = 0; // ok, illegal in C++11
 #if __cplusplus >= 201103L
   // expected-error@-2 {{static const volatile data member must be initialized out of line}}
 #endif
   static const E evi = 0;
-  static const int overflow = 1000000*1000000; // cxx11-error {{in-class initializer for static data member is not a constant expression}}
-                                               // expected-warning@-1 {{overflow in expression}}
-  static const int overflow_shift = 1<<32; // cxx11-error {{in-class initializer for static data member is not a constant expression}}
-  static const int overflow_shift2 = 1>>32; // cxx11-error {{in-class initializer for static data member is not a constant expression}}
-  static const int overflow_shift3 = 1<<-1; // cxx11-error {{in-class initializer for static data member is not a constant expression}}
-  static const int overflow_shift4 = 1<<-1; // cxx11-error {{in-class initializer for static data member is not a constant expression}}
-  static const int overflow_shift5 = -1<<1; // cxx11-error {{in-class initializer for static data member is not a constant expression}}
+  static const int overflow = 1000000*1000000; // cxx11-error {{in-class initializer for static data member is not a constant expression}} \
+                                               // cxx11-note {{value 1000000000000 is outside the range of representable values of type 'int'}} \
+                                               // expected-warning {{overflow in expression}}
+  static const int overflow_shift = 1<<32; // cxx11-error {{in-class initializer for static data member is not a constant expression}} \
+                                           // cxx11-note {{shift count 32 >= width of type 'int' (32 bits)}}
+  static const int overflow_shift2 = 1>>32; // cxx11-error {{in-class initializer for static data member is not a constant expression}}\
+                                            // cxx11-note {{shift count 32 >= width of type 'int' (32 bits)}}
+  static const int overflow_shift3 = 1<<-1; // cxx11-error {{in-class initializer for static data member is not a constant expression}} \
+                                            // cxx11-note {{negative shift count -1}}
+  static const int overflow_shift4 = 1<<-1; // cxx11-error {{in-class initializer for static data member is not a constant expression}} \
+                                            // cxx11-note {{negative shift count -1}}
+  static const int overflow_shift5 = -1<<1; // cxx11-error {{in-class initializer for static data member is not a constant expression}} \
+                                            // cxx11-note {{left shift of negative value -1}}
 
   void m() {
     sx = 0;
diff --git a/clang/test/SemaCXX/cxx0x-class.cpp b/clang/test/SemaCXX/cxx0x-class.cpp
index a612a5c07e6ed..4b54221cceff2 100644
--- a/clang/test/SemaCXX/cxx0x-class.cpp
+++ b/clang/test/SemaCXX/cxx0x-class.cpp
@@ -1,6 +1,6 @@
 // RUN: %clang_cc1 -Wno-uninitialized -fsyntax-only -verify -std=c++11 -Wno-error=static-float-init %s 
 
-int vs = 0;
+int vs = 0; // expected-note {{declared here}}
 
 class C {
 public:
@@ -11,17 +11,20 @@ class C {
   int i = 0;
   static int si = 0; // expected-error {{non-const static data member must be initialized out of line}}
   static const NestedC ci = 0; // expected-error {{static data member of type 'const NestedC' must be initialized out of line}}
-  static const int nci = vs; // expected-error {{in-class initializer for static data member is not a constant expression}}
+  static const int nci = vs; // expected-error {{in-class initializer for static data member is not a constant expression}} \
+                             // expected-note {{read of non-const variable 'vs' is not allowed in a constant expression}}
   static const int vi = 0;
   static const volatile int cvi = 0; // expected-error {{static const volatile data member must be initialized out of line}}
 };
 
 namespace rdar8367341 {
-  float foo(); // expected-note {{here}}
+  float foo(); // expected-note 2 {{here}}
 
   struct A {
     static const float x = 5.0f; // expected-warning {{requires 'constexpr'}} expected-note {{add 'constexpr'}}
-    static const float y = foo(); // expected-warning {{requires 'constexpr'}} expected-note {{add 'constexpr'}}
+    static const float y = foo(); // expected-warning {{requires 'constexpr'}} expected-note {{add 'constexpr'}} \
+                                  // expected-error {{in-class initializer for static data member is not a constant expression}} \
+                                  // expected-note {{non-constexpr function 'foo' cannot be used in a constant expression}}
     static constexpr float x2 = 5.0f;
     static constexpr float y2 = foo(); // expected-error {{must be initialized by a constant expression}} expected-note {{non-constexpr function 'foo'}}
   };
diff --git a/clang/test/SemaCXX/cxx2a-consteval.cpp b/clang/test/SemaCXX/cxx2a-consteval.cpp
index d9932e4dd8241..1474c48cda3c1 100644
--- a/clang/test/SemaCXX/cxx2a-consteval.cpp
+++ b/clang/test/SemaCXX/cxx2a-consteval.cpp
@@ -1154,20 +1154,20 @@ namespace GH65985 {
 int consteval operator""_foo(unsigned long long V) {
     return 0;
 }
-int consteval operator""_bar(unsigned long long V); // expected-note 3{{here}}
+int consteval operator""_bar(unsigned long long V); // expected-note 4 {{here}}
 
 int consteval f() {
   return 0;
 }
 
-int consteval g();  // expected-note {{here}}
+int consteval g();  // expected-note 2 {{here}}
 
 
 struct C {
     static const int a = 1_foo;
     static constexpr int b = 1_foo;
     static const int c = 1_bar; // expected-error {{call to consteval function 'GH65985::operator""_bar' is not a constant expression}} \
-                                // expected-note {{undefined function 'operator""_bar' cannot be used in a constant expression}} \
+                                // expected-note 2 {{undefined function 'operator""_bar' cannot be used in a constant expression}} \
                                 // expected-error {{in-class initializer for static data member is not a constant expression}}
 
     // FIXME: remove duplicate diagnostics
@@ -1179,7 +1179,7 @@ struct C {
     static const int e = f();
     static const int f = g(); // expected-error {{call to consteval function 'GH65985::g' is not a constant expression}} \
                               // expected-error {{in-class initializer for static data member is not a constant expression}} \
-                              // expected-note  {{undefined function 'g' cannot be used in a constant expression}}
+                              // expected-note 2 {{undefined function 'g' cannot be used in a constant expression}}
 };
 
 }
diff --git a/clang/test/SemaTemplate/instantiate-static-var.cpp b/clang/test/SemaTemplate/instantiate-static-var.cpp
index 63d8366b617c1..6602670af901f 100644
--- a/clang/test/SemaTemplate/instantiate-static-var.cpp
+++ b/clang/test/SemaTemplate/instantiate-static-var.cpp
@@ -1,11 +1,13 @@
-// RUN: %clang_cc1 -fsyntax-only -verify %s
-// RUN: %clang_cc1 -fsyntax-only -verify -std=c++98 %s
-// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 %s
+// RUN: %clang_cc1 -fsyntax-only -verify=expected,cxx11 %s
+// RUN: %clang_cc1 -fsyntax-only -verify=expected,cxx98 -std=c++98 %s
+// RUN: %clang_cc1 -fsyntax-only -verify=expected,cxx11 -std=c++11 %s
 
 template<typename T, T Divisor>
 class X {
 public:
-  static const T value = 10 / Divisor; // expected-error{{in-class initializer for static data member is not a constant expression}}
+  static const T value = 10 / Divisor; // expected-error{{in-class initializer for static data member is not a constant expression}} \
+  // cxx11-note {{division by zero}} \
+  // cxx98-note {{subexpression not valid}}
 };
 
 int array1[X<int, 2>::value == 5? 1 : -1];

github-actions · 2025-06-04T02:35:51Z

✅ With the latest revision this PR passed the C/C++ code formatter.

tbaederr · 2025-06-04T05:06:37Z

Could you run this through the compile time tracker?

efriedma-quic · 2025-06-04T20:48:38Z

https://llvm-compile-time-tracker.com/compare.php?from=d204aa9deb72b8dcaf5e5b5550871d0ebe982825&to=9bc16d5c3b3c540ca3058e181b509f4122a2073b&stat=instructions:u . Basically no change... which is what I was expecting. There's maybe some slight savings from skipping the call to isIntegerConstantExpr().

shafik

So just to confirm my reading, this is benefiting from CheckCompleteVariableDeclaration doing:

  // Evaluate the initializer to see if it's a constant initializer.
  HasConstInit = var->checkForConstantInitialization(Notes);

or is there more?

shafik

I think some of these examples are covered in the test suite but I am not sure if all are. Can you confirm this is consistent with: https://eel.is/c++draft/expr.const#28.5

shafik · 2025-06-05T19:42:30Z

clang/test/SemaCXX/builtin-is-constant-evaluated.cpp

+
+struct GH99680 {
+  static const int x = 1/(1-__builtin_is_constant_evaluated()); // expected-error {{in-class initializer for static data member is not a constant expression}} \
+    // expected-note {{division by zero}}


Based on the example here: https://eel.is/c++draft/expr.const#28.5

I don't think the division by zero diagnostic makes sense.

This example is illegal for the same reason you can't write int f(); struct X { static const int x = f(); };. Normally static data members aren't allowed to have initializers at all if they're not inline. But there's a very narrow carveout in [class.static.data]p4: if the type is a const non-volatile integer type, it's allowed if the initializer is constant expression.

So if this is constant evaluated we have UB (divide by zero) so then it can't be constant evaluated and therefore __builtin_is_constant_evaluated() must be zero and therefore no divide by zero.

Which is the same logic used in the example in [expr.const]p28.5

@shafik I think you're right based on the current standard wording, but that this is a bug in the wording of the standard. The situation described in [expr.const]/28.5's example is a fallback from constant evaluation of the initializer to runtime evaluation, and that fallback shouldn't be possible for a static const data member. Will ask CWG.

Note that the rules also say that x is not usable in constant expressions and has dynamic initialization in this case. This really looks like a wording oversight :)

Oh, I see what you mean. the variable doesn't have constant initialization, so it's not manifestly constant-evaluated... but it's still technically a constant expression. We could implement this by guarding the diagnostic with an isCXX11ConstantExpr check.

That said, I don't want to think about how to generate an understandable diagnostic for static const int x = 1/(1-__builtin_is_constant_evaluated()) + 1/__builtin_is_constant_evaluated();.

I also think this outcome is pretty surprising: https://godbolt.org/z/cobb3Px5Y
The new behavior seems like what we ought to be doing, even if the old behavior is the one the rules appear to require. All other implementations seem to be checking whether the variable is constant-initialized, not whether its initializer is a constant expression.

Aha, the resolution for CWG1721 (not yet resolved but in "review" state, which means CWG thinks the wording is ready for final review before resolution) changes the rule to check whether the variable is constant-initialized (thanks @t3nsor for pointing this out!). So this PR is implementing the proposed resolution of that issue.

Okay, updated the commit message with a reference to CWG1721.

efriedma-quic

or is there more?

No, that's it, basically. (Except for the C++98 codepath.)

efriedma-quic · 2025-06-05T20:42:40Z

clang/test/SemaCXX/builtin-is-constant-evaluated.cpp

+
+struct GH99680 {
+  static const int x = 1/(1-__builtin_is_constant_evaluated()); // expected-error {{in-class initializer for static data member is not a constant expression}} \
+    // expected-note {{division by zero}}


This example is illegal for the same reason you can't write int f(); struct X { static const int x = f(); };. Normally static data members aren't allowed to have initializers at all if they're not inline. But there's a very narrow carveout in [class.static.data]p4: if the type is a const non-volatile integer type, it's allowed if the initializer is constant expression.

efriedma-quic · 2025-06-17T23:17:21Z

Ping

zygoloid

LGTM

efriedma-quic requested review from cor3ntin and shafik June 4, 2025 02:32

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

Fix formatting.

3ddce15

shafik reviewed Jun 5, 2025

View reviewed changes

efriedma-quic commented Jun 5, 2025

View reviewed changes

efriedma-quic added 2 commits June 5, 2025 14:10

Merge remote-tracking branch 'origin/main' into static-member-init-eval

39e6548

Add a few more tests.

8573eb1

zygoloid approved these changes Jun 17, 2025

View reviewed changes

efriedma-quic merged commit f2d2c99 into llvm:main Jun 17, 2025
7 checks passed

[clang] Remove separate evaluation step for static class member init. #142713

[clang] Remove separate evaluation step for static class member init. #142713

Conversation

efriedma-quic commented Jun 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Jun 4, 2025

Uh oh!

github-actions bot commented Jun 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tbaederr commented Jun 4, 2025

Uh oh!

efriedma-quic commented Jun 4, 2025

Uh oh!

shafik left a comment

Choose a reason for hiding this comment

Uh oh!

shafik left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

efriedma-quic left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

efriedma-quic commented Jun 17, 2025

Uh oh!

zygoloid left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

efriedma-quic commented Jun 4, 2025 •

edited

Loading

github-actions bot commented Jun 4, 2025 •

edited

Loading