[clang] Don't wrap immediate invocations in ConstantExprs within constexpr initializers (PR #89565)
Daniel M. Katz via cfe-commits
cfe-commits at lists.llvm.org
Sun Apr 21 19:48:16 PDT 2024
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 <katzdm at 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>)
More information about the cfe-commits
mailing list