katzdm wrote: This ended up being a lot more subtle than I expected. My latest push proposes a different solution; @cor3ntin and @efriedma-quic - please let me know your thoughts.
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. Sorry for the long-winded explanation of a relatively small change - the language in `[expr.const]` is (at least I find) quite involved, so I figured more detail couldn't hurt. https://github.com/llvm/llvm-project/pull/89565 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
