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 -} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
