https://github.com/katzdm created 
https://github.com/llvm/llvm-project/pull/89565

The following program produces a diagnostic in Clang and EDG, but compiles 
correctly in GCC and MSVC:
```cpp
#include <vector>

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

Clang's diagnostic is as follows:
```cpp
<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`](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 <line:6:1, col:25> col:15 a 'const int' constexpr cinit
  |-value: Int 2
  `-ExprWithCleanups <col:19, col:25> 'value_type':'int'
    `-ImplicitCastExpr <col:19, col:25> 'value_type':'int' <LValueToRValue>
      `-CXXOperatorCallExpr <col:19, col:25> 'value_type':'int' lvalue '[]'
        |-ImplicitCastExpr <col:23, col:25> 'reference (*)(size_type) noexcept' 
<FunctionToPointerDecay>
        | `-DeclRefExpr <col:23, col:25> 'reference (size_type) noexcept' 
lvalue CXXMethod 0xd1497a0 'operator[]' 'reference (size_type) noexcept'
        |-MaterializeTemporaryExpr <col:19, col:22> 'std::vector<int>' lvalue
        | `-ConstantExpr <col:19, col:22> 'std::vector<int>'
        |   `-ExprWithCleanups <col:19, col:22> 'std::vector<int>'
        |     `-CXXBindTemporaryExpr <col:19, col:22> 'std::vector<int>' 
(CXXTemporary 0xd180de8)
        |       `-CallExpr <col:19, col:22> 'std::vector<int>'
        |         `-ImplicitCastExpr <col:19> 'std::vector<int> (*)()' 
<FunctionToPointerDecay>
        |           `-DeclRefExpr <col:19> 'std::vector<int> ()' lvalue 
Function 0xd12e7b8 'fn' 'std::vector<int> ()'
        `-ImplicitCastExpr <col:24> 'size_type':'unsigned long' <IntegralCast>
          `-IntegerLiteral <col:24> '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`](https://github.com/llvm/llvm-project/blob/main/clang/lib/Sema/SemaDeclCXX.cpp#L18579-L18581)
 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](https://github.com/llvm/llvm-project/compare/main...katzdm:constexpr-init-fix?expand=1#diff-f10defc3f20fb095bf22b3a79bead200d494bde9d503e283067a57aff483936cL729)
 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.

>From a3f8a8648e2002273d47d7886b29fb02c728b5b7 Mon Sep 17 00:00:00 2001
From: Dan Katz <kat...@gmail.com>
Date: Tue, 16 Apr 2024 17:14:50 -0400
Subject: [PATCH] Fix bug with constexpr initialization.

---
 clang/lib/Parse/ParseDecl.cpp                 | 12 ++++++-
 clang/test/CXX/expr/expr.const/p6-2a.cpp      |  7 ++--
 clang/test/SemaCXX/builtin_vectorelements.cpp |  4 +--
 clang/test/SemaCXX/cxx2a-consteval.cpp        | 32 ++++++++++++-------
 clang/unittests/Support/TimeProfilerTest.cpp  |  1 -
 5 files changed, 36 insertions(+), 20 deletions(-)

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>)

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to