[clang] 443377a - [Clang] Fix P2564 handling of variable initializers (#89565)
via cfe-commits
cfe-commits at lists.llvm.org
Thu May 9 00:22:15 PDT 2024
Author: Daniel M. Katz
Date: 2024-05-09T09:22:11+02:00
New Revision: 443377a9d1a8d4a69a317a1a892184c59dd0aec6
URL: https://github.com/llvm/llvm-project/commit/443377a9d1a8d4a69a317a1a892184c59dd0aec6
DIFF: https://github.com/llvm/llvm-project/commit/443377a9d1a8d4a69a317a1a892184c59dd0aec6.diff
LOG: [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 <corentinjabot at gmail.com>
Added:
Modified:
clang/docs/ReleaseNotes.rst
clang/include/clang/Sema/Sema.h
clang/lib/Parse/ParseDecl.cpp
clang/lib/Sema/SemaChecking.cpp
clang/lib/Sema/SemaDeclCXX.cpp
clang/lib/Sema/SemaExpr.cpp
clang/test/SemaCXX/cxx2a-consteval.cpp
clang/test/SemaCXX/cxx2b-consteval-propagate.cpp
clang/test/SemaCXX/enum-scoped.cpp
Removed:
################################################################################
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index a3c8e4141ca54..4547636318a74 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -699,6 +699,10 @@ Bug Fixes to C++ Support
performed incorrectly when checking constraints. Fixes (#GH90349).
- Clang now allows constrained member functions to be explicitly specialized for an implicit instantiation
of a class template.
+- Fix a C++23 bug in implementation of P2564R3 which evaluates immediate invocations in place
+ within initializers for variables that are usable in constant expressions or are constant
+ initialized, rather than evaluating them as a part of the larger manifestly constant evaluated
+ expression.
Bug Fixes to AST Handling
^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index ddb3de2b66023..4efd3878e861b 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -10202,7 +10202,9 @@ class Sema final : public SemaBase {
S.ExprEvalContexts.back().InImmediateFunctionContext =
FD->isImmediateFunction() ||
S.ExprEvalContexts[S.ExprEvalContexts.size() - 2]
- .isConstantEvaluated();
+ .isConstantEvaluated() ||
+ S.ExprEvalContexts[S.ExprEvalContexts.size() - 2]
+ .isImmediateFunctionContext();
S.ExprEvalContexts.back().InImmediateEscalatingFunctionContext =
S.getLangOpts().CPlusPlus20 && FD->isImmediateEscalating();
} else
diff --git a/clang/lib/Parse/ParseDecl.cpp b/clang/lib/Parse/ParseDecl.cpp
index 4e4b05b21383e..2c11ae693c354 100644
--- a/clang/lib/Parse/ParseDecl.cpp
+++ b/clang/lib/Parse/ParseDecl.cpp
@@ -2587,25 +2587,30 @@ Decl *Parser::ParseDeclarationAfterDeclaratorAndAttributes(
Parser &P;
Declarator &D;
Decl *ThisDecl;
+ bool Entered;
InitializerScopeRAII(Parser &P, Declarator &D, Decl *ThisDecl)
- : P(P), D(D), ThisDecl(ThisDecl) {
+ : P(P), D(D), ThisDecl(ThisDecl), Entered(false) {
if (ThisDecl && P.getLangOpts().CPlusPlus) {
Scope *S = nullptr;
if (D.getCXXScopeSpec().isSet()) {
P.EnterScope(0);
S = P.getCurScope();
}
- P.Actions.ActOnCXXEnterDeclInitializer(S, ThisDecl);
+ if (ThisDecl && !ThisDecl->isInvalidDecl()) {
+ P.Actions.ActOnCXXEnterDeclInitializer(S, ThisDecl);
+ Entered = true;
+ }
}
}
- ~InitializerScopeRAII() { pop(); }
- void pop() {
+ ~InitializerScopeRAII() {
if (ThisDecl && P.getLangOpts().CPlusPlus) {
Scope *S = nullptr;
if (D.getCXXScopeSpec().isSet())
S = P.getCurScope();
- P.Actions.ActOnCXXExitDeclInitializer(S, ThisDecl);
+
+ if (Entered)
+ P.Actions.ActOnCXXExitDeclInitializer(S, ThisDecl);
if (S)
P.ExitScope();
}
@@ -2736,8 +2741,6 @@ Decl *Parser::ParseDeclarationAfterDeclaratorAndAttributes(
FRI->RangeExpr = Init;
}
- InitScope.pop();
-
if (Init.isInvalid()) {
SmallVector<tok::TokenKind, 2> StopTokens;
StopTokens.push_back(tok::comma);
@@ -2785,8 +2788,6 @@ Decl *Parser::ParseDeclarationAfterDeclaratorAndAttributes(
bool SawError = ParseExpressionList(Exprs, ExpressionStarts);
- InitScope.pop();
-
if (SawError) {
if (ThisVarDecl && PP.isCodeCompletionReached() && !CalledSignatureHelp) {
Actions.ProduceConstructorSignatureHelp(
@@ -2818,8 +2819,6 @@ Decl *Parser::ParseDeclarationAfterDeclaratorAndAttributes(
PreferredType.enterVariableInit(Tok.getLocation(), ThisDecl);
ExprResult Init(ParseBraceInitializer());
- InitScope.pop();
-
if (Init.isInvalid()) {
Actions.ActOnInitializerError(ThisDecl);
} else
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index e8e74467208c7..54789dde50691 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -16574,11 +16574,10 @@ static void CheckImplicitConversion(Sema &S, Expr *E, QualType T,
std::string PrettySourceValue = toString(Value, 10);
std::string PrettyTargetValue = PrettyPrintInRange(Value, TargetRange);
- S.DiagRuntimeBehavior(
- E->getExprLoc(), E,
- S.PDiag(diag::warn_impcast_integer_precision_constant)
- << PrettySourceValue << PrettyTargetValue << E->getType() << T
- << E->getSourceRange() << SourceRange(CC));
+ S.Diag(E->getExprLoc(),
+ S.PDiag(diag::warn_impcast_integer_precision_constant)
+ << PrettySourceValue << PrettyTargetValue << E->getType()
+ << T << E->getSourceRange() << SourceRange(CC));
return;
}
}
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 157d42c09cfcd..d77b9507066b0 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -18553,15 +18553,6 @@ void Sema::ActOnPureSpecifier(Decl *D, SourceLocation ZeroLoc) {
Diag(D->getLocation(), diag::err_illegal_initializer);
}
-/// Determine whether the given declaration is a global variable or
-/// static data member.
-static bool isNonlocalVariable(const Decl *D) {
- if (const VarDecl *Var = dyn_cast_or_null<VarDecl>(D))
- return Var->hasGlobalStorage();
-
- return false;
-}
-
/// Invoked when we are about to parse an initializer for the declaration
/// 'Dcl'.
///
@@ -18570,9 +18561,7 @@ static bool isNonlocalVariable(const Decl *D) {
/// class X. If the declaration had a scope specifier, a scope will have
/// been created and passed in for this purpose. Otherwise, S will be null.
void Sema::ActOnCXXEnterDeclInitializer(Scope *S, Decl *D) {
- // If there is no declaration, there was an error parsing it.
- if (!D || D->isInvalidDecl())
- return;
+ assert(D && !D->isInvalidDecl());
// We will always have a nested name specifier here, but this declaration
// might not be out of line if the specifier names the current namespace:
@@ -18581,25 +18570,41 @@ void Sema::ActOnCXXEnterDeclInitializer(Scope *S, Decl *D) {
if (S && D->isOutOfLine())
EnterDeclaratorContext(S, D->getDeclContext());
- // If we are parsing the initializer for a static data member, push a
- // new expression evaluation context that is associated with this static
- // data member.
- if (isNonlocalVariable(D))
- PushExpressionEvaluationContext(
- ExpressionEvaluationContext::PotentiallyEvaluated, D);
+ PushExpressionEvaluationContext(
+ ExpressionEvaluationContext::PotentiallyEvaluated, D);
}
/// Invoked after we are finished parsing an initializer for the declaration D.
void Sema::ActOnCXXExitDeclInitializer(Scope *S, Decl *D) {
- // If there is no declaration, there was an error parsing it.
- if (!D || D->isInvalidDecl())
- return;
-
- if (isNonlocalVariable(D))
- PopExpressionEvaluationContext();
+ assert(D);
if (S && D->isOutOfLine())
ExitDeclaratorContext(S);
+
+ if (getLangOpts().CPlusPlus23) {
+ // 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.
+ if (auto *VD = dyn_cast<VarDecl>(D);
+ VD && (VD->isUsableInConstantExpressions(Context) ||
+ VD->hasConstantInitialization())) {
+ // 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.
+ ExprEvalContexts.back().InImmediateFunctionContext = true;
+ }
+ }
+
+ // Unless the initializer is in an immediate function context (as determined
+ // above), this will evaluate all contained immediate function calls as
+ // constant expressions. If the initializer IS an immediate function context,
+ // the initializer has been determined to be a constant expression, and all
+ // such evaluations will be elided (i.e., as if we "knew the whole time" that
+ // it was a constant expression).
+ PopExpressionEvaluationContext();
}
/// ActOnCXXConditionDeclarationExpr - Parsed a condition declaration of a
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 94f99a423f0e0..c688cb21f2364 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -18039,7 +18039,7 @@ HandleImmediateInvocations(Sema &SemaRef,
Sema::ExpressionEvaluationContextRecord &Rec) {
if ((Rec.ImmediateInvocationCandidates.size() == 0 &&
Rec.ReferenceToConsteval.size() == 0) ||
- SemaRef.RebuildingImmediateInvocation)
+ Rec.isImmediateFunctionContext() || SemaRef.RebuildingImmediateInvocation)
return;
/// When we have more than 1 ImmediateInvocationCandidates or previously
diff --git a/clang/test/SemaCXX/cxx2a-consteval.cpp b/clang/test/SemaCXX/cxx2a-consteval.cpp
index e198074372072..622ec31c459dd 100644
--- a/clang/test/SemaCXX/cxx2a-consteval.cpp
+++ b/clang/test/SemaCXX/cxx2a-consteval.cpp
@@ -1068,6 +1068,14 @@ void test() {
constexpr int (*f2)(void) = lstatic; // expected-error {{constexpr variable 'f2' must be initialized by a constant expression}} \
// expected-note {{pointer to a consteval declaration is not a constant expression}}
+ int (*f3)(void) = []() consteval { return 3; }; // expected-error {{cannot take address of consteval call operator of '(lambda at}} \
+ // expected-note {{declared here}}
+}
+
+consteval void consteval_test() {
+ constexpr auto l1 = []() consteval { return 3; };
+
+ int (*f1)(void) = l1; // ok
}
}
@@ -1098,11 +1106,11 @@ int bad = 10; // expected-note 6{{declared here}}
tester glob1(make_name("glob1"));
tester glob2(make_name("glob2"));
constexpr tester cglob(make_name("cglob"));
-tester paddedglob(make_name(pad(bad))); // expected-error {{call to consteval function 'GH58207::make_name' is not a constant expression}} \
+tester paddedglob(make_name(pad(bad))); // expected-error {{call to consteval function 'GH58207::tester::tester' is not a constant expression}} \
// 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}} \
+constexpr tester glob4 = { make_name(pad(bad)) }; // expected-error {{call to consteval function 'GH58207::tester::tester' 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}}
@@ -1114,12 +1122,12 @@ auto V1 = make_name(pad(bad)); // expected-error {{call to consteval function 'G
void foo() {
static tester loc1(make_name("loc1"));
static constexpr tester loc2(make_name("loc2"));
- static tester paddedloc(make_name(pad(bad))); // expected-error {{call to consteval function 'GH58207::make_name' is not a constant expression}} \
+ static tester paddedloc(make_name(pad(bad))); // expected-error {{call to consteval function 'GH58207::tester::tester' is not a constant expression}} \
// expected-note {{read of non-const variable 'bad' is not allowed in a constant expression}}
}
void bar() {
- static tester paddedloc(make_name(pad(bad))); // expected-error {{call to consteval function 'GH58207::make_name' is not a constant expression}} \
+ static tester paddedloc(make_name(pad(bad))); // expected-error {{call to consteval function 'GH58207::tester::tester' is not a constant expression}} \
// expected-note {{read of non-const variable 'bad' is not allowed in a constant expression}}
}
}
diff --git a/clang/test/SemaCXX/cxx2b-consteval-propagate.cpp b/clang/test/SemaCXX/cxx2b-consteval-propagate.cpp
index 4a75392045d05..37fa1f1bdf59d 100644
--- a/clang/test/SemaCXX/cxx2b-consteval-propagate.cpp
+++ b/clang/test/SemaCXX/cxx2b-consteval-propagate.cpp
@@ -394,3 +394,29 @@ static_assert(none_of(
));
}
+
+#if __cplusplus >= 202302L
+namespace lvalue_to_rvalue_init_from_heap {
+
+struct S {
+ int *value;
+ constexpr S(int v) : value(new int {v}) {} // expected-note 2 {{heap allocation performed here}}
+ constexpr ~S() { delete value; }
+};
+consteval S fn() { return S(5); }
+int fn2() { return 2; } // expected-note {{declared here}}
+
+constexpr int a = *fn().value;
+constinit int b = *fn().value;
+const int c = *fn().value;
+int d = *fn().value;
+
+constexpr int e = *fn().value + fn2(); // expected-error {{must be initialized by a constant expression}} \
+ // expected-error {{call to consteval function 'lvalue_to_rvalue_init_from_heap::fn' is not a constant expression}} \
+ // expected-note {{non-constexpr function 'fn2'}} \
+ // expected-note {{pointer to heap-allocated object}}
+
+int f = *fn().value + fn2(); // expected-error {{call to consteval function 'lvalue_to_rvalue_init_from_heap::fn' is not a constant expression}} \
+ // expected-note {{pointer to heap-allocated object}}
+}
+#endif
diff --git a/clang/test/SemaCXX/enum-scoped.cpp b/clang/test/SemaCXX/enum-scoped.cpp
index b1d9a215c437c..d7b7923430aff 100644
--- a/clang/test/SemaCXX/enum-scoped.cpp
+++ b/clang/test/SemaCXX/enum-scoped.cpp
@@ -53,6 +53,7 @@ enum class E4 {
e1 = -2147483648, // ok
e2 = 2147483647, // ok
e3 = 2147483648 // expected-error{{enumerator value evaluates to 2147483648, which cannot be narrowed to type 'int'}}
+ // expected-warning at -1{{changes value}}
};
enum class E5 {
More information about the cfe-commits
mailing list