wchilders added inline comments.
================
Comment at: clang/lib/Sema/SemaDeclCXX.cpp:16793-16797
+ if (isManifestlyEvaluatedVar(*this, D)) {
+ using ExpressionKind = ExpressionEvaluationContextRecord::ExpressionKind;
+
+ PushExpressionEvaluationContext(
+ ExpressionEvaluationContext::ConstantEvaluated, D,
ExpressionKind::EK_ConstexprVarInit);
----------------
rsmith wrote:
> We can't implement the checks for manifestly constant-evaluated initializers
> this way in general. Per [expr.const]/14, we need to treat the initializer as
> manifestly constant-evaluated if it's "the initializer of a variable that is
> usable in constant expressions or has constant initialization". We can't test
> either of those conditions in general until we know what the initializer is,
> because they can both depend on whether evaluation of the initializer
> succeeds. (This approach works for the case of `constexpr` variables, which
> are always usable in constant expressions, but not any of the other cases,
> and the approach we'll need for the other cases will also handle `constexpr`
> variables. There is a very modest benefit to special-casing `constexpr`
> variable initializers regardless -- we can avoid forming and then pruning out
> nested `ConstantExpr` nodes for immediate invocations inside the initializer
> -- but I think it's probably not worth the added complexity.)
>
> To address the general problem, we should handle this in
> `CheckCompleteVariableDeclaration`, which is where we evaluate the
> initializer and generally determine whether the variable has constant
> initialization and / or is usable in constant expressions. Probably the
> cleanest approach -- and certainly the one I'd been intending to pursue --
> would be to wrap the initializer with a `ConstantExpr` there in the relevant
> cases, and allow the usual handling of nested immediate invocations to prune
> out any `ConstantExpr`s nested within the initializer representing inner
> calls to `consteval` functions. (I think I've mentioned elsewhere that I
> would like to remove the "evaluated value" storage on `VarDecl` in favor of
> using `ConstantExpr` for this purpose.)
> There is a very modest benefit to special-casing constexpr variable
> initializers regardless -- we can avoid forming and then pruning out nested
> ConstantExpr nodes for immediate invocations inside the initializer -- but I
> think it's probably not worth the added complexity.
So, this patch is motivated for us by the desire to check if a "meta type"
variable, belongs to a compile time, or runtime. Additionally, this is used
being used to verify our compile time, "injection statement", is not appearing
in a runtime context. This prevents reflections/metaprogramming values and
logic from leaking nonsensically into runtime code.
I think this can be accomplished as you said in the
`CheckCompleteVariableDeclaration`. We're already checking for out of place
"meta type" variables there, though we catch fragments, and injection
statements more eagerly. In retrospect, I don't think there is any issue
removing the eager check from the fragments, and I don't think the checking of
the injection statements should be affected by this case.
I'll try and look more into this in the morning.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D76447/new/
https://reviews.llvm.org/D76447
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
