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 &SemaRef) {
+ 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;
+
ExpressionEvaluationContextRecord& Rec = ExprEvalContexts.back();
unsigned NumTypos = Rec.NumTypos;
if (!Rec.Lambdas.empty()) {
- using ExpressionKind = ExpressionEvaluationContextRecord::ExpressionKind;
if (Rec.ExprContext == ExpressionKind::EK_TemplateArgument || Rec.isUnevaluated() ||
(Rec.isConstantEvaluated() && !getLangOpts().CPlusPlus17)) {
unsigned D;
@@ -15577,7 +15585,16 @@
// temporaries that we may have created as part of the evaluation of
// the expression in that context: they aren't relevant because they
// will never be constructed.
- if (Rec.isUnevaluated() || Rec.isConstantEvaluated()) {
+ //
+ // FIXME: The condition Rec.ExprContext != EK_ConstexprVarInit
+ // has been added as a stop gap fix. After adding a ConstEvaluated
+ // context to constexpr variable initializers, an issue manifests here
+ // where the cleanups are dropped prior to VarDecl::evaluateValue, which
+ // subsequently requires them. This has been done as a temporary workaround
+ // as it was not obvious how to produce.
+ if (Rec.isUnevaluated() ||
+ (Rec.isConstantEvaluated() &&
+ Rec.ExprContext != ExpressionKind::EK_ConstexprVarInit)) {
ExprCleanupObjects.erase(ExprCleanupObjects.begin() + Rec.NumCleanupObjects,
ExprCleanupObjects.end());
Cleanup = Rec.ParentCleanup;
Index: clang/lib/Sema/SemaDeclCXX.cpp
===================================================================
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -16746,6 +16746,19 @@
Diag(D->getLocation(), diag::err_illegal_initializer);
}
+/// Determine whether the given declaration is a variable
+/// which will be manifestly constant evaluated due to
+/// a constexpr specifier.
+static bool isManifestlyEvaluatedVar(Sema &SemaRef, const Decl *D) {
+ if (!SemaRef.getLangOpts().CPlusPlus2a)
+ return false;
+
+ if (const VarDecl *Var = dyn_cast_or_null<VarDecl>(D))
+ return Var->isConstexpr();
+
+ return false;
+}
+
/// Determine whether the given declaration is a global variable or
/// static data member.
static bool isNonlocalVariable(const Decl *D) {
@@ -16777,7 +16790,12 @@
// 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))
+ if (isManifestlyEvaluatedVar(*this, D)) {
+ using ExpressionKind = ExpressionEvaluationContextRecord::ExpressionKind;
+
+ PushExpressionEvaluationContext(
+ ExpressionEvaluationContext::ConstantEvaluated, D, ExpressionKind::EK_ConstexprVarInit);
+ } else if (isNonlocalVariable(D))
PushExpressionEvaluationContext(
ExpressionEvaluationContext::PotentiallyEvaluated, D);
}
@@ -16788,7 +16806,7 @@
if (!D || D->isInvalidDecl())
return;
- if (isNonlocalVariable(D))
+ if (isManifestlyEvaluatedVar(*this, D) || isNonlocalVariable(D))
PopExpressionEvaluationContext();
if (S && D->isOutOfLine())
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(&InitStmt, 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(&InitStmt, Cond, IfLoc,
+ IsConstexpr ? Sema::ConditionKind::ConstexprIf
+ : Sema::ConditionKind::Boolean))
+ return StmtError();
+ }
llvm::Optional<bool> ConstexprCondition;
if (IsConstexpr)
Index: clang/lib/Parse/ParseDeclCXX.cpp
===================================================================
--- clang/lib/Parse/ParseDeclCXX.cpp
+++ clang/lib/Parse/ParseDeclCXX.cpp
@@ -2962,11 +2962,18 @@
/// be a constant-expression.
ExprResult Parser::ParseCXXMemberInitializer(Decl *D, bool IsFunction,
SourceLocation &EqualLoc) {
+ 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<VarDecl>(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/include/clang/Sema/Sema.h
===================================================================
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -1139,7 +1139,7 @@
/// \brief Describes whether we are in an expression constext which we have
/// to handle differently.
enum ExpressionKind {
- EK_Decltype, EK_TemplateArgument, EK_Other
+ EK_Decltype, EK_TemplateArgument, EK_ConstexprVarInit, EK_Other
} ExprContext;
ExpressionEvaluationContextRecord(ExpressionEvaluationContext Context,
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
