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 <corentinja...@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@-1{{changes value}} }; enum class E5 { _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
