[clang] aac588a - Reapply "[clang][Interp] Fix locals created in ExprWithCleanups"
Timm Bäder via cfe-commits
cfe-commits at lists.llvm.org
Thu May 2 07:56:52 PDT 2024
Author: Timm Bäder
Date: 2024-05-02T16:56:38+02:00
New Revision: aac588abfa03e9721d96546df917fec913137ecb
URL: https://github.com/llvm/llvm-project/commit/aac588abfa03e9721d96546df917fec913137ecb
DIFF: https://github.com/llvm/llvm-project/commit/aac588abfa03e9721d96546df917fec913137ecb.diff
LOG: Reapply "[clang][Interp] Fix locals created in ExprWithCleanups"
This reverts commit 427c5bfd39ebb9d008b621370579444fbf2a60d7.
Added:
Modified:
clang/lib/AST/Interp/ByteCodeExprGen.cpp
clang/lib/AST/Interp/ByteCodeExprGen.h
clang/test/AST/Interp/records.cpp
Removed:
################################################################################
diff --git a/clang/lib/AST/Interp/ByteCodeExprGen.cpp b/clang/lib/AST/Interp/ByteCodeExprGen.cpp
index 9fd6e2dd06991f..b9318fbd9ae9e6 100644
--- a/clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ b/clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -29,15 +29,11 @@ namespace interp {
template <class Emitter> class DeclScope final : public VariableScope<Emitter> {
public:
DeclScope(ByteCodeExprGen<Emitter> *Ctx, const ValueDecl *VD)
- : VariableScope<Emitter>(Ctx), Scope(Ctx->P, VD),
+ : VariableScope<Emitter>(Ctx, nullptr), Scope(Ctx->P, VD),
OldGlobalDecl(Ctx->GlobalDecl) {
Ctx->GlobalDecl = Context::shouldBeGloballyIndexed(VD);
}
- void addExtended(const Scope::Local &Local) override {
- return this->addLocal(Local);
- }
-
~DeclScope() { this->Ctx->GlobalDecl = OldGlobalDecl; }
private:
@@ -85,8 +81,7 @@ bool ByteCodeExprGen<Emitter>::VisitCastExpr(const CastExpr *CE) {
std::optional<PrimType> SubExprT = classify(SubExpr->getType());
// Prepare storage for the result.
if (!Initializing && !SubExprT) {
- std::optional<unsigned> LocalIndex =
- allocateLocal(SubExpr, /*IsExtended=*/false);
+ std::optional<unsigned> LocalIndex = allocateLocal(SubExpr);
if (!LocalIndex)
return false;
if (!this->emitGetPtrLocal(*LocalIndex, CE))
@@ -362,8 +357,7 @@ bool ByteCodeExprGen<Emitter>::VisitCastExpr(const CastExpr *CE) {
// We're creating a complex value here, so we need to
// allocate storage for it.
if (!Initializing) {
- std::optional<unsigned> LocalIndex =
- allocateLocal(CE, /*IsExtended=*/true);
+ std::optional<unsigned> LocalIndex = allocateLocal(CE);
if (!LocalIndex)
return false;
if (!this->emitGetPtrLocal(*LocalIndex, CE))
@@ -390,8 +384,7 @@ bool ByteCodeExprGen<Emitter>::VisitCastExpr(const CastExpr *CE) {
return this->discard(SubExpr);
if (!Initializing) {
- std::optional<unsigned> LocalIndex =
- allocateLocal(CE, /*IsExtended=*/true);
+ std::optional<unsigned> LocalIndex = allocateLocal(CE);
if (!LocalIndex)
return false;
if (!this->emitGetPtrLocal(*LocalIndex, CE))
@@ -492,7 +485,7 @@ bool ByteCodeExprGen<Emitter>::VisitImaginaryLiteral(
return true;
if (!Initializing) {
- std::optional<unsigned> LocalIndex = allocateLocal(E, /*IsExtended=*/false);
+ std::optional<unsigned> LocalIndex = allocateLocal(E);
if (!LocalIndex)
return false;
if (!this->emitGetPtrLocal(*LocalIndex, E))
@@ -561,7 +554,7 @@ bool ByteCodeExprGen<Emitter>::VisitBinaryOperator(const BinaryOperator *BO) {
// We need a temporary variable holding our return value.
if (!Initializing) {
- std::optional<unsigned> ResultIndex = this->allocateLocal(BO, false);
+ std::optional<unsigned> ResultIndex = this->allocateLocal(BO);
if (!this->emitGetPtrLocal(*ResultIndex, BO))
return false;
}
@@ -784,7 +777,7 @@ template <class Emitter>
bool ByteCodeExprGen<Emitter>::VisitComplexBinOp(const BinaryOperator *E) {
// Prepare storage for result.
if (!Initializing) {
- std::optional<unsigned> LocalIndex = allocateLocal(E, /*IsExtended=*/false);
+ std::optional<unsigned> LocalIndex = allocateLocal(E);
if (!LocalIndex)
return false;
if (!this->emitGetPtrLocal(*LocalIndex, E))
@@ -1841,11 +1834,12 @@ bool ByteCodeExprGen<Emitter>::VisitCompoundAssignOperator(
template <class Emitter>
bool ByteCodeExprGen<Emitter>::VisitExprWithCleanups(
const ExprWithCleanups *E) {
+ ExprScope<Emitter> ES(this);
const Expr *SubExpr = E->getSubExpr();
assert(E->getNumObjects() == 0 && "TODO: Implement cleanups");
- return this->delegate(SubExpr);
+ return this->delegate(SubExpr) && ES.destroyLocals();
}
template <class Emitter>
@@ -1910,9 +1904,8 @@ bool ByteCodeExprGen<Emitter>::VisitMaterializeTemporaryExpr(
return this->emitGetPtrLocal(LocalIndex, E);
} else {
const Expr *Inner = E->getSubExpr()->skipRValueSubobjectAdjustments();
-
if (std::optional<unsigned> LocalIndex =
- allocateLocal(Inner, /*IsExtended=*/true)) {
+ allocateLocal(Inner, E->getExtendingDecl())) {
if (!this->emitGetPtrLocal(*LocalIndex, E))
return false;
return this->visitInitializer(SubExpr);
@@ -2119,8 +2112,7 @@ bool ByteCodeExprGen<Emitter>::VisitCXXConstructExpr(
// to allocate a variable and call the constructor and destructor.
if (DiscardResult) {
assert(!Initializing);
- std::optional<unsigned> LocalIndex =
- allocateLocal(E, /*IsExtended=*/true);
+ std::optional<unsigned> LocalIndex = allocateLocal(E);
if (!LocalIndex)
return false;
@@ -2300,8 +2292,7 @@ bool ByteCodeExprGen<Emitter>::VisitCXXScalarValueInitExpr(
if (const auto *CT = Ty->getAs<ComplexType>()) {
if (!Initializing) {
- std::optional<unsigned> LocalIndex =
- allocateLocal(E, /*IsExtended=*/false);
+ std::optional<unsigned> LocalIndex = allocateLocal(E);
if (!LocalIndex)
return false;
if (!this->emitGetPtrLocal(*LocalIndex, E))
@@ -2324,8 +2315,7 @@ bool ByteCodeExprGen<Emitter>::VisitCXXScalarValueInitExpr(
if (const auto *VT = Ty->getAs<VectorType>()) {
// FIXME: Code duplication with the _Complex case above.
if (!Initializing) {
- std::optional<unsigned> LocalIndex =
- allocateLocal(E, /*IsExtended=*/false);
+ std::optional<unsigned> LocalIndex = allocateLocal(E);
if (!LocalIndex)
return false;
if (!this->emitGetPtrLocal(*LocalIndex, E))
@@ -2517,7 +2507,7 @@ template <class Emitter> bool ByteCodeExprGen<Emitter>::visit(const Expr *E) {
// Create local variable to hold the return value.
if (!E->isGLValue() && !E->getType()->isAnyComplexType() &&
!classify(E->getType())) {
- std::optional<unsigned> LocalIndex = allocateLocal(E, /*IsExtended=*/true);
+ std::optional<unsigned> LocalIndex = allocateLocal(E);
if (!LocalIndex)
return false;
@@ -2771,7 +2761,8 @@ unsigned ByteCodeExprGen<Emitter>::allocateLocalPrimitive(DeclTy &&Src,
template <class Emitter>
std::optional<unsigned>
-ByteCodeExprGen<Emitter>::allocateLocal(DeclTy &&Src, bool IsExtended) {
+ByteCodeExprGen<Emitter>::allocateLocal(DeclTy &&Src,
+ const ValueDecl *ExtendingDecl) {
// Make sure we don't accidentally register the same decl twice.
if ([[maybe_unused]] const auto *VD =
dyn_cast_if_present<ValueDecl>(Src.dyn_cast<const Decl *>())) {
@@ -2804,7 +2795,10 @@ ByteCodeExprGen<Emitter>::allocateLocal(DeclTy &&Src, bool IsExtended) {
Scope::Local Local = this->createLocal(D);
if (Key)
Locals.insert({Key, Local});
- VarScope->add(Local, IsExtended);
+ if (ExtendingDecl)
+ VarScope->addExtended(Local, ExtendingDecl);
+ else
+ VarScope->add(Local, false);
return Local.Offset;
}
@@ -2839,14 +2833,14 @@ bool ByteCodeExprGen<Emitter>::visitExpr(const Expr *E) {
if (E->getType()->isVoidType()) {
if (!visit(E))
return false;
- return this->emitRetVoid(E);
+ return this->emitRetVoid(E) && RootScope.destroyLocals();
}
// Expressions with a primitive return type.
if (std::optional<PrimType> T = classify(E)) {
if (!visit(E))
return false;
- return this->emitRet(*T, E);
+ return this->emitRet(*T, E) && RootScope.destroyLocals();
}
// Expressions with a composite return type.
@@ -2867,7 +2861,7 @@ bool ByteCodeExprGen<Emitter>::visitExpr(const Expr *E) {
return this->emitRetValue(E) && RootScope.destroyLocals();
}
- return false;
+ return RootScope.destroyLocals();
}
/// Toplevel visitDecl().
@@ -2958,7 +2952,7 @@ bool ByteCodeExprGen<Emitter>::visitVarDecl(const VarDecl *VD) {
return !Init || initGlobal(*GlobalIndex);
} else {
- VariableScope<Emitter> LocalScope(this);
+ VariableScope<Emitter> LocalScope(this, VD);
if (VarT) {
unsigned Offset = this->allocateLocalPrimitive(
VD, *VarT, VD->getType().isConstQualified());
@@ -2975,6 +2969,7 @@ bool ByteCodeExprGen<Emitter>::visitVarDecl(const VarDecl *VD) {
return !Init || this->visitLocalInitializer(Init, *Offset);
return false;
}
+
return true;
}
@@ -3056,7 +3051,7 @@ bool ByteCodeExprGen<Emitter>::VisitBuiltinCallExpr(const CallExpr *E) {
// Non-primitive return type. Prepare storage.
if (!Initializing && !ReturnT && !ReturnType->isVoidType()) {
- std::optional<unsigned> LocalIndex = allocateLocal(E, /*IsExtended=*/false);
+ std::optional<unsigned> LocalIndex = allocateLocal(E);
if (!LocalIndex)
return false;
if (!this->emitGetPtrLocal(*LocalIndex, E))
@@ -3470,8 +3465,7 @@ bool ByteCodeExprGen<Emitter>::VisitComplexUnaryOperator(
std::optional<PrimType> ResT = classify(E);
auto prepareResult = [=]() -> bool {
if (!ResT && !Initializing) {
- std::optional<unsigned> LocalIndex =
- allocateLocal(SubExpr, /*IsExtended=*/false);
+ std::optional<unsigned> LocalIndex = allocateLocal(SubExpr);
if (!LocalIndex)
return false;
return this->emitGetPtrLocal(*LocalIndex, E);
diff --git a/clang/lib/AST/Interp/ByteCodeExprGen.h b/clang/lib/AST/Interp/ByteCodeExprGen.h
index 0cd8cf1cb397a9..9f83d173bbae3a 100644
--- a/clang/lib/AST/Interp/ByteCodeExprGen.h
+++ b/clang/lib/AST/Interp/ByteCodeExprGen.h
@@ -235,7 +235,8 @@ class ByteCodeExprGen : public ConstStmtVisitor<ByteCodeExprGen<Emitter>, bool>,
bool IsExtended = false);
/// Allocates a space storing a local given its type.
- std::optional<unsigned> allocateLocal(DeclTy &&Decl, bool IsExtended = false);
+ std::optional<unsigned>
+ allocateLocal(DeclTy &&Decl, const ValueDecl *ExtendingDecl = nullptr);
private:
friend class VariableScope<Emitter>;
@@ -322,8 +323,8 @@ extern template class ByteCodeExprGen<EvalEmitter>;
/// Scope chain managing the variable lifetimes.
template <class Emitter> class VariableScope {
public:
- VariableScope(ByteCodeExprGen<Emitter> *Ctx)
- : Ctx(Ctx), Parent(Ctx->VarScope) {
+ VariableScope(ByteCodeExprGen<Emitter> *Ctx, const ValueDecl *VD)
+ : Ctx(Ctx), Parent(Ctx->VarScope), ValDecl(VD) {
Ctx->VarScope = this;
}
@@ -346,6 +347,24 @@ template <class Emitter> class VariableScope {
this->Parent->addExtended(Local);
}
+ void addExtended(const Scope::Local &Local, const ValueDecl *ExtendingDecl) {
+ // Walk up the chain of scopes until we find the one for ExtendingDecl.
+ // If there is no such scope, attach it to the parent one.
+ VariableScope *P = this;
+ while (P) {
+ if (P->ValDecl == ExtendingDecl) {
+ P->addLocal(Local);
+ return;
+ }
+ P = P->Parent;
+ if (!P)
+ break;
+ }
+
+ // Use the parent scope.
+ addExtended(Local);
+ }
+
virtual void emitDestruction() {}
virtual bool emitDestructors() { return true; }
VariableScope *getParent() const { return Parent; }
@@ -355,12 +374,14 @@ template <class Emitter> class VariableScope {
ByteCodeExprGen<Emitter> *Ctx;
/// Link to the parent scope.
VariableScope *Parent;
+ const ValueDecl *ValDecl = nullptr;
};
/// Generic scope for local variables.
template <class Emitter> class LocalScope : public VariableScope<Emitter> {
public:
- LocalScope(ByteCodeExprGen<Emitter> *Ctx) : VariableScope<Emitter>(Ctx) {}
+ LocalScope(ByteCodeExprGen<Emitter> *Ctx)
+ : VariableScope<Emitter>(Ctx, nullptr) {}
/// Emit a Destroy op for this scope.
~LocalScope() override {
@@ -474,16 +495,9 @@ template <class Emitter> class BlockScope final : public AutoScope<Emitter> {
}
};
-/// Expression scope which tracks potentially lifetime extended
-/// temporaries which are hoisted to the parent scope on exit.
template <class Emitter> class ExprScope final : public AutoScope<Emitter> {
public:
ExprScope(ByteCodeExprGen<Emitter> *Ctx) : AutoScope<Emitter>(Ctx) {}
-
- void addExtended(const Scope::Local &Local) override {
- if (this->Parent)
- this->Parent->addLocal(Local);
- }
};
template <class Emitter> class ArrayIndexScope final {
diff --git a/clang/test/AST/Interp/records.cpp b/clang/test/AST/Interp/records.cpp
index 26efd6896a9f7e..41be9b71a27f90 100644
--- a/clang/test/AST/Interp/records.cpp
+++ b/clang/test/AST/Interp/records.cpp
@@ -1444,3 +1444,18 @@ namespace {
static_assert(waldo == 4, "");
#endif
}
+
+
+namespace TemporaryWithInvalidDestructor {
+#if __cplusplus >= 202002L
+ struct A {
+ bool a = true;
+ constexpr ~A() noexcept(false) { // both-error {{never produces a constant expression}}
+ throw; // both-note 2{{not valid in a constant expression}} \
+ // both-error {{cannot use 'throw' with exceptions disabled}}
+ }
+ };
+ static_assert(A().a, ""); // both-error {{not an integral constant expression}} \
+ // both-note {{in call to}}
+#endif
+}
More information about the cfe-commits
mailing list