[Clang] Fix P2564 handling of variable initializers #89565

katzdm · 2024-04-22T02:48:09Z

The following program produces a diagnostic in Clang and EDG, but compiles correctly in GCC and MSVC:

#include <vector>

consteval std::vector<int> fn() { return {1,2,3}; }
constexpr int a = fn()[1];

Clang's diagnostic is as follows:

<source>:6:19: error: call to consteval function 'fn' is not a constant expression
    6 | constexpr int a = fn()[1];
      |                   ^
<source>:6:19: note: pointer to subobject of heap-allocated object is not a constant expression
/opt/compiler-explorer/gcc-snapshot/lib/gcc/x86_64-linux-gnu/14.0.1/../../../../include/c++/14.0.1/bits/allocator.h:193:31: note: heap allocation performed here
  193 |             return static_cast<_Tp*>(::operator new(__n));
      |                                      ^
1 error generated.
Compiler returned: 1

Based on my understanding of [dcl.constexpr]/6:

In any constexpr variable declaration, the full-expression of the initialization shall be a constant expression

It seems to me that GCC and MSVC are correct: the initializer fn()[1] does not evaluate to an lvalue referencing a heap-allocated value within the vector returned by fn(); it evaluates to an lvalue-to-rvalue conversion from that heap-allocated value.

This PR turns out to be a bug fix on the implementation of P2564R3; as such, it only applies to C++23 and later. The core problem is that the definition of a constant-initialized variable ([expr.const/2]) is contingent on whether the initializer can be evaluated as a constant expression:

A variable or temporary object o is constant-initialized if [...] the full-expression of its initialization is a constant expression when interpreted as a constant-expression, [...]

That can't be known until we've finished parsing the initializer, by which time we've already added immediate invocations and consteval references to the current expression evaluation context. This will have the effect of evaluating said invocations as full expressions when the context is popped, even if they're subexpressions of a larger constant expression initializer. If, however, the variable is constant-initialized, then its initializer is manifestly constant-evaluated:

An expression or conversion is manifestly constant-evaluated if it is [...] the initializer of a variable that is usable in constant expressions or has constant initialization [...]

which in turn means that any subexpressions naming an immediate function are in an immediate function context:

An expression or conversion is in an immediate function context if it is potentially evaluated and either [...] it is a subexpression of a manifestly constant-evaluated expression or conversion

and therefore are not to be considered immediate invocations or immediate-escalating expressions in the first place:

An invocation is an immediate invocation if it is a potentially-evaluated explicit or implicit invocation of an immediate function and is not in an immediate function context.

An expression or conversion is immediate-escalating if it is not initially in an immediate function context and [...]

The approach that I'm therefore proposing is:

Create a new expression evaluation context for every variable initializer (rather than only nonlocal ones).
Attach initializers to VarDecls prior to popping the expression evaluation context / scope / etc. This sequences the determination of whether the initializer is in an immediate function context before any contained immediate invocations are evaluated.
When popping an expression evaluation context, elide all evaluations of constant invocations, and all checks for consteval references, if the context is an immediate function context. Note that if it could be ascertained that this was an immediate function context at parse-time, we would never have registered these immediate invocations or consteval references in the first place.

Most of the test changes previously made for this PR are now reverted and passing as-is. The only test updates needed are now as follows:

A few diagnostics in consteval-cxx2a.cpp are updated to reflect that it is the consteval tester::tester constructor, not the more narrow make_name function call, which fails to be evaluated as a constant expression.
The reclassification of warn_impcast_integer_precision_constant as a compile-time diagnostic adds a (somewhat duplicative) warning when attempting to define an enum constant using a narrowing conversion. It also, however, retains the existing diagnostics which @erichkeane (rightly) objected to being lost from an earlier revision of this PR.

llvmbot · 2024-04-22T02:48:40Z

@llvm/pr-subscribers-clang

Author: Daniel M. Katz (katzdm)

Changes

The following program produces a diagnostic in Clang and EDG, but compiles correctly in GCC and MSVC:

#include &lt;vector&gt;

