Skip to content

Commit 443377a

Browse files
katzdmcor3ntin
andauthored
[Clang] Fix P2564 handling of variable initializers (#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. This PR turns out to be a bug fix on the implementation of [P2564R3](https://wg21.link/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]`](https://eel.is/c++draft/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](https://eel.is/c++draft/expr.const#20): > 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](https://eel.is/c++draft/expr.const#16): > 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](https://eel.is/c++draft/expr.const#16) or [immediate-escalating expressions](https://eel.is/c++draft/expr.const#17) 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: 1. Create a new expression evaluation context for _every_ variable initializer (rather than only nonlocal ones). 2. Attach initializers to `VarDecl`s _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. 3. 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](https://github.com/llvm/llvm-project/blob/760910ddb918d77e7632be1678f69909384d69ae/clang/lib/Sema/SemaExpr.cpp#L17799) 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. --------- Co-authored-by: cor3ntin <[email protected]>
1 parent c4a3d18 commit 443377a

File tree

9 files changed

+90
-46
lines changed

9 files changed

+90
-46
lines changed

clang/docs/ReleaseNotes.rst

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -699,6 +699,10 @@ Bug Fixes to C++ Support
699699
performed incorrectly when checking constraints. Fixes (#GH90349).
700700
- Clang now allows constrained member functions to be explicitly specialized for an implicit instantiation
701701
of a class template.
702+
- Fix a C++23 bug in implementation of P2564R3 which evaluates immediate invocations in place
703+
within initializers for variables that are usable in constant expressions or are constant
704+
initialized, rather than evaluating them as a part of the larger manifestly constant evaluated
705+
expression.
702706

703707
Bug Fixes to AST Handling
704708
^^^^^^^^^^^^^^^^^^^^^^^^^

clang/include/clang/Sema/Sema.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10202,7 +10202,9 @@ class Sema final : public SemaBase {
1020210202
S.ExprEvalContexts.back().InImmediateFunctionContext =
1020310203
FD->isImmediateFunction() ||
1020410204
S.ExprEvalContexts[S.ExprEvalContexts.size() - 2]
10205-
.isConstantEvaluated();
10205+
.isConstantEvaluated() ||
10206+
S.ExprEvalContexts[S.ExprEvalContexts.size() - 2]
10207+
.isImmediateFunctionContext();
1020610208
S.ExprEvalContexts.back().InImmediateEscalatingFunctionContext =
1020710209
S.getLangOpts().CPlusPlus20 && FD->isImmediateEscalating();
1020810210
} else

clang/lib/Parse/ParseDecl.cpp

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2587,25 +2587,30 @@ Decl *Parser::ParseDeclarationAfterDeclaratorAndAttributes(
25872587
Parser &P;
25882588
Declarator &D;
25892589
Decl *ThisDecl;
2590+
bool Entered;
25902591

25912592
InitializerScopeRAII(Parser &P, Declarator &D, Decl *ThisDecl)
2592-
: P(P), D(D), ThisDecl(ThisDecl) {
2593+
: P(P), D(D), ThisDecl(ThisDecl), Entered(false) {
25932594
if (ThisDecl && P.getLangOpts().CPlusPlus) {
25942595
Scope *S = nullptr;
25952596
if (D.getCXXScopeSpec().isSet()) {
25962597
P.EnterScope(0);
25972598
S = P.getCurScope();
25982599
}
2599-
P.Actions.ActOnCXXEnterDeclInitializer(S, ThisDecl);
2600+
if (ThisDecl && !ThisDecl->isInvalidDecl()) {
2601+
P.Actions.ActOnCXXEnterDeclInitializer(S, ThisDecl);
2602+
Entered = true;
2603+
}
26002604
}
26012605
}
2602-
~InitializerScopeRAII() { pop(); }
2603-
void pop() {
2606+
~InitializerScopeRAII() {
26042607
if (ThisDecl && P.getLangOpts().CPlusPlus) {
26052608
Scope *S = nullptr;
26062609
if (D.getCXXScopeSpec().isSet())
26072610
S = P.getCurScope();
2608-
P.Actions.ActOnCXXExitDeclInitializer(S, ThisDecl);
2611+
2612+
if (Entered)
2613+
P.Actions.ActOnCXXExitDeclInitializer(S, ThisDecl);
26092614
if (S)
26102615
P.ExitScope();
26112616
}
@@ -2736,8 +2741,6 @@ Decl *Parser::ParseDeclarationAfterDeclaratorAndAttributes(
27362741
FRI->RangeExpr = Init;
27372742
}
27382743

2739-
InitScope.pop();
2740-
27412744
if (Init.isInvalid()) {
27422745
SmallVector<tok::TokenKind, 2> StopTokens;
27432746
StopTokens.push_back(tok::comma);
@@ -2785,8 +2788,6 @@ Decl *Parser::ParseDeclarationAfterDeclaratorAndAttributes(
27852788

27862789
bool SawError = ParseExpressionList(Exprs, ExpressionStarts);
27872790

2788-
InitScope.pop();
2789-
27902791
if (SawError) {
27912792
if (ThisVarDecl && PP.isCodeCompletionReached() && !CalledSignatureHelp) {
27922793
Actions.ProduceConstructorSignatureHelp(
@@ -2818,8 +2819,6 @@ Decl *Parser::ParseDeclarationAfterDeclaratorAndAttributes(
28182819
PreferredType.enterVariableInit(Tok.getLocation(), ThisDecl);
28192820
ExprResult Init(ParseBraceInitializer());
28202821

2821-
InitScope.pop();
2822-
28232822
if (Init.isInvalid()) {
28242823
Actions.ActOnInitializerError(ThisDecl);
28252824
} else

clang/lib/Sema/SemaChecking.cpp

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16574,11 +16574,10 @@ static void CheckImplicitConversion(Sema &S, Expr *E, QualType T,
1657416574
std::string PrettySourceValue = toString(Value, 10);
1657516575
std::string PrettyTargetValue = PrettyPrintInRange(Value, TargetRange);
1657616576

16577-
S.DiagRuntimeBehavior(
16578-
E->getExprLoc(), E,
16579-
S.PDiag(diag::warn_impcast_integer_precision_constant)
16580-
<< PrettySourceValue << PrettyTargetValue << E->getType() << T
16581-
<< E->getSourceRange() << SourceRange(CC));
16577+
S.Diag(E->getExprLoc(),
16578+
S.PDiag(diag::warn_impcast_integer_precision_constant)
16579+
<< PrettySourceValue << PrettyTargetValue << E->getType()
16580+
<< T << E->getSourceRange() << SourceRange(CC));
1658216581
return;
1658316582
}
1658416583
}

clang/lib/Sema/SemaDeclCXX.cpp

Lines changed: 29 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -18553,15 +18553,6 @@ void Sema::ActOnPureSpecifier(Decl *D, SourceLocation ZeroLoc) {
1855318553
Diag(D->getLocation(), diag::err_illegal_initializer);
1855418554
}
1855518555

18556-
/// Determine whether the given declaration is a global variable or
18557-
/// static data member.
18558-
static bool isNonlocalVariable(const Decl *D) {
18559-
if (const VarDecl *Var = dyn_cast_or_null<VarDecl>(D))
18560-
return Var->hasGlobalStorage();
18561-
18562-
return false;
18563-
}
18564-
1856518556
/// Invoked when we are about to parse an initializer for the declaration
1856618557
/// 'Dcl'.
1856718558
///
@@ -18570,9 +18561,7 @@ static bool isNonlocalVariable(const Decl *D) {
1857018561
/// class X. If the declaration had a scope specifier, a scope will have
1857118562
/// been created and passed in for this purpose. Otherwise, S will be null.
1857218563
void Sema::ActOnCXXEnterDeclInitializer(Scope *S, Decl *D) {
18573-
// If there is no declaration, there was an error parsing it.
18574-
if (!D || D->isInvalidDecl())
18575-
return;
18564+
assert(D && !D->isInvalidDecl());
1857618565

1857718566
// We will always have a nested name specifier here, but this declaration
1857818567
// might not be out of line if the specifier names the current namespace:
@@ -18581,25 +18570,41 @@ void Sema::ActOnCXXEnterDeclInitializer(Scope *S, Decl *D) {
1858118570
if (S && D->isOutOfLine())
1858218571
EnterDeclaratorContext(S, D->getDeclContext());
1858318572

18584-
// If we are parsing the initializer for a static data member, push a
18585-
// new expression evaluation context that is associated with this static
18586-
// data member.
18587-
if (isNonlocalVariable(D))
18588-
PushExpressionEvaluationContext(
18589-
ExpressionEvaluationContext::PotentiallyEvaluated, D);
18573+
PushExpressionEvaluationContext(
18574+
ExpressionEvaluationContext::PotentiallyEvaluated, D);
1859018575
}
1859118576

1859218577
/// Invoked after we are finished parsing an initializer for the declaration D.
1859318578
void Sema::ActOnCXXExitDeclInitializer(Scope *S, Decl *D) {
18594-
// If there is no declaration, there was an error parsing it.
18595-
if (!D || D->isInvalidDecl())
18596-
return;
18597-
18598-
if (isNonlocalVariable(D))
18599-
PopExpressionEvaluationContext();
18579+
assert(D);
1860018580

1860118581
if (S && D->isOutOfLine())
1860218582
ExitDeclaratorContext(S);
18583+
18584+
if (getLangOpts().CPlusPlus23) {
18585+
// An expression or conversion is 'manifestly constant-evaluated' if it is:
18586+
// [...]
18587+
// - the initializer of a variable that is usable in constant expressions or
18588+
// has constant initialization.
18589+
if (auto *VD = dyn_cast<VarDecl>(D);
18590+
VD && (VD->isUsableInConstantExpressions(Context) ||
18591+
VD->hasConstantInitialization())) {
18592+
// An expression or conversion is in an 'immediate function context' if it
18593+
// is potentially evaluated and either:
18594+
// [...]
18595+
// - it is a subexpression of a manifestly constant-evaluated expression
18596+
// or conversion.
18597+
ExprEvalContexts.back().InImmediateFunctionContext = true;
18598+
}
18599+
}
18600+
18601+
// Unless the initializer is in an immediate function context (as determined
18602+
// above), this will evaluate all contained immediate function calls as
18603+
// constant expressions. If the initializer IS an immediate function context,
18604+
// the initializer has been determined to be a constant expression, and all
18605+
// such evaluations will be elided (i.e., as if we "knew the whole time" that
18606+
// it was a constant expression).
18607+
PopExpressionEvaluationContext();
1860318608
}
1860418609

1860518610
/// ActOnCXXConditionDeclarationExpr - Parsed a condition declaration of a

clang/lib/Sema/SemaExpr.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18039,7 +18039,7 @@ HandleImmediateInvocations(Sema &SemaRef,
1803918039
Sema::ExpressionEvaluationContextRecord &Rec) {
1804018040
if ((Rec.ImmediateInvocationCandidates.size() == 0 &&
1804118041
Rec.ReferenceToConsteval.size() == 0) ||
18042-
SemaRef.RebuildingImmediateInvocation)
18042+
Rec.isImmediateFunctionContext() || SemaRef.RebuildingImmediateInvocation)
1804318043
return;
1804418044

1804518045
/// When we have more than 1 ImmediateInvocationCandidates or previously

clang/test/SemaCXX/cxx2a-consteval.cpp

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1068,6 +1068,14 @@ void test() {
10681068
constexpr int (*f2)(void) = lstatic; // expected-error {{constexpr variable 'f2' must be initialized by a constant expression}} \
10691069
// expected-note {{pointer to a consteval declaration is not a constant expression}}
10701070

1071+
int (*f3)(void) = []() consteval { return 3; }; // expected-error {{cannot take address of consteval call operator of '(lambda at}} \
1072+
// expected-note {{declared here}}
1073+
}
1074+
1075+
consteval void consteval_test() {
1076+
constexpr auto l1 = []() consteval { return 3; };
1077+
1078+
int (*f1)(void) = l1; // ok
10711079
}
10721080
}
10731081

@@ -1098,11 +1106,11 @@ int bad = 10; // expected-note 6{{declared here}}
10981106
tester glob1(make_name("glob1"));
10991107
tester glob2(make_name("glob2"));
11001108
constexpr tester cglob(make_name("cglob"));
1101-
tester paddedglob(make_name(pad(bad))); // expected-error {{call to consteval function 'GH58207::make_name' is not a constant expression}} \
1109+
tester paddedglob(make_name(pad(bad))); // expected-error {{call to consteval function 'GH58207::tester::tester' is not a constant expression}} \
11021110
// expected-note {{read of non-const variable 'bad' is not allowed in a constant expression}}
11031111

11041112
constexpr tester glob3 = { make_name("glob3") };
1105-
constexpr tester glob4 = { make_name(pad(bad)) }; // expected-error {{call to consteval function 'GH58207::make_name' is not a constant expression}} \
1113+
constexpr tester glob4 = { make_name(pad(bad)) }; // expected-error {{call to consteval function 'GH58207::tester::tester' is not a constant expression}} \
11061114
// expected-error {{constexpr variable 'glob4' must be initialized by a constant expression}} \
11071115
// expected-note 2{{read of non-const variable 'bad' is not allowed in a constant expression}}
11081116

@@ -1114,12 +1122,12 @@ auto V1 = make_name(pad(bad)); // expected-error {{call to consteval function 'G
11141122
void foo() {
11151123
static tester loc1(make_name("loc1"));
11161124
static constexpr tester loc2(make_name("loc2"));
1117-
static tester paddedloc(make_name(pad(bad))); // expected-error {{call to consteval function 'GH58207::make_name' is not a constant expression}} \
1125+
static tester paddedloc(make_name(pad(bad))); // expected-error {{call to consteval function 'GH58207::tester::tester' is not a constant expression}} \
11181126
// expected-note {{read of non-const variable 'bad' is not allowed in a constant expression}}
11191127
}
11201128

11211129
void bar() {
1122-
static tester paddedloc(make_name(pad(bad))); // expected-error {{call to consteval function 'GH58207::make_name' is not a constant expression}} \
1130+
static tester paddedloc(make_name(pad(bad))); // expected-error {{call to consteval function 'GH58207::tester::tester' is not a constant expression}} \
11231131
// expected-note {{read of non-const variable 'bad' is not allowed in a constant expression}}
11241132
}
11251133
}

clang/test/SemaCXX/cxx2b-consteval-propagate.cpp

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -394,3 +394,29 @@ static_assert(none_of(
394394
));
395395

396396
}
397+
398+
#if __cplusplus >= 202302L
399+
namespace lvalue_to_rvalue_init_from_heap {
400+
401+
struct S {
402+
int *value;
403+
constexpr S(int v) : value(new int {v}) {} // expected-note 2 {{heap allocation performed here}}
404+
constexpr ~S() { delete value; }
405+
};
406+
consteval S fn() { return S(5); }
407+
int fn2() { return 2; } // expected-note {{declared here}}
408+
409+
constexpr int a = *fn().value;
410+
constinit int b = *fn().value;
411+
const int c = *fn().value;
412+
int d = *fn().value;
413+
414+
constexpr int e = *fn().value + fn2(); // expected-error {{must be initialized by a constant expression}} \
415+
// expected-error {{call to consteval function 'lvalue_to_rvalue_init_from_heap::fn' is not a constant expression}} \
416+
// expected-note {{non-constexpr function 'fn2'}} \
417+
// expected-note {{pointer to heap-allocated object}}
418+
419+
int f = *fn().value + fn2(); // expected-error {{call to consteval function 'lvalue_to_rvalue_init_from_heap::fn' is not a constant expression}} \
420+
// expected-note {{pointer to heap-allocated object}}
421+
}
422+
#endif

clang/test/SemaCXX/enum-scoped.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ enum class E4 {
5353
e1 = -2147483648, // ok
5454
e2 = 2147483647, // ok
5555
e3 = 2147483648 // expected-error{{enumerator value evaluates to 2147483648, which cannot be narrowed to type 'int'}}
56+
// expected-warning@-1{{changes value}}
5657
};
5758

5859
enum class E5 {

0 commit comments

Comments
 (0)