diff --git a/clang/lib/AST/Interp/ByteCodeExprGen.h b/clang/lib/AST/Interp/ByteCodeExprGen.h --- a/clang/lib/AST/Interp/ByteCodeExprGen.h +++ b/clang/lib/AST/Interp/ByteCodeExprGen.h @@ -29,6 +29,7 @@ namespace interp { template class LocalScope; +template class DestructorScope; template class RecordScope; template class VariableScope; template class DeclScope; @@ -189,6 +190,7 @@ private: friend class VariableScope; friend class LocalScope; + friend class DestructorScope; friend class RecordScope; friend class DeclScope; friend class OptionScope; @@ -256,6 +258,8 @@ return FPO.getRoundingMode(); } + bool emitRecordDestruction(const Descriptor *Desc); + protected: /// Variable to storage mapping. llvm::DenseMap Locals; @@ -304,7 +308,7 @@ } virtual void emitDestruction() {} - + virtual void emitDestructors() {} VariableScope *getParent() const { return Parent; } protected: @@ -314,15 +318,26 @@ VariableScope *Parent; }; -/// Scope for local variables. -/// -/// When the scope is destroyed, instructions are emitted to tear down -/// all variables declared in this scope. +/// Generic scope for local variables. template class LocalScope : public VariableScope { public: LocalScope(ByteCodeExprGen *Ctx) : VariableScope(Ctx) {} - ~LocalScope() override { this->emitDestruction(); } + /// Emit a Destroy op for this scope. + ~LocalScope() override { + if (!Idx) + return; + this->Ctx->emitDestroy(*Idx, SourceInfo{}); + } + + /// Overriden to support explicit destruction. + void emitDestruction() override { + if (!Idx) + return; + this->emitDestructors(); + this->Ctx->emitDestroy(*Idx, SourceInfo{}); + this->Idx = std::nullopt; + } void addLocal(const Scope::Local &Local) override { if (!Idx) { @@ -333,21 +348,51 @@ this->Ctx->Descriptors[*Idx].emplace_back(Local); } - void emitDestruction() override { + void emitDestructors() override { if (!Idx) return; - this->Ctx->emitDestroy(*Idx, SourceInfo{}); + // 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()) { + this->Ctx->emitGetPtrLocal(Local.Offset, SourceInfo{}); + this->Ctx->emitRecordDestruction(Local.Desc); + } + } } -protected: /// Index of the scope in the chain. std::optional 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 DestructorScope final { +public: + DestructorScope(LocalScope &OtherScope) : OtherScope(OtherScope) {} + + ~DestructorScope() { OtherScope.emitDestructors(); } + +private: + LocalScope &OtherScope; +}; + +/// Like a regular LocalScope, except that the destructors of all local +/// variables are automatically emitted when the AutoScope is destroyed. +template class AutoScope : public LocalScope { +public: + AutoScope(ByteCodeExprGen *Ctx) + : LocalScope(Ctx), DS(*this) {} + +private: + DestructorScope DS; +}; + /// Scope for storage declared in a compound statement. -template class BlockScope final : public LocalScope { +template class BlockScope final : public AutoScope { public: - BlockScope(ByteCodeExprGen *Ctx) : LocalScope(Ctx) {} + BlockScope(ByteCodeExprGen *Ctx) : AutoScope(Ctx) {} void addExtended(const Scope::Local &Local) override { // If we to this point, just add the variable as a normal local @@ -359,9 +404,9 @@ /// Expression scope which tracks potentially lifetime extended /// temporaries which are hoisted to the parent scope on exit. -template class ExprScope final : public LocalScope { +template class ExprScope final : public AutoScope { public: - ExprScope(ByteCodeExprGen *Ctx) : LocalScope(Ctx) {} + ExprScope(ByteCodeExprGen *Ctx) : AutoScope(Ctx) {} void addExtended(const Scope::Local &Local) override { if (this->Parent) diff --git a/clang/lib/AST/Interp/ByteCodeExprGen.cpp b/clang/lib/AST/Interp/ByteCodeExprGen.cpp --- a/clang/lib/AST/Interp/ByteCodeExprGen.cpp +++ b/clang/lib/AST/Interp/ByteCodeExprGen.cpp @@ -26,10 +26,10 @@ namespace interp { /// Scope used to handle temporaries in toplevel variable declarations. -template class DeclScope final : public LocalScope { +template class DeclScope final : public VariableScope { public: DeclScope(ByteCodeExprGen *Ctx, const ValueDecl *VD) - : LocalScope(Ctx), Scope(Ctx->P, VD) {} + : VariableScope(Ctx), Scope(Ctx->P, VD) {} void addExtended(const Scope::Local &Local) override { return this->addLocal(Local); @@ -1857,6 +1857,80 @@ C->emitDestruction(); } +/// When calling this, we have a pointer of the local-to-destroy +/// on the stack. +/// Emit destruction of record types (or arrays of record types). +/// FIXME: Handle virtual destructors. +template +bool ByteCodeExprGen::emitRecordDestruction(const Descriptor *Desc) { + assert(Desc); + assert(!Desc->isPrimitive()); + assert(!Desc->isPrimitiveArray()); + + // Arrays. + if (Desc->isArray()) { + const Descriptor *ElemDesc = Desc->ElemDesc; + const Record *ElemRecord = ElemDesc->ElemRecord; + assert(ElemRecord); // This is not a primitive array. + + if (const CXXDestructorDecl *Dtor = ElemRecord->getDestructor(); + Dtor && !Dtor->isTrivial()) { + for (ssize_t I = Desc->getNumElems() - 1; I >= 0; --I) { + if (!this->emitConstUint64(I, SourceInfo{})) + return false; + if (!this->emitArrayElemPtrUint64(SourceInfo{})) + return false; + if (!this->emitRecordDestruction(Desc->ElemDesc)) + return false; + } + } + return this->emitPopPtr(SourceInfo{}); + } + + const Record *R = Desc->ElemRecord; + assert(R); + // First, destroy all fields. + for (const Record::Field &Field : llvm::reverse(R->fields())) { + const Descriptor *D = Field.Desc; + if (!D->isPrimitive() && !D->isPrimitiveArray()) { + if (!this->emitDupPtr(SourceInfo{})) + return false; + if (!this->emitGetPtrField(Field.Offset, SourceInfo{})) + return false; + if (!this->emitRecordDestruction(D)) + return false; + } + } + + // FIXME: Unions need to be handled differently here. We don't want to + // call the destructor of its members. + + // Now emit the destructor and recurse into base classes. + if (const CXXDestructorDecl *Dtor = R->getDestructor(); + Dtor && !Dtor->isTrivial()) { + const Function *DtorFunc = getFunction(Dtor); + if (DtorFunc && DtorFunc->isConstexpr()) { + assert(DtorFunc->hasThisPointer()); + assert(DtorFunc->getNumParams() == 1); + if (!this->emitDupPtr(SourceInfo{})) + return false; + if (!this->emitCall(DtorFunc, SourceInfo{})) + return false; + } + } + + for (const Record::Base &Base : llvm::reverse(R->bases())) { + if (!this->emitGetPtrBase(Base.Offset, SourceInfo{})) + return false; + if (!this->emitRecordDestruction(Base.Desc)) + return false; + } + // FIXME: Virtual bases. + + // Remove the instance pointer. + return this->emitPopPtr(SourceInfo{}); +} + namespace clang { namespace interp { diff --git a/clang/lib/AST/Interp/ByteCodeStmtGen.h b/clang/lib/AST/Interp/ByteCodeStmtGen.h --- a/clang/lib/AST/Interp/ByteCodeStmtGen.h +++ b/clang/lib/AST/Interp/ByteCodeStmtGen.h @@ -54,6 +54,7 @@ // Statement visitors. bool visitStmt(const Stmt *S); 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); diff --git a/clang/lib/AST/Interp/ByteCodeStmtGen.cpp b/clang/lib/AST/Interp/ByteCodeStmtGen.cpp --- a/clang/lib/AST/Interp/ByteCodeStmtGen.cpp +++ b/clang/lib/AST/Interp/ByteCodeStmtGen.cpp @@ -194,6 +194,23 @@ } } +/// Visits the given statment without creating a variable +/// scope for it in case it is a compound statement. +template +bool ByteCodeStmtGen::visitLoopBody(const Stmt *S) { + if (isa(S)) + return true; + + if (const auto *CS = dyn_cast(S)) { + for (auto *InnerStmt : CS->body()) + if (!visitStmt(InnerStmt)) + return false; + return true; + } + + return this->visitStmt(S); +} + template bool ByteCodeStmtGen::visitCompoundStmt( const CompoundStmt *CompoundStmt) { @@ -306,11 +323,15 @@ if (!this->jumpFalse(EndLabel)) return false; - if (!this->visitStmt(Body)) - return false; + LocalScope Scope(this); + { + DestructorScope DS(Scope); + if (!this->visitLoopBody(Body)) + return false; + } + if (!this->jump(CondLabel)) return false; - this->emitLabel(EndLabel); return true; @@ -325,15 +346,21 @@ LabelTy EndLabel = this->getLabel(); LabelTy CondLabel = this->getLabel(); LoopScope LS(this, EndLabel, CondLabel); + LocalScope Scope(this); this->emitLabel(StartLabel); - if (!this->visitStmt(Body)) - return false; - this->emitLabel(CondLabel); - if (!this->visitBool(Cond)) - return false; + { + DestructorScope DS(Scope); + + if (!this->visitLoopBody(Body)) + return false; + this->emitLabel(CondLabel); + if (!this->visitBool(Cond)) + return false; + } if (!this->jumpTrue(StartLabel)) return false; + this->emitLabel(EndLabel); return true; } @@ -350,6 +377,7 @@ LabelTy CondLabel = this->getLabel(); LabelTy IncLabel = this->getLabel(); LoopScope LS(this, EndLabel, IncLabel); + LocalScope Scope(this); if (Init && !this->visitStmt(Init)) return false; @@ -360,11 +388,17 @@ if (!this->jumpFalse(EndLabel)) return false; } - if (Body && !this->visitStmt(Body)) - return false; - this->emitLabel(IncLabel); - if (Inc && !this->discard(Inc)) - return false; + + { + DestructorScope DS(Scope); + + if (Body && !this->visitLoopBody(Body)) + return false; + this->emitLabel(IncLabel); + if (Inc && !this->discard(Inc)) + return false; + } + if (!this->jump(CondLabel)) return false; this->emitLabel(EndLabel); @@ -386,38 +420,40 @@ LabelTy CondLabel = this->getLabel(); LabelTy IncLabel = this->getLabel(); LoopScope LS(this, EndLabel, IncLabel); - { - ExprScope ES(this); - // Emit declarations needed in the loop. - if (Init && !this->visitStmt(Init)) - return false; - if (!this->visitStmt(RangeStmt)) - return false; - if (!this->visitStmt(BeginStmt)) - return false; - if (!this->visitStmt(EndStmt)) - return false; + // Emit declarations needed in the loop. + if (Init && !this->visitStmt(Init)) + return false; + if (!this->visitStmt(RangeStmt)) + return false; + if (!this->visitStmt(BeginStmt)) + return false; + if (!this->visitStmt(EndStmt)) + return false; - // Now the condition as well as the loop variable assignment. - this->emitLabel(CondLabel); - if (!this->visitBool(Cond)) - return false; - if (!this->jumpFalse(EndLabel)) - return false; + // Now the condition as well as the loop variable assignment. + this->emitLabel(CondLabel); + if (!this->visitBool(Cond)) + return false; + if (!this->jumpFalse(EndLabel)) + return false; - if (!this->visitVarDecl(LoopVar)) - return false; + if (!this->visitVarDecl(LoopVar)) + return false; + + // Body. + LocalScope Scope(this); + { + DestructorScope DS(Scope); - // Body. - if (!this->visitStmt(Body)) + if (!this->visitLoopBody(Body)) return false; this->emitLabel(IncLabel); if (!this->discard(Inc)) return false; - if (!this->jump(CondLabel)) - return false; } + if (!this->jump(CondLabel)) + return false; this->emitLabel(EndLabel); return true; @@ -428,6 +464,7 @@ if (!BreakLabel) return false; + this->VarScope->emitDestructors(); return this->jump(*BreakLabel); } @@ -436,6 +473,7 @@ if (!ContinueLabel) return false; + this->VarScope->emitDestructors(); return this->jump(*ContinueLabel); } diff --git a/clang/test/AST/Interp/cxx20.cpp b/clang/test/AST/Interp/cxx20.cpp --- a/clang/test/AST/Interp/cxx20.cpp +++ b/clang/test/AST/Interp/cxx20.cpp @@ -271,3 +271,250 @@ // ref-error {{must have constant destruction}} \ // ref-note {{in call to}} }; + +namespace Destructors { + + class Inc final { + public: + int &I; + constexpr Inc(int &I) : I(I) {} + constexpr ~Inc() { + I++; + } + }; + + class Dec final { + public: + int &I; + constexpr Dec(int &I) : I(I) {} + constexpr ~Dec() { + I--; + } + }; + + + + constexpr int m() { + int i = 0; + { + Inc f1(i); + Inc f2(i); + Inc f3(i); + } + return i; + } + static_assert(m() == 3, ""); + + + constexpr int C() { + int i = 0; + + while (i < 10) { + Inc inc(i); + continue; + Dec dec(i); + } + return i; + } + static_assert(C() == 10, ""); + + + constexpr int D() { + int i = 0; + + { + Inc i1(i); + { + Inc i2(i); + return i; + } + } + + return i; + } + static_assert(D() == 0, ""); + + constexpr int E() { + int i = 0; + + for(;;) { + Inc i1(i); + break; + } + return i; + } + static_assert(E() == 1, ""); + + + /// FIXME: This should be rejected, since we call the destructor + /// twice. However, GCC doesn't care either. + constexpr int ManualDtor() { + int i = 0; + { + Inc I(i); // ref-note {{destroying object 'I' whose lifetime has already ended}} + I.~Inc(); + } + return i; + } + static_assert(ManualDtor() == 1, ""); // expected-error {{static assertion failed}} \ + // expected-note {{evaluates to '2 == 1'}} \ + // ref-error {{not an integral constant expression}} \ + // ref-note {{in call to 'ManualDtor()'}} + + constexpr void doInc(int &i) { + Inc I(i); + return; + } + constexpr int testInc() { + int i = 0; + doInc(i); + return i; + } + static_assert(testInc() == 1, ""); + constexpr void doInc2(int &i) { + Inc I(i); + // No return statement. + } + constexpr int testInc2() { + int i = 0; + doInc2(i); + return i; + } + static_assert(testInc2() == 1, ""); + + + namespace DtorOrder { + class A { + public: + int &I; + constexpr A(int &I) : I(I) {} + constexpr ~A() { + I = 1337; + } + }; + + class B : public A { + public: + constexpr B(int &I) : A(I) {} + constexpr ~B() { + I = 42; + } + }; + + constexpr int foo() { + int i = 0; + { + B b(i); + } + return i; + } + + static_assert(foo() == 1337); + } + + class FieldDtor1 { + public: + Inc I1; + Inc I2; + constexpr FieldDtor1(int &I) : I1(I), I2(I){} + }; + + constexpr int foo2() { + int i = 0; + { + FieldDtor1 FD1(i); + } + return i; + } + + static_assert(foo2() == 2); + + class FieldDtor2 { + public: + Inc Incs[3]; + constexpr FieldDtor2(int &I) : Incs{Inc(I), Inc(I), Inc(I)} {} + }; + + constexpr int foo3() { + int i = 0; + { + FieldDtor2 FD2(i); + } + return i; + } + + static_assert(foo3() == 3); + + struct ArrD { + int index; + int *arr; + int &p; + constexpr ~ArrD() { + arr[p] = index; + ++p; + } + }; + constexpr bool ArrayOrder() { + int order[3] = {0, 0, 0}; + int p = 0; + { + ArrD ds[3] = { + {1, order, p}, + {2, order, p}, + {3, order, p}, + }; + // ds will be destroyed. + } + return order[0] == 3 && order[1] == 2 && order[2] == 1; + } + static_assert(ArrayOrder()); + + + // Static members aren't destroyed. + class Dec2 { + public: + int A = 0; + constexpr ~Dec2() { + A++; + } + }; + class Foo { + public: + static constexpr Dec2 a; + static Dec2 b; + }; + static_assert(Foo::a.A == 0); + constexpr bool f() { + Foo f; + return true; + } + static_assert(Foo::a.A == 0); + static_assert(f()); + static_assert(Foo::a.A == 0); + + + struct NotConstexpr { + NotConstexpr() {} + ~NotConstexpr() {} + }; + + struct Outer { + constexpr Outer() = default; + constexpr ~Outer(); + + constexpr int foo() { + return 12; + } + + constexpr int bar()const { + return Outer{}.foo(); + } + + static NotConstexpr Val; + }; + + constexpr Outer::~Outer() {} + + constexpr Outer O; + static_assert(O.bar() == 12); +}