Author: Timm Bäder Date: 2024-05-02T13:51:50+02:00 New Revision: 1c7673b91d4d3bab4e296f5c67751d3879fb21a2
URL: https://github.com/llvm/llvm-project/commit/1c7673b91d4d3bab4e296f5c67751d3879fb21a2 DIFF: https://github.com/llvm/llvm-project/commit/1c7673b91d4d3bab4e296f5c67751d3879fb21a2.diff LOG: [clang][Interp] Fix locals created in ExprWithCleanups Save the declaration we're creating a scope for (if applicable), so we can attach a local temporary variable to the right scope. 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 b096d4ddd88246..30627e6300ca22 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)) @@ -2513,7 +2503,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; @@ -2767,7 +2757,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 *>())) { @@ -2800,7 +2791,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; } @@ -2835,14 +2829,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. @@ -2863,7 +2857,7 @@ bool ByteCodeExprGen<Emitter>::visitExpr(const Expr *E) { return this->emitRetValue(E) && RootScope.destroyLocals(); } - return false; + return RootScope.destroyLocals(); } /// Toplevel visitDecl(). @@ -2954,7 +2948,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()); @@ -2971,6 +2965,7 @@ bool ByteCodeExprGen<Emitter>::visitVarDecl(const VarDecl *VD) { return !Init || this->visitLocalInitializer(Init, *Offset); return false; } + return true; } @@ -3052,7 +3047,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)) @@ -3466,8 +3461,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 +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
