[clang] 427c5bf - Revert "[clang][Interp] Fix locals created in ExprWithCleanups"
Timm Bäder via cfe-commits
cfe-commits at lists.llvm.org
Thu May 2 05:04:07 PDT 2024
Author: Timm Bäder
Date: 2024-05-02T14:03:48+02:00
New Revision: 427c5bfd39ebb9d008b621370579444fbf2a60d7
URL: https://github.com/llvm/llvm-project/commit/427c5bfd39ebb9d008b621370579444fbf2a60d7
DIFF: https://github.com/llvm/llvm-project/commit/427c5bfd39ebb9d008b621370579444fbf2a60d7.diff
LOG: Revert "[clang][Interp] Fix locals created in ExprWithCleanups"
This reverts commit 1c7673b91d4d3bab4e296f5c67751d3879fb21a2.
Unfortunately, this one is broken because de04e6cd90b891215f1dfc83ec886d037a7c2ed0
has been reverted.
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 30627e6300ca22..b096d4ddd88246 100644
--- a/clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ b/clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -29,11 +29,15 @@ namespace interp {
template <class Emitter> class DeclScope final : public VariableScope<Emitter> {
public:
DeclScope(ByteCodeExprGen<Emitter> *Ctx, const ValueDecl *VD)
- : VariableScope<Emitter>(Ctx, nullptr), Scope(Ctx->P, VD),
+ : VariableScope<Emitter>(Ctx), 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:
@@ -81,7 +85,8 @@ 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);
+ std::optional<unsigned> LocalIndex =
+ allocateLocal(SubExpr, /*IsExtended=*/false);
if (!LocalIndex)
return false;
if (!this->emitGetPtrLocal(*LocalIndex, CE))
@@ -357,7 +362,8 @@ 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);
+ std::optional<unsigned> LocalIndex =
+ allocateLocal(CE, /*IsExtended=*/true);
if (!LocalIndex)
return false;
if (!this->emitGetPtrLocal(*LocalIndex, CE))
@@ -384,7 +390,8 @@ bool ByteCodeExprGen<Emitter>::VisitCastExpr(const CastExpr *CE) {
return this->discard(SubExpr);
if (!Initializing) {
- std::optional<unsigned> LocalIndex = allocateLocal(CE);
+ std::optional<unsigned> LocalIndex =
+ allocateLocal(CE, /*IsExtended=*/true);
if (!LocalIndex)
return false;
if (!this->emitGetPtrLocal(*LocalIndex, CE))
@@ -485,7 +492,7 @@ bool ByteCodeExprGen<Emitter>::VisitImaginaryLiteral(
return true;
if (!Initializing) {
- std::optional<unsigned> LocalIndex = allocateLocal(E);
+ std::optional<unsigned> LocalIndex = allocateLocal(E, /*IsExtended=*/false);
if (!LocalIndex)
return false;
if (!this->emitGetPtrLocal(*LocalIndex, E))
@@ -554,7 +561,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);
+ std::optional<unsigned> ResultIndex = this->allocateLocal(BO, false);
if (!this->emitGetPtrLocal(*ResultIndex, BO))
return false;
}
@@ -777,7 +784,7 @@ template <class Emitter>
bool ByteCodeExprGen<Emitter>::VisitComplexBinOp(const BinaryOperator *E) {
// Prepare storage for result.
if (!Initializing) {
- std::optional<unsigned> LocalIndex = allocateLocal(E);
+ std::optional<unsigned> LocalIndex = allocateLocal(E, /*IsExtended=*/false);
if (!LocalIndex)
return false;
if (!this->emitGetPtrLocal(*LocalIndex, E))
@@ -1834,12 +1841,11 @@ 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) && ES.destroyLocals();
+ return this->delegate(SubExpr);
}
template <class Emitter>
@@ -1904,8 +1910,9 @@ bool ByteCodeExprGen<Emitter>::VisitMaterializeTemporaryExpr(
return this->emitGetPtrLocal(LocalIndex, E);
} else {
const Expr *Inner = E->getSubExpr()->skipRValueSubobjectAdjustments();
+
if (std::optional<unsigned> LocalIndex =
- allocateLocal(Inner, E->getExtendingDecl())) {
+ allocateLocal(Inner, /*IsExtended=*/true)) {
if (!this->emitGetPtrLocal(*LocalIndex, E))
return false;
return this->visitInitializer(SubExpr);
@@ -2112,7 +2119,8 @@ bool ByteCodeExprGen<Emitter>::VisitCXXConstructExpr(
// to allocate a variable and call the constructor and destructor.
if (DiscardResult) {
assert(!Initializing);
- std::optional<unsigned> LocalIndex = allocateLocal(E);
+ std::optional<unsigned> LocalIndex =
+ allocateLocal(E, /*IsExtended=*/true);
if (!LocalIndex)
return false;
@@ -2292,7 +2300,8 @@ bool ByteCodeExprGen<Emitter>::VisitCXXScalarValueInitExpr(
if (const auto *CT = Ty->getAs<ComplexType>()) {
if (!Initializing) {
- std::optional<unsigned> LocalIndex = allocateLocal(E);
+ std::optional<unsigned> LocalIndex =
+ allocateLocal(E, /*IsExtended=*/false);
if (!LocalIndex)
return false;
if (!this->emitGetPtrLocal(*LocalIndex, E))
@@ -2315,7 +2324,8 @@ 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);
+ std::optional<unsigned> LocalIndex =
+ allocateLocal(E, /*IsExtended=*/false);
if (!LocalIndex)
return false;
if (!this->emitGetPtrLocal(*LocalIndex, E))
@@ -2503,7 +2513,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);
+ std::optional<unsigned> LocalIndex = allocateLocal(E, /*IsExtended=*/true);
if (!LocalIndex)
return false;
@@ -2757,8 +2767,7 @@ unsigned ByteCodeExprGen<Emitter>::allocateLocalPrimitive(DeclTy &&Src,
template <class Emitter>
std::optional<unsigned>
-ByteCodeExprGen<Emitter>::allocateLocal(DeclTy &&Src,
- const ValueDecl *ExtendingDecl) {
+ByteCodeExprGen<Emitter>::allocateLocal(DeclTy &&Src, bool IsExtended) {
// 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 *>())) {
@@ -2791,10 +2800,7 @@ ByteCodeExprGen<Emitter>::allocateLocal(DeclTy &&Src,
Scope::Local Local = this->createLocal(D);
if (Key)
Locals.insert({Key, Local});
- if (ExtendingDecl)
- VarScope->addExtended(Local, ExtendingDecl);
- else
- VarScope->add(Local, false);
+ VarScope->add(Local, IsExtended);
return Local.Offset;
}
@@ -2829,14 +2835,14 @@ bool ByteCodeExprGen<Emitter>::visitExpr(const Expr *E) {
if (E->getType()->isVoidType()) {
if (!visit(E))
return false;
- return this->emitRetVoid(E) && RootScope.destroyLocals();
+ return this->emitRetVoid(E);
}
// Expressions with a primitive return type.
if (std::optional<PrimType> T = classify(E)) {
if (!visit(E))
return false;
- return this->emitRet(*T, E) && RootScope.destroyLocals();
+ return this->emitRet(*T, E);
}
// Expressions with a composite return type.
@@ -2857,7 +2863,7 @@ bool ByteCodeExprGen<Emitter>::visitExpr(const Expr *E) {
return this->emitRetValue(E) && RootScope.destroyLocals();
}
- return RootScope.destroyLocals();
+ return false;
}
/// Toplevel visitDecl().
@@ -2948,7 +2954,7 @@ bool ByteCodeExprGen<Emitter>::visitVarDecl(const VarDecl *VD) {
return !Init || initGlobal(*GlobalIndex);
} else {
- VariableScope<Emitter> LocalScope(this, VD);
+ VariableScope<Emitter> LocalScope(this);
if (VarT) {
unsigned Offset = this->allocateLocalPrimitive(
VD, *VarT, VD->getType().isConstQualified());
@@ -2965,7 +2971,6 @@ bool ByteCodeExprGen<Emitter>::visitVarDecl(const VarDecl *VD) {
return !Init || this->visitLocalInitializer(Init, *Offset);
return false;
}
-
return true;
}
@@ -3047,7 +3052,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);
+ std::optional<unsigned> LocalIndex = allocateLocal(E, /*IsExtended=*/false);
if (!LocalIndex)
return false;
if (!this->emitGetPtrLocal(*LocalIndex, E))
@@ -3461,7 +3466,8 @@ bool ByteCodeExprGen<Emitter>::VisitComplexUnaryOperator(
std::optional<PrimType> ResT = classify(E);
auto prepareResult = [=]() -> bool {
if (!ResT && !Initializing) {
- std::optional<unsigned> LocalIndex = allocateLocal(SubExpr);
+ std::optional<unsigned> LocalIndex =
+ allocateLocal(SubExpr, /*IsExtended=*/false);
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 9f83d173bbae3a..0cd8cf1cb397a9 100644
--- a/clang/lib/AST/Interp/ByteCodeExprGen.h
+++ b/clang/lib/AST/Interp/ByteCodeExprGen.h
@@ -235,8 +235,7 @@ 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, const ValueDecl *ExtendingDecl = nullptr);
+ std::optional<unsigned> allocateLocal(DeclTy &&Decl, bool IsExtended = false);
private:
friend class VariableScope<Emitter>;
@@ -323,8 +322,8 @@ extern template class ByteCodeExprGen<EvalEmitter>;
/// Scope chain managing the variable lifetimes.
template <class Emitter> class VariableScope {
public:
- VariableScope(ByteCodeExprGen<Emitter> *Ctx, const ValueDecl *VD)
- : Ctx(Ctx), Parent(Ctx->VarScope), ValDecl(VD) {
+ VariableScope(ByteCodeExprGen<Emitter> *Ctx)
+ : Ctx(Ctx), Parent(Ctx->VarScope) {
Ctx->VarScope = this;
}
@@ -347,24 +346,6 @@ 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; }
@@ -374,14 +355,12 @@ 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, nullptr) {}
+ LocalScope(ByteCodeExprGen<Emitter> *Ctx) : VariableScope<Emitter>(Ctx) {}
/// Emit a Destroy op for this scope.
~LocalScope() override {
@@ -495,9 +474,16 @@ 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 41be9b71a27f90..26efd6896a9f7e 100644
--- a/clang/test/AST/Interp/records.cpp
+++ b/clang/test/AST/Interp/records.cpp
@@ -1444,18 +1444,3 @@ 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