Timm =?utf-8?q?Bäder?= <tbae...@redhat.com> Message-ID: In-Reply-To: <llvm.org/llvm/llvm-project/pull/101...@github.com>
llvmbot wrote: <!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Timm Baeder (tbaederr) <details> <summary>Changes</summary> --- Full diff: https://github.com/llvm/llvm-project/pull/101788.diff 9 Files Affected: - (modified) clang/lib/AST/Interp/Compiler.cpp (+15-48) - (modified) clang/lib/AST/Interp/Compiler.h (+20-24) - (modified) clang/lib/AST/Interp/Interp.h (+5) - (modified) clang/lib/AST/Interp/InterpFrame.cpp (+11-3) - (modified) clang/lib/AST/Interp/InterpFrame.h (+1) - (modified) clang/lib/AST/Interp/Opcodes.td (+3) - (modified) clang/lib/AST/Interp/Source.h (+9-4) - (modified) clang/test/AST/Interp/if.cpp (+18) - (modified) clang/test/AST/Interp/loops.cpp (+28) ``````````diff diff --git a/clang/lib/AST/Interp/Compiler.cpp b/clang/lib/AST/Interp/Compiler.cpp index 258e4ed645254..f600d9b5b80f8 100644 --- a/clang/lib/AST/Interp/Compiler.cpp +++ b/clang/lib/AST/Interp/Compiler.cpp @@ -2226,7 +2226,7 @@ bool Compiler<Emitter>::VisitExprWithCleanups(const ExprWithCleanups *E) { assert(E->getNumObjects() == 0 && "TODO: Implement cleanups"); - return this->delegate(SubExpr) && ES.destroyLocals(); + return this->delegate(SubExpr) && ES.destroyLocals(E); } template <class Emitter> @@ -2537,13 +2537,8 @@ bool Compiler<Emitter>::VisitCXXConstructExpr(const CXXConstructExpr *E) { return false; } - // Immediately call the destructor if we have to. - if (DiscardResult) { - if (!this->emitRecordDestruction(getRecord(E->getType()))) - return false; - if (!this->emitPopPtr(E)) - return false; - } + if (DiscardResult) + return this->emitPopPtr(E); return true; } @@ -4222,22 +4217,6 @@ template <class Emitter> bool Compiler<Emitter>::visitStmt(const Stmt *S) { } } -/// Visits the given statment without creating a variable -/// scope for it in case it is a compound statement. -template <class Emitter> bool Compiler<Emitter>::visitLoopBody(const Stmt *S) { - if (isa<NullStmt>(S)) - return true; - - if (const auto *CS = dyn_cast<CompoundStmt>(S)) { - for (const auto *InnerStmt : CS->body()) - if (!visitStmt(InnerStmt)) - return false; - return true; - } - - return this->visitStmt(S); -} - template <class Emitter> bool Compiler<Emitter>::visitCompoundStmt(const CompoundStmt *S) { BlockScope<Emitter> Scope(this); @@ -4300,8 +4279,6 @@ bool Compiler<Emitter>::visitReturnStmt(const ReturnStmt *RS) { } template <class Emitter> bool Compiler<Emitter>::visitIfStmt(const IfStmt *IS) { - BlockScope<Emitter> IfScope(this); - if (IS->isNonNegatedConsteval()) return visitStmt(IS->getThen()); if (IS->isNegatedConsteval()) @@ -4340,7 +4317,7 @@ template <class Emitter> bool Compiler<Emitter>::visitIfStmt(const IfStmt *IS) { this->emitLabel(LabelEnd); } - return IfScope.destroyLocals(); + return true; } template <class Emitter> @@ -4364,12 +4341,8 @@ bool Compiler<Emitter>::visitWhileStmt(const WhileStmt *S) { if (!this->jumpFalse(EndLabel)) return false; - LocalScope<Emitter> Scope(this); - { - DestructorScope<Emitter> DS(Scope); - if (!this->visitLoopBody(Body)) - return false; - } + if (!this->visitStmt(Body)) + return false; if (!this->jump(CondLabel)) return false; @@ -4387,14 +4360,11 @@ template <class Emitter> bool Compiler<Emitter>::visitDoStmt(const DoStmt *S) { LabelTy EndLabel = this->getLabel(); LabelTy CondLabel = this->getLabel(); LoopScope<Emitter> LS(this, EndLabel, CondLabel); - LocalScope<Emitter> Scope(this); this->fallthrough(StartLabel); this->emitLabel(StartLabel); { - DestructorScope<Emitter> DS(Scope); - - if (!this->visitLoopBody(Body)) + if (!this->visitStmt(Body)) return false; this->fallthrough(CondLabel); this->emitLabel(CondLabel); @@ -4421,10 +4391,10 @@ bool Compiler<Emitter>::visitForStmt(const ForStmt *S) { LabelTy CondLabel = this->getLabel(); LabelTy IncLabel = this->getLabel(); LoopScope<Emitter> LS(this, EndLabel, IncLabel); - LocalScope<Emitter> Scope(this); if (Init && !this->visitStmt(Init)) return false; + this->fallthrough(CondLabel); this->emitLabel(CondLabel); @@ -4440,10 +4410,9 @@ bool Compiler<Emitter>::visitForStmt(const ForStmt *S) { } { - DestructorScope<Emitter> DS(Scope); - - if (Body && !this->visitLoopBody(Body)) + if (Body && !this->visitStmt(Body)) return false; + this->fallthrough(IncLabel); this->emitLabel(IncLabel); if (Inc && !this->discard(Inc)) @@ -4495,13 +4464,11 @@ bool Compiler<Emitter>::visitCXXForRangeStmt(const CXXForRangeStmt *S) { return false; // Body. - LocalScope<Emitter> Scope(this); { - DestructorScope<Emitter> DS(Scope); - - if (!this->visitLoopBody(Body)) + if (!this->visitStmt(Body)) return false; - this->fallthrough(IncLabel); + + this->fallthrough(IncLabel); this->emitLabel(IncLabel); if (!this->discard(Inc)) return false; @@ -4520,7 +4487,7 @@ bool Compiler<Emitter>::visitBreakStmt(const BreakStmt *S) { if (!BreakLabel) return false; - this->VarScope->emitDestructors(); + this->emitCleanup(); return this->jump(*BreakLabel); } @@ -4529,7 +4496,7 @@ bool Compiler<Emitter>::visitContinueStmt(const ContinueStmt *S) { if (!ContinueLabel) return false; - this->VarScope->emitDestructors(); + this->emitCleanup(); return this->jump(*ContinueLabel); } diff --git a/clang/lib/AST/Interp/Compiler.h b/clang/lib/AST/Interp/Compiler.h index 6bc9985fe7232..eabde051b4278 100644 --- a/clang/lib/AST/Interp/Compiler.h +++ b/clang/lib/AST/Interp/Compiler.h @@ -202,7 +202,6 @@ class Compiler : public ConstStmtVisitor<Compiler<Emitter>, bool>, // Statements. bool visitCompoundStmt(const CompoundStmt *S); - bool visitLoopBody(const Stmt *S); bool visitDeclStmt(const DeclStmt *DS); bool visitReturnStmt(const ReturnStmt *RS); bool visitIfStmt(const IfStmt *IS); @@ -452,11 +451,15 @@ template <class Emitter> class VariableScope { } // Use the parent scope. - addExtended(Local); + if (this->Parent) + this->Parent->addLocal(Local); + else + this->addLocal(Local); } virtual void emitDestruction() {} - virtual bool emitDestructors() { return true; } + virtual bool emitDestructors(const Expr *E = nullptr) { return true; } + virtual bool destroyLocals(const Expr *E = nullptr) { return true; } VariableScope *getParent() const { return Parent; } protected: @@ -483,16 +486,21 @@ template <class Emitter> class LocalScope : public VariableScope<Emitter> { } /// Overriden to support explicit destruction. - void emitDestruction() override { destroyLocals(); } + void emitDestruction() override { + if (!Idx) + return; + + this->emitDestructors(); + this->Ctx->emitDestroy(*Idx, SourceInfo{}); + } /// Explicit destruction of local variables. - bool destroyLocals() { + bool destroyLocals(const Expr *E = nullptr) override { if (!Idx) return true; - bool Success = this->emitDestructors(); - this->Ctx->emitDestroy(*Idx, SourceInfo{}); - removeStoredOpaqueValues(); + bool Success = this->emitDestructors(E); + this->Ctx->emitDestroy(*Idx, E); this->Idx = std::nullopt; return Success; } @@ -501,25 +509,26 @@ template <class Emitter> class LocalScope : public VariableScope<Emitter> { if (!Idx) { Idx = this->Ctx->Descriptors.size(); this->Ctx->Descriptors.emplace_back(); + this->Ctx->emitInitScope(*Idx, {}); } this->Ctx->Descriptors[*Idx].emplace_back(Local); } - bool emitDestructors() override { + bool emitDestructors(const Expr *E = nullptr) override { if (!Idx) return true; // Emit destructor calls for local variables of record // type with a destructor. for (Scope::Local &Local : this->Ctx->Descriptors[*Idx]) { if (!Local.Desc->isPrimitive() && !Local.Desc->isPrimitiveArray()) { - if (!this->Ctx->emitGetPtrLocal(Local.Offset, SourceInfo{})) + if (!this->Ctx->emitGetPtrLocal(Local.Offset, E)) return false; if (!this->Ctx->emitDestruction(Local.Desc)) return false; - if (!this->Ctx->emitPopPtr(SourceInfo{})) + if (!this->Ctx->emitPopPtr(E)) return false; removeIfStoredOpaqueValue(Local); } @@ -549,19 +558,6 @@ template <class Emitter> class LocalScope : public VariableScope<Emitter> { std::optional<unsigned> Idx; }; -/// Emits the destructors of the variables of \param OtherScope -/// when this scope is destroyed. Does not create a Scope in the bytecode at -/// all, this is just a RAII object to emit destructors. -template <class Emitter> class DestructorScope final { -public: - DestructorScope(LocalScope<Emitter> &OtherScope) : OtherScope(OtherScope) {} - - ~DestructorScope() { OtherScope.emitDestructors(); } - -private: - LocalScope<Emitter> &OtherScope; -}; - /// Scope for storage declared in a compound statement. template <class Emitter> class BlockScope final : public LocalScope<Emitter> { public: diff --git a/clang/lib/AST/Interp/Interp.h b/clang/lib/AST/Interp/Interp.h index 63e9966b831db..b54635b9988e2 100644 --- a/clang/lib/AST/Interp/Interp.h +++ b/clang/lib/AST/Interp/Interp.h @@ -2012,6 +2012,11 @@ inline bool Destroy(InterpState &S, CodePtr OpPC, uint32_t I) { return true; } +inline bool InitScope(InterpState &S, CodePtr OpPC, uint32_t I) { + S.Current->initScope(I); + return true; +} + //===----------------------------------------------------------------------===// // Cast, CastFP //===----------------------------------------------------------------------===// diff --git a/clang/lib/AST/Interp/InterpFrame.cpp b/clang/lib/AST/Interp/InterpFrame.cpp index 1c37450ae1c6e..1925ffeabb1d7 100644 --- a/clang/lib/AST/Interp/InterpFrame.cpp +++ b/clang/lib/AST/Interp/InterpFrame.cpp @@ -37,9 +37,9 @@ InterpFrame::InterpFrame(InterpState &S, const Function *Func, Locals = std::make_unique<char[]>(FrameSize); for (auto &Scope : Func->scopes()) { for (auto &Local : Scope.locals()) { - Block *B = - new (localBlock(Local.Offset)) Block(S.Ctx.getEvalID(), Local.Desc); - B->invokeCtor(); + new (localBlock(Local.Offset)) Block(S.Ctx.getEvalID(), Local.Desc); + // Note that we are NOT calling invokeCtor() here, since that is done + // via the InitScope op. new (localInlineDesc(Local.Offset)) InlineDescriptor(Local.Desc); } } @@ -83,6 +83,14 @@ InterpFrame::~InterpFrame() { } } +void InterpFrame::initScope(unsigned Idx) { + if (!Func) + return; + for (auto &Local : Func->getScope(Idx).locals()) { + localBlock(Local.Offset)->invokeCtor(); + } +} + void InterpFrame::destroy(unsigned Idx) { for (auto &Local : Func->getScope(Idx).locals()) { S.deallocate(localBlock(Local.Offset)); diff --git a/clang/lib/AST/Interp/InterpFrame.h b/clang/lib/AST/Interp/InterpFrame.h index 4a312a71bcf1c..91b9b41b5d334 100644 --- a/clang/lib/AST/Interp/InterpFrame.h +++ b/clang/lib/AST/Interp/InterpFrame.h @@ -44,6 +44,7 @@ class InterpFrame final : public Frame { /// Invokes the destructors for a scope. void destroy(unsigned Idx); + void initScope(unsigned Idx); /// Pops the arguments off the stack. void popArgs(); diff --git a/clang/lib/AST/Interp/Opcodes.td b/clang/lib/AST/Interp/Opcodes.td index eeb9cb2e933a6..3e830f89754dc 100644 --- a/clang/lib/AST/Interp/Opcodes.td +++ b/clang/lib/AST/Interp/Opcodes.td @@ -232,6 +232,9 @@ def Destroy : Opcode { let Args = [ArgUint32]; let HasCustomEval = 1; } +def InitScope : Opcode { + let Args = [ArgUint32]; +} //===----------------------------------------------------------------------===// // Constants diff --git a/clang/lib/AST/Interp/Source.h b/clang/lib/AST/Interp/Source.h index c28b488ff554d..88b5ec7740df5 100644 --- a/clang/lib/AST/Interp/Source.h +++ b/clang/lib/AST/Interp/Source.h @@ -29,7 +29,7 @@ class Function; /// Pointer into the code segment. class CodePtr final { public: - CodePtr() : Ptr(nullptr) {} + CodePtr() = default; CodePtr &operator+=(int32_t Offset) { Ptr += Offset; @@ -45,11 +45,16 @@ class CodePtr final { assert(Ptr != nullptr && "Invalid code pointer"); return CodePtr(Ptr - RHS); } + CodePtr operator+(ssize_t RHS) const { + assert(Ptr != nullptr && "Invalid code pointer"); + return CodePtr(Ptr + RHS); + } bool operator!=(const CodePtr &RHS) const { return Ptr != RHS.Ptr; } const std::byte *operator*() const { return Ptr; } - - operator bool() const { return Ptr; } + explicit operator bool() const { return Ptr; } + bool operator<=(const CodePtr &RHS) const { return Ptr <= RHS.Ptr; } + bool operator>=(const CodePtr &RHS) const { return Ptr >= RHS.Ptr; } /// Reads data and advances the pointer. template <typename T> std::enable_if_t<!std::is_pointer<T>::value, T> read() { @@ -65,7 +70,7 @@ class CodePtr final { /// Constructor used by Function to generate pointers. CodePtr(const std::byte *Ptr) : Ptr(Ptr) {} /// Pointer into the code owned by a function. - const std::byte *Ptr; + const std::byte *Ptr = nullptr; }; /// Describes the statement/declaration an opcode was generated from. diff --git a/clang/test/AST/Interp/if.cpp b/clang/test/AST/Interp/if.cpp index 37289d69d3255..540cb76fbaac3 100644 --- a/clang/test/AST/Interp/if.cpp +++ b/clang/test/AST/Interp/if.cpp @@ -58,3 +58,21 @@ constexpr char g(char const (&x)[2]) { ; } static_assert(g("x") == 'x'); + +namespace IfScope { + struct Inc { + int &a; + constexpr Inc(int &a) : a(a) {} + constexpr ~Inc() { ++a; } + }; + + constexpr int foo() { + int a= 0; + int b = 12; + if (Inc{a}; true) { + b += a; + } + return b; + } + static_assert(foo() == 13, ""); +} diff --git a/clang/test/AST/Interp/loops.cpp b/clang/test/AST/Interp/loops.cpp index 2e235123af76e..38ab5613e1cbd 100644 --- a/clang/test/AST/Interp/loops.cpp +++ b/clang/test/AST/Interp/loops.cpp @@ -324,3 +324,31 @@ namespace RangeForLoop { // ref-note {{semicolon on a separate line}} } } + +namespace Scopes { + constexpr int foo() { + int n = 0; + { + int m = 12; + for (int i = 0;i < 10;++i) { + + { + int a = 10; + { + int b = 20; + { + int c = 30; + continue; + } + } + } + } + ++m; + n = m; + } + + ++n; + return n; + } + static_assert(foo() == 14, ""); +} `````````` </details> https://github.com/llvm/llvm-project/pull/101788 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