consteval std::vector&lt;int&gt; fn() { return {1,2,3}; }
constexpr int a = fn()[1];

Clang's diagnostic is as follows:

&lt;source&gt;:6:19: error: call to consteval function 'fn' is not a constant expression
    6 | constexpr int a = fn()[1];
      |                   ^
&lt;source&gt;:6:19: note: pointer to subobject of heap-allocated object is not a constant expression
/opt/compiler-explorer/gcc-snapshot/lib/gcc/x86_64-linux-gnu/14.0.1/../../../../include/c++/14.0.1/bits/allocator.h:193:31: note: heap allocation performed here
  193 |             return static_cast&lt;_Tp*&gt;(::operator new(__n));
      |                                      ^
1 error generated.
Compiler returned: 1

Based on my understanding of [[dcl.constexpr]/6](https://eel.is/c++draft/dcl.constexpr#6):
> In any constexpr variable declaration, the full-expression of the initialization shall be a constant expression

It seems to me that GCC and MSVC are correct: the initializer fn()[1] does not evaluate to an lvalue referencing a heap-allocated value within the vector returned by fn(); it evaluates to an lvalue-to-rvalue conversion from that heap-allocated value.

A dump of the AST for the variable declaration elucidates what I believe to be a problem:

`-VarDecl &lt;line:6:1, col:25&gt; col:15 a 'const int' constexpr cinit
  |-value: Int 2
  `-ExprWithCleanups &lt;col:19, col:25&gt; 'value_type':'int'
    `-ImplicitCastExpr &lt;col:19, col:25&gt; 'value_type':'int' &lt;LValueToRValue&gt;
      `-CXXOperatorCallExpr &lt;col:19, col:25&gt; 'value_type':'int' lvalue '[]'
        |-ImplicitCastExpr &lt;col:23, col:25&gt; 'reference (*)(size_type) noexcept' &lt;FunctionToPointerDecay&gt;
        | `-DeclRefExpr &lt;col:23, col:25&gt; 'reference (size_type) noexcept' lvalue CXXMethod 0xd1497a0 'operator[]' 'reference (size_type) noexcept'
        |-MaterializeTemporaryExpr &lt;col:19, col:22&gt; 'std::vector&lt;int&gt;' lvalue
        | `-ConstantExpr &lt;col:19, col:22&gt; 'std::vector&lt;int&gt;'
        |   `-ExprWithCleanups &lt;col:19, col:22&gt; 'std::vector&lt;int&gt;'
        |     `-CXXBindTemporaryExpr &lt;col:19, col:22&gt; 'std::vector&lt;int&gt;' (CXXTemporary 0xd180de8)
        |       `-CallExpr &lt;col:19, col:22&gt; 'std::vector&lt;int&gt;'
        |         `-ImplicitCastExpr &lt;col:19&gt; 'std::vector&lt;int&gt; (*)()' &lt;FunctionToPointerDecay&gt;
        |           `-DeclRefExpr &lt;col:19&gt; 'std::vector&lt;int&gt; ()' lvalue Function 0xd12e7b8 'fn' 'std::vector&lt;int&gt; ()'
        `-ImplicitCastExpr &lt;col:24&gt; 'size_type':'unsigned long' &lt;IntegralCast&gt;
          `-IntegerLiteral &lt;col:24&gt; 'int' 1

There is a ConstantExpr wrapping the CallExpr to fn, causing it to be evaluated independently as a constant expression rather than evaluated as a component of the larger LValueToRValue-ImplicitCastExpr. Because the initializer of any constexpr variable declaration is itself a constant expression (as cited above), I believe this is incorrect.

The ConstantExpr wrapper is created because fn() is an immediate function call within an evaluation context that Clang classifies as PotentiallyEvaluated rather than ConstantEvaluated. Setting the Sema::isConstantEvaluatedOverride flag during evaluation of an initializer seems to address the problem.

I added a test to clang/test/SemaCXX/cxx2a-consteval.cpp to cover this case, and "fixed" the few tests that fail in response to it. One such test was documented by a FIXME that this change appears to fix.

Most of the other changes to tests consist of removing annotations expecting diagnostics for the issue being fixed here - from a quick spot check, nothing jumped out as "hey, that looks like it really should be an error", but another set of eyes would be helpful here.

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

5 Files Affected:

(modified) clang/lib/Parse/ParseDecl.cpp (+11-1)
(modified) clang/test/CXX/expr/expr.const/p6-2a.cpp (+3-4)
(modified) clang/test/SemaCXX/builtin_vectorelements.cpp (+2-2)
(modified) clang/test/SemaCXX/cxx2a-consteval.cpp (+20-12)
(modified) clang/unittests/Support/TimeProfilerTest.cpp (-1)

diff --git a/clang/lib/Parse/ParseDecl.cpp b/clang/lib/Parse/ParseDecl.cpp
index 583232f2d610d0..0ea6fccaa7eb34 100644
--- a/clang/lib/Parse/ParseDecl.cpp
+++ b/clang/lib/Parse/ParseDecl.cpp
@@ -2554,6 +2554,13 @@ Decl *Parser::ParseDeclarationAfterDeclarator(
   return ParseDeclarationAfterDeclaratorAndAttributes(D, TemplateInfo);
 }
 
+static bool isConstexprVariable(const Decl *D) {
+  if (const VarDecl *Var = dyn_cast_or_null<VarDecl>(D))
+    return Var->isConstexpr();
+
+  return false;
+}
+
 Decl *Parser::ParseDeclarationAfterDeclaratorAndAttributes(
     Declarator &D, const ParsedTemplateInfo &TemplateInfo, ForRangeInit *FRI) {
   // RAII type used to track whether we're inside an initializer.
@@ -2561,9 +2568,12 @@ Decl *Parser::ParseDeclarationAfterDeclaratorAndAttributes(
     Parser &P;
     Declarator &D;
     Decl *ThisDecl;
+    llvm::SaveAndRestore<bool> ConstantContext;
 
     InitializerScopeRAII(Parser &P, Declarator &D, Decl *ThisDecl)
-        : P(P), D(D), ThisDecl(ThisDecl) {
+        : P(P), D(D), ThisDecl(ThisDecl),
+          ConstantContext(P.Actions.isConstantEvaluatedOverride,
+                          isConstexprVariable(ThisDecl)) {
       if (ThisDecl && P.getLangOpts().CPlusPlus) {
         Scope *S = nullptr;
         if (D.getCXXScopeSpec().isSet()) {
diff --git a/clang/test/CXX/expr/expr.const/p6-2a.cpp b/clang/test/CXX/expr/expr.const/p6-2a.cpp
index a937474d53b221..8b940b2ee9fb2a 100644
--- a/clang/test/CXX/expr/expr.const/p6-2a.cpp
+++ b/clang/test/CXX/expr/expr.const/p6-2a.cpp
@@ -43,12 +43,11 @@ struct Temporary {
 constexpr Temporary t = {3}; // expected-error {{must have constant destruction}} expected-note {{created here}} expected-note {{in call}}
 
 namespace P1073R3 {
-consteval int f() { return 42; } // expected-note 2 {{declared here}}
+consteval int f() { return 42; } // expected-note {{declared here}}
 consteval auto g() { return f; }
 consteval int h(int (*p)() = g()) { return p(); }
 constexpr int r = h();
-constexpr auto e = g();  // expected-error {{call to consteval function 'P1073R3::g' is not a constant expression}} \
-                            expected-error {{constexpr variable 'e' must be initialized by a constant expression}} \
-                            expected-note 2 {{pointer to a consteval declaration is not a constant expression}}
+constexpr auto e = g();  // expected-error {{constexpr variable 'e' must be initialized by a constant expression}} \
+                            expected-note {{pointer to a consteval declaration is not a constant expression}}
 static_assert(r == 42);
 } // namespace P1073R3
diff --git a/clang/test/SemaCXX/builtin_vectorelements.cpp b/clang/test/SemaCXX/builtin_vectorelements.cpp
index 59ff09ac72e42d..12f2cbe57bd47d 100644
--- a/clang/test/SemaCXX/builtin_vectorelements.cpp
+++ b/clang/test/SemaCXX/builtin_vectorelements.cpp
@@ -42,11 +42,11 @@ void test_builtin_vectorelements() {
 #include <arm_sve.h>
 
 consteval int consteval_elements() { // expected-error {{consteval function never produces a constant expression}}
-  return __builtin_vectorelements(svuint64_t); // expected-note {{cannot determine number of elements for sizeless vectors in a constant expression}}  // expected-note {{cannot determine number of elements for sizeless vectors in a constant expression}} // expected-note {{cannot determine number of elements for sizeless vectors in a constant expression}}
+  return __builtin_vectorelements(svuint64_t); // expected-note {{cannot determine number of elements for sizeless vectors in a constant expression}} // expected-note {{cannot determine number of elements for sizeless vectors in a constant expression}}
 }
 
 void test_bad_constexpr() {
-  constexpr int eval = consteval_elements(); // expected-error {{initialized by a constant expression}} // expected-error {{not a constant expression}} // expected-note {{in call}} // expected-note {{in call}}
+  constexpr int eval = consteval_elements(); // expected-error {{initialized by a constant expression}} // expected-note {{in call}}
   constexpr int i32 = __builtin_vectorelements(svuint32_t); // expected-error {{initialized by a constant expression}} // expected-note {{cannot determine number of elements for sizeless vectors in a constant expression}}
   constexpr int i16p8 = __builtin_vectorelements(svuint16_t) + 16; // expected-error {{initialized by a constant expression}} // expected-note {{cannot determine number of elements for sizeless vectors in a constant expression}}
   constexpr int lambda = [] { return __builtin_vectorelements(svuint16_t); }(); // expected-error {{initialized by a constant expression}} // expected-note {{cannot determine number of elements for sizeless vectors in a constant expression}} // expected-note {{in call}}
diff --git a/clang/test/SemaCXX/cxx2a-consteval.cpp b/clang/test/SemaCXX/cxx2a-consteval.cpp
index 192621225a543c..d1f3b094f8afd5 100644
--- a/clang/test/SemaCXX/cxx2a-consteval.cpp
+++ b/clang/test/SemaCXX/cxx2a-consteval.cpp
@@ -260,6 +260,18 @@ int(*test)(int)  = l1;
 
 }
 
+namespace lvalue_to_rvalue_init_from_heap {
+
+struct S {
+    int *value;
+    constexpr S(int v) : value(new int {v}) {}
+    constexpr ~S() { delete value; }
+};
+consteval S fn() { return S(5); }
+
+constexpr int a = *fn().value;
+}
+
 namespace std {
 
 template <typename T> struct remove_reference { using type = T; };
@@ -713,7 +725,7 @@ constexpr derp d;
 struct test {
   consteval int operator[](int i) const { return {}; }
   consteval const derp * operator->() const { return &d; }
-  consteval int f() const { return 12; } // expected-note 2{{declared here}}
+  consteval int f() const { return 12; } // expected-note {{declared here}}
 };
 
 constexpr test a;
@@ -726,8 +738,7 @@ constexpr int s = a.operator[](1);
 constexpr int t = a[1];
 constexpr int u = a.operator->()->b;
 constexpr int v = a->b;
-// FIXME: I believe this case should work, but we currently reject.
-constexpr int w = (a.*&test::f)(); // expected-error {{cannot take address of consteval function 'f' outside of an immediate invocation}}
+constexpr int w = (a.*&test::f)();
 constexpr int x = a.f();
 
 // Show that we reject when not in an immediate context.
@@ -1032,17 +1043,15 @@ int f() {
 
 namespace GH57682 {
 void test() {
-  constexpr auto l1 = []() consteval { // expected-error {{cannot take address of consteval call operator of '(lambda at}} \
-                                       // expected-note  2{{declared here}}
+  constexpr auto l1 = []() consteval { // expected-note  {{declared here}}
         return 3;
   };
   constexpr int (*f1)(void) = l1; // expected-error {{constexpr variable 'f1' must be initialized by a constant expression}} \
                                   // expected-note  {{pointer to a consteval declaration is not a constant expression}}
 
 
-  constexpr auto lstatic = []() static consteval { // expected-error {{cannot take address of consteval call operator of '(lambda at}} \
-                                       // expected-note  2{{declared here}} \
-                                       // expected-warning {{extension}}
+  constexpr auto lstatic = []() static consteval { // expected-note  {{declared here}} \
+                                                   // expected-warning {{extension}}
         return 3;
   };
   constexpr int (*f2)(void) = lstatic; // expected-error {{constexpr variable 'f2' must be initialized by a constant expression}} \
@@ -1073,7 +1082,7 @@ struct tester {
 consteval const char* make_name(const char* name) { return name;}
 consteval const char* pad(int P) { return "thestring"; }
 
-int bad = 10; // expected-note 6{{declared here}}
+int bad = 10; // expected-note 5{{declared here}}
 
 tester glob1(make_name("glob1"));
 tester glob2(make_name("glob2"));
@@ -1082,9 +1091,8 @@ tester paddedglob(make_name(pad(bad))); // expected-error {{call to consteval fu
                                         // expected-note {{read of non-const variable 'bad' is not allowed in a constant expression}}
 
 constexpr tester glob3 = { make_name("glob3") };
-constexpr tester glob4 = { make_name(pad(bad)) }; // expected-error {{call to consteval function 'GH58207::make_name' is not a constant expression}} \
-                                                  // expected-error {{constexpr variable 'glob4' must be initialized by a constant expression}} \
-                                                  // expected-note 2{{read of non-const variable 'bad' is not allowed in a constant expression}}
+constexpr tester glob4 = { make_name(pad(bad)) }; // expected-error {{constexpr variable 'glob4' must be initialized by a constant expression}} \
+                                                  // expected-note 1{{read of non-const variable 'bad' is not allowed in a constant expression}}
 
 auto V = make_name(pad(3));
 auto V1 = make_name(pad(bad)); // expected-error {{call to consteval function 'GH58207::make_name' is not a constant expression}} \
diff --git a/clang/unittests/Support/TimeProfilerTest.cpp b/clang/unittests/Support/TimeProfilerTest.cpp
index 97fdbb7232b135..9c1a955743f1ee 100644
--- a/clang/unittests/Support/TimeProfilerTest.cpp
+++ b/clang/unittests/Support/TimeProfilerTest.cpp
@@ -193,7 +193,6 @@ Frontend
 | ParseDeclarationOrFunctionDefinition (test.cc:16:1)
 | | ParseFunctionDefinition (slow_test)
 | | | EvaluateAsInitializer (slow_value)
-| | | EvaluateAsConstantExpr (<test.cc:17:33, col:59>)
 | | | EvaluateAsConstantExpr (<test.cc:18:11, col:37>)
 | ParseDeclarationOrFunctionDefinition (test.cc:22:1)
 | | EvaluateAsConstantExpr (<test.cc:23:31, col:57>)

cor3ntin · 2024-04-22T08:47:35Z

I think the change makes sense.
Can you add a release note?

clang/lib/Parse/ParseDecl.cpp

clang/test/SemaCXX/cxx2a-consteval.cpp

katzdm · 2024-04-22T12:53:03Z

I think the change makes sense. Can you add a release note?

Appreciate the reminder!

katzdm

@cor3ntin Thanks for your review; this is ready for another look.

Note that the use of a full ExpressionEvaluationContext caused a handful of other tests to start failing - in particular, ones that previously expected warnings related to changes of value from implicit casts. I verified that these warnings are all emitted through Sema::DiagRuntimeBehavior, which no-ops when in a ConstantEvaluated context - so the lack of warning seems expected since the initializer is now treated as a constant expression.

clang/lib/Parse/ParseDecl.cpp

clang/test/SemaCXX/cxx2a-consteval.cpp

erichkeane

I don't have a great idea on this one, but losing the diagnostics seems incorrect. I think the whole initializer should probably be treated in 1 shot as a constant expr (so move the ConstantExpr to the top (above the ImplicitCastExpr in your example), but we DEFINITELY should be getting some of those diagnostics.

clang/test/AST/Interp/intap.cpp

clang/test/CXX/expr/expr.const/p2-0x.cpp

clang/lib/Parse/ParseDecl.cpp

Co-authored-by: cor3ntin <corentinjabot@gmail.com>

katzdm · 2024-04-26T18:28:00Z

This ended up being a lot more subtle than I expected. My latest push proposes a different solution; @cor3ntin and @efriedma-quic - please let me know your thoughts.

This PR turns out to be a bug fix on the implementation of P2564R3; as such, it only applies to C++23 and later. The core problem is that the definition of a constant-initialized variable ([expr.const/2]) is contingent on whether the initializer can be evaluated as a constant expression:

A variable or temporary object o is constant-initialized if [...] the full-expression of its initialization is a constant expression when interpreted as a constant-expression, [...]

That can't be known until we've finished parsing the initializer, by which time we've already added immediate invocations and consteval references to the current expression evaluation context. This will have the effect of evaluating said invocations as full expressions when the context is popped, even if they're subexpressions of a larger constant expression initializer. If, however, the variable is constant-initialized, then its initializer is manifestly constant-evaluated:

An expression or conversion is manifestly constant-evaluated if it is [...] the initializer of a variable that is usable in constant expressions or has constant initialization [...]

which in turn means that any subexpressions naming an immediate function are in an immediate function context:

An expression or conversion is in an immediate function context if it is potentially evaluated and either [...] it is a subexpression of a manifestly constant-evaluated expression or conversion

and therefore are not to be considered immediate invocations or immediate-escalating expressions in the first place:

An invocation is an immediate invocation if it is a potentially-evaluated explicit or implicit invocation of an immediate function and is not in an immediate function context.

An expression or conversion is immediate-escalating if it is not initially in an immediate function context and [...]

The approach that I'm therefore proposing is:

Create a new expression evaluation context for every variable initializer (rather than only nonlocal ones).
Attach initializers to VarDecls prior to popping the expression evaluation context / scope / etc. This sequences the determination of whether the initializer is in an immediate function context before any contained immediate invocations are evaluated.
When popping an expression evaluation context, elide all evaluations of constant invocations, and all checks for consteval references, if the context is an immediate function context. Note that if it could be ascertained that this was an immediate function context at parse-time, we would never have registered these immediate invocations or consteval references in the first place.

Most of the test changes previously made for this PR are now reverted and passing as-is. The only test updates needed are now as follows:

A few diagnostics in consteval-cxx2a.cpp are updated to reflect that it is the consteval tester::tester constructor, not the more narrow make_name function call, which fails to be evaluated as a constant expression.
The reclassification of warn_impcast_integer_precision_constant as a compile-time diagnostic adds a (somewhat duplicative) warning when attempting to define an enum constant using a narrowing conversion. It also, however, retains the existing diagnostics which @erichkeane (rightly) objected to being lost from an earlier revision of this PR.

Sorry for the long-winded explanation of a relatively small change - the language in [expr.const] is (at least I find) quite involved, so I figured more detail couldn't hurt.

github-actions · 2024-04-26T18:30:47Z

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

efriedma-quic · 2024-04-26T19:37:50Z

The approach makes sense. I'll defer to someone who's more familiar with this code for the full review.

cor3ntin · 2024-04-29T12:21:53Z

clang/lib/Parse/ParseDecl.cpp

      }
    }
-    ~InitializerScopeRAII() { pop(); }
-    void pop() {
+    ~InitializerScopeRAII() {


This is a nice improvement

clang/lib/Sema/SemaChecking.cpp

clang/lib/Sema/SemaDeclCXX.cpp

katzdm · 2024-05-06T13:50:10Z

👋 Friendly ping - No worries if more time is needed to consider the impact of this one, but wanted to check if any more changes are needed at this time.

erichkeane

This seems reasonable to me, I think @cor3ntin should also approve before committing though.

cor3ntin

LGTM too

Endilll

Sema.h changes look good.

cor3ntin · 2024-05-09T07:21:56Z

CI failures look unrelated

Fznamznon · 2024-05-16T09:28:07Z

@katzdm
It seems after this a warn_impcast_integer_precision_constant warning went missing:

https://godbolt.org/z/ndsPb44b4

Is that expected?

katzdm · 2024-05-16T15:37:42Z

@katzdm It seems after this a warn_impcast_integer_precision_constant warning went missing:

https://godbolt.org/z/ndsPb44b4

Is that expected?

We changed this diagnostic from a "runtime behavior" to "any time" check, but not this one - I'm guessing we don't have any tests to cover the later case.

Seems like we'd probably want to retain this warning, too - I'll throw a change together, see if it breaks any other tests, and see what reviewers have to say. Thanks for calling this out.

katzdm · 2024-05-16T19:01:29Z

@Fznamznon Hmm...my suggested change does seem to break some tests :( But to circle back to your original question, I think it's not totally unexpected - a broader class of variable initializers are now (correctly afaict) evaluated as constant expressions, which could make some warnings marked as "runtime behavior" no longer apply. I agree though that the result is unfortunate - @erichkeane had pushed back on some other warnings similarly getting swallowed during review.

I wonder if we're taking the wrong approach here - we could also leave both of these warnings as "runtime only", and separately try to diagnose during constant evaluation. If that could work, it might make it easier to e.g., only diagnose overflow in a branch of a compile-time conditional if it's actually evaluated (which GCC seems to manage to do).

efriedma-quic · 2024-05-16T20:35:17Z

clang already generates certain diagnostics from ExprConstant; expanding the set of diagnostics could be reasonable.

Alternatively, we could try to add some sort of cooperation between DiagRuntimeBehavior and constant evaluation, to try to avoid having to diagnose everything twice: basically, Sema makes a list of constant-evaluated expressions it wants to warn about, then if ExprConstant prints a warning if it sees one of those expressions.

Fznamznon · 2024-05-17T07:58:36Z

@katzdm
Does it make sense to file an issue so we don't forget to bring the warning back?

katzdm · 2024-05-18T15:50:58Z

@katzdm Does it make sense to file an issue so we don't forget to bring the warning back?

Yep, for sure - I've opened #92656.

llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Apr 22, 2024

cor3ntin reviewed Apr 22, 2024

View reviewed changes

clang/lib/Parse/ParseDecl.cpp Outdated Show resolved Hide resolved

clang/lib/Parse/ParseDecl.cpp Outdated Show resolved Hide resolved

clang/test/SemaCXX/cxx2a-consteval.cpp Outdated Show resolved Hide resolved

katzdm requested a review from tbaederr as a code owner April 22, 2024 17:28

katzdm commented Apr 22, 2024

View reviewed changes

clang/lib/Parse/ParseDecl.cpp Outdated Show resolved Hide resolved

clang/lib/Parse/ParseDecl.cpp Outdated Show resolved Hide resolved

clang/test/SemaCXX/cxx2a-consteval.cpp Outdated Show resolved Hide resolved

cor3ntin requested review from AaronBallman and erichkeane April 23, 2024 06:29

erichkeane reviewed Apr 23, 2024

View reviewed changes

clang/test/AST/Interp/intap.cpp Outdated Show resolved Hide resolved

clang/test/CXX/expr/expr.const/p2-0x.cpp Outdated Show resolved Hide resolved

efriedma-quic reviewed Apr 23, 2024

View reviewed changes

clang/lib/Parse/ParseDecl.cpp Outdated Show resolved Hide resolved

efriedma-quic mentioned this pull request Apr 26, 2024

Clang rejects valid program involving evaluation of this in constexpr context #90052

Open

katzdm and others added 6 commits April 26, 2024 13:27

Fix bug with constexpr initialization.

218ffb6

Use dyn_cast without checking for null

8cd385f

Co-authored-by: cor3ntin <corentinjabot@gmail.com>

Add release notes.

76ca0e0

Re-introduce null check. Use EvaluationContext instead of override.

5026863

Adjust other tests newly failing from change.

9f98e40

Addressing feedback. Redesigning proposed solution.

ec17c76

katzdm force-pushed the constexpr-init-fix branch from cf4ecf0 to ec17c76 Compare April 26, 2024 18:27

katzdm requested a review from Endilll as a code owner April 26, 2024 18:27

Formatting fixes.

c58a324

katzdm force-pushed the constexpr-init-fix branch from 2d577e9 to c58a324 Compare April 26, 2024 18:41

cor3ntin reviewed Apr 29, 2024

View reviewed changes

Add clarifying comment.

aeadf72

efriedma-quic mentioned this pull request May 4, 2024

[Clang] Implement P2809: Trivial infinite loops are not Undefined Behavior #90066

Merged

erichkeane approved these changes May 6, 2024

View reviewed changes

cor3ntin approved these changes May 6, 2024

View reviewed changes

Endilll reviewed May 6, 2024

View reviewed changes

katzdm changed the title ~~Don't wrap immediate invocations in ConstantExprs within constexpr initializers~~ Fix P2564 handling of variable initializers May 7, 2024

Merge branch 'main' into constexpr-init-fix

0e99692

cor3ntin changed the title ~~Fix P2564 handling of variable initializers~~ [Clang] Fix P2564 handling of variable initializers May 8, 2024

katzdm added 2 commits May 8, 2024 09:45

Merge branch 'main' into constexpr-init-fix

13228a2

Merge branch 'main' into constexpr-init-fix

3ce356a

cor3ntin merged commit 443377a into llvm:main May 9, 2024
4 of 5 checks passed

katzdm deleted the constexpr-init-fix branch May 9, 2024 11:57

katzdm mentioned this pull request May 18, 2024

No warning for implicit narrowing conversion during constant evaluation #92656

Open

efriedma-quic mentioned this pull request Nov 22, 2024

[clang] Immediate invocations in constexpr initailizers are evaluated twice #117321

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Clang] Fix P2564 handling of variable initializers #89565

[Clang] Fix P2564 handling of variable initializers #89565

katzdm commented Apr 22, 2024 •

edited

Loading

llvmbot commented Apr 22, 2024

cor3ntin commented Apr 22, 2024

katzdm commented Apr 22, 2024

katzdm left a comment

erichkeane left a comment

katzdm commented Apr 26, 2024

github-actions bot commented Apr 26, 2024 •

edited

Loading

efriedma-quic commented Apr 26, 2024

cor3ntin Apr 29, 2024

katzdm commented May 6, 2024

erichkeane left a comment

cor3ntin left a comment

Endilll left a comment

cor3ntin commented May 9, 2024

Fznamznon commented May 16, 2024

katzdm commented May 16, 2024

katzdm commented May 16, 2024

efriedma-quic commented May 16, 2024

Fznamznon commented May 17, 2024

katzdm commented May 18, 2024

[Clang] Fix P2564 handling of variable initializers #89565

[Clang] Fix P2564 handling of variable initializers #89565

Conversation

katzdm commented Apr 22, 2024 • edited Loading

llvmbot commented Apr 22, 2024

cor3ntin commented Apr 22, 2024

katzdm commented Apr 22, 2024

katzdm left a comment

Choose a reason for hiding this comment

erichkeane left a comment

Choose a reason for hiding this comment

katzdm commented Apr 26, 2024

github-actions bot commented Apr 26, 2024 • edited Loading

efriedma-quic commented Apr 26, 2024

cor3ntin Apr 29, 2024

Choose a reason for hiding this comment

katzdm commented May 6, 2024

erichkeane left a comment

Choose a reason for hiding this comment

cor3ntin left a comment

Choose a reason for hiding this comment

Endilll left a comment

Choose a reason for hiding this comment

cor3ntin commented May 9, 2024

Fznamznon commented May 16, 2024

katzdm commented May 16, 2024

katzdm commented May 16, 2024

efriedma-quic commented May 16, 2024

Fznamznon commented May 17, 2024

katzdm commented May 18, 2024

katzdm commented Apr 22, 2024 •

edited

Loading

github-actions bot commented Apr 26, 2024 •

edited

Loading