[PATCH] D76447: Apply ConstantEvaluated evaluation contexts to more manifestly constant evaluated scopes
wchilders updated this revision to Diff 252357.
wchilders added a comment.
Updated the patch to correct formatting issues, and removed application of the
`ConstantEvaluated` evaluation context to var decls as it introduces too much
complexity.
Additionally removed a fix for `decltype` with immediate functions that snuck
into this patch. This will be submitted separately.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D76447/new/
https://reviews.llvm.org/D76447
Files:
clang/lib/Parse/ParseDeclCXX.cpp
clang/lib/Parse/ParseStmt.cpp
clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
clang/lib/Sema/TreeTransform.h
Index: clang/lib/Sema/TreeTransform.h
===
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -12169,7 +12169,9 @@
// FIXME: Sema's lambda-building mechanism expects us to push an expression
// evaluation context even if we're not transforming the function body.
getSema().PushExpressionEvaluationContext(
- Sema::ExpressionEvaluationContext::PotentiallyEvaluated);
+ E->getCallOperator()->getConstexprKind() == CSK_consteval
+ ? Sema::ExpressionEvaluationContext::ConstantEvaluated
+ : Sema::ExpressionEvaluationContext::PotentiallyEvaluated);
// Instantiate the body of the lambda expression.
StmtResult Body =
Index: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -4644,7 +4644,9 @@
Function->setInnerLocStart(PatternDecl->getInnerLocStart());
EnterExpressionEvaluationContext EvalContext(
- *this, Sema::ExpressionEvaluationContext::PotentiallyEvaluated);
+ *this, PatternDecl->getConstexprKind() == CSK_consteval
+ ? Sema::ExpressionEvaluationContext::ConstantEvaluated
+ : Sema::ExpressionEvaluationContext::PotentiallyEvaluated);
// Introduce a new scope where local variable instantiations will be
// recorded, unless we're actually a member function within a local
@@ -4947,8 +4949,13 @@
Var->setImplicitlyInline();
if (OldVar->getInit()) {
+bool IsConstexpr = OldVar->isConstexpr() || isConstantEvaluated();
EnterExpressionEvaluationContext Evaluated(
-*this, Sema::ExpressionEvaluationContext::PotentiallyEvaluated, Var);
+*this,
+getLangOpts().CPlusPlus2a && IsConstexpr
+? Sema::ExpressionEvaluationContext::ConstantEvaluated
+: Sema::ExpressionEvaluationContext::PotentiallyEvaluated,
+Var);
// Instantiate the initializer.
ExprResult Init;
Index: clang/lib/Parse/ParseStmt.cpp
===
--- clang/lib/Parse/ParseStmt.cpp
+++ clang/lib/Parse/ParseStmt.cpp
@@ -1329,10 +1329,19 @@
// Parse the condition.
StmtResult InitStmt;
Sema::ConditionResult Cond;
- if (ParseParenExprOrCondition(, Cond, IfLoc,
-IsConstexpr ? Sema::ConditionKind::ConstexprIf
-: Sema::ConditionKind::Boolean))
-return StmtError();
+
+ {
+EnterExpressionEvaluationContext Unevaluated(
+Actions, Sema::ExpressionEvaluationContext::ConstantEvaluated,
+/*LambdaContextDecl=*/nullptr, /*ExprContext=*/
+Sema::ExpressionEvaluationContextRecord::EK_Other,
+/*ShouldEnter=*/IsConstexpr);
+
+if (ParseParenExprOrCondition(, Cond, IfLoc,
+ IsConstexpr ?
Sema::ConditionKind::ConstexprIf
+ : Sema::ConditionKind::Boolean))
+ return StmtError();
+ }
llvm::Optional ConstexprCondition;
if (IsConstexpr)
Index: clang/lib/Parse/ParseDeclCXX.cpp
===
--- clang/lib/Parse/ParseDeclCXX.cpp
+++ clang/lib/Parse/ParseDeclCXX.cpp
@@ -2974,11 +2974,20 @@
/// be a constant-expression.
ExprResult Parser::ParseCXXMemberInitializer(Decl *D, bool IsFunction,
SourceLocation ) {
+ using EEC = Sema::ExpressionEvaluationContext;
+
assert(Tok.isOneOf(tok::equal, tok::l_brace)
&& "Data member initializer not starting with '=' or '{'");
+ bool IsConstexpr = false;
+ if (auto *VD = dyn_cast_or_null(D))
+IsConstexpr = VD->isConstexpr();
+
EnterExpressionEvaluationContext Context(
- Actions, Sema::ExpressionEvaluationContext::PotentiallyEvaluated, D);
+ Actions,
+ getLangOpts().CPlusPlus2a && IsConstexpr ? EEC::ConstantEvaluated
+ : EEC::PotentiallyEvaluated,
+ D);
if (TryConsumeToken(tok::equal, EqualLoc)) {
if (Tok.is(tok::kw_delete)) {
// In principle, an initializer of '= delete p;' is legal, but it will
Index: clang/lib/Sema/TreeTransform.h
[PATCH] D76447: Apply ConstantEvaluated evaluation contexts to more manifestly constant evaluated scopes
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
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D76447: Apply ConstantEvaluated evaluation contexts to more manifestly constant evaluated scopes
rsmith 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);
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.)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D76447/new/
https://reviews.llvm.org/D76447
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D76447: Apply ConstantEvaluated evaluation contexts to more manifestly constant evaluated scopes
wchilders marked an inline comment as done.
wchilders added inline comments.
Comment at: clang/lib/Sema/SemaExpr.cpp:15597
+ (Rec.isConstantEvaluated() &&
+ Rec.ExprContext != ExpressionKind::EK_ConstexprVarInit)) {
ExprCleanupObjects.erase(ExprCleanupObjects.begin() +
Rec.NumCleanupObjects,
This is my main concern with this patch. There's something subtle here, and
it's unclear from an outsider's (my) perspective what the root issue is here.
Effectively we get into trouble when we first enter
`ActOnCXXEnterDeclInitializer` on the `isManifestlyEvaluatedVar` branch. We
then exit at `ActOnCXXExitDeclInitializer`, triggering this branch, as
`Rec.isConstantEvaluated()` is true. This results in the cleanups being
dropped. Then when we enter the constexpr evaluator for
`VarDecl::evaluateValue`. the evaluator is unhappy as we're missing a cleanup
for the created temporary.
Any input that allows this to be resolved without the special case, or that
otherwise informs how this should work, and why unevaluated and constant
evaluated are treated equally here, would be awesome. I'll note that in
reviewing the revision history, this condition dates back to when
`ConstantEvaluated` was created, so perhaps this is a "historical artifact" of
that time?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D76447/new/
https://reviews.llvm.org/D76447
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D76447: Apply ConstantEvaluated evaluation contexts to more manifestly constant evaluated scopes
wchilders created this revision.
wchilders added reviewers: Tyker, rsmith.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
wchilders marked an inline comment as done.
wchilders added inline comments.
Comment at: clang/lib/Sema/SemaExpr.cpp:15597
+ (Rec.isConstantEvaluated() &&
+ Rec.ExprContext != ExpressionKind::EK_ConstexprVarInit)) {
ExprCleanupObjects.erase(ExprCleanupObjects.begin() +
Rec.NumCleanupObjects,
This is my main concern with this patch. There's something subtle here, and
it's unclear from an outsider's (my) perspective what the root issue is here.
Effectively we get into trouble when we first enter
`ActOnCXXEnterDeclInitializer` on the `isManifestlyEvaluatedVar` branch. We
then exit at `ActOnCXXExitDeclInitializer`, triggering this branch, as
`Rec.isConstantEvaluated()` is true. This results in the cleanups being
dropped. Then when we enter the constexpr evaluator for
`VarDecl::evaluateValue`. the evaluator is unhappy as we're missing a cleanup
for the created temporary.
Any input that allows this to be resolved without the special case, or that
otherwise informs how this should work, and why unevaluated and constant
evaluated are treated equally here, would be awesome. I'll note that in
reviewing the revision history, this condition dates back to when
`ConstantEvaluated` was created, so perhaps this is a "historical artifact" of
that time?
This patch attempts to update application of evaluation contexts to better
represent, what is manifestly constant evaluated.
There are some outstanding concerns I'll mark with inline comments.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D76447
Files:
clang/include/clang/Sema/Sema.h
clang/lib/Parse/ParseDeclCXX.cpp
clang/lib/Parse/ParseStmt.cpp
clang/lib/Sema/SemaDeclCXX.cpp
clang/lib/Sema/SemaExpr.cpp
clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
clang/lib/Sema/TreeTransform.h
Index: clang/lib/Sema/TreeTransform.h
===
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -12079,6 +12079,8 @@
// FIXME: Sema's lambda-building mechanism expects us to push an expression
// evaluation context even if we're not transforming the function body.
getSema().PushExpressionEvaluationContext(
+ E->getCallOperator()->getConstexprKind() == CSK_consteval ?
+ Sema::ExpressionEvaluationContext::ConstantEvaluated :
Sema::ExpressionEvaluationContext::PotentiallyEvaluated);
// Instantiate the body of the lambda expression.
Index: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -4644,7 +4644,10 @@
Function->setInnerLocStart(PatternDecl->getInnerLocStart());
EnterExpressionEvaluationContext EvalContext(
- *this, Sema::ExpressionEvaluationContext::PotentiallyEvaluated);
+ *this,
+ PatternDecl->getConstexprKind() == CSK_consteval
+ ? Sema::ExpressionEvaluationContext::ConstantEvaluated
+ : Sema::ExpressionEvaluationContext::PotentiallyEvaluated);
// Introduce a new scope where local variable instantiations will be
// recorded, unless we're actually a member function within a local
@@ -4947,8 +4950,11 @@
Var->setImplicitlyInline();
if (OldVar->getInit()) {
+bool IsConstexpr = OldVar->isConstexpr() || isConstantEvaluated();
EnterExpressionEvaluationContext Evaluated(
-*this, Sema::ExpressionEvaluationContext::PotentiallyEvaluated, Var);
+*this, getLangOpts().CPlusPlus2a && IsConstexpr ?
+Sema::ExpressionEvaluationContext::ConstantEvaluated :
+Sema::ExpressionEvaluationContext::PotentiallyEvaluated, Var);
// Instantiate the initializer.
ExprResult Init;
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -15369,9 +15369,16 @@
}
}
+// Check to see if this expression is part of a decltype specifier.
+// We're not interested in evaluating this expression, immediately or otherwise.
+static bool isInDeclType(Sema ) {
+ return SemaRef.ExprEvalContexts.back().ExprContext ==
+Sema::ExpressionEvaluationContextRecord::EK_Decltype;
+}
+
ExprResult Sema::CheckForImmediateInvocation(ExprResult E, FunctionDecl *Decl) {
if (!E.isUsable() || !Decl || !Decl->isConsteval() || isConstantEvaluated() ||
- RebuildingImmediateInvocation)
+ isInDeclType(*this) || RebuildingImmediateInvocation)
return E;
/// Opportunistically remove the callee from ReferencesToConsteval if we can.
@@ -15532,11 +15539,12 @@
}
void Sema::PopExpressionEvaluationContext() {
+ using ExpressionKind = ExpressionEvaluationContextRecord::ExpressionKind;
+
