diff --git a/clang/lib/AST/Interp/ByteCodeExprGen.cpp b/clang/lib/AST/Interp/ByteCodeExprGen.cpp index f193f959d3a6c..b151f8d0d7a79 100644 --- a/clang/lib/AST/Interp/ByteCodeExprGen.cpp +++ b/clang/lib/AST/Interp/ByteCodeExprGen.cpp @@ -2587,7 +2587,10 @@ bool ByteCodeExprGen::visitExpr(const Expr *E) { if (!this->emitFinishInit(E)) return false; - return this->emitRetValue(E); + // We are destroying the locals AFTER the Ret op. + // The Ret op needs to copy the (alive) values, but the + // destructors may still turn the entire expression invalid. + return this->emitRetValue(E) && RootScope.destroyLocals(); } return false; @@ -3414,14 +3417,15 @@ bool ByteCodeExprGen::emitRecordDestruction(const Record *R) { // Now emit the destructor and recurse into base classes. if (const CXXDestructorDecl *Dtor = R->getDestructor(); Dtor && !Dtor->isTrivial()) { - if (const Function *DtorFunc = getFunction(Dtor)) { - assert(DtorFunc->hasThisPointer()); - assert(DtorFunc->getNumParams() == 1); - if (!this->emitDupPtr(SourceInfo{})) - return false; - if (!this->emitCall(DtorFunc, 0, SourceInfo{})) - return false; - } + const Function *DtorFunc = getFunction(Dtor); + if (!DtorFunc) + return false; + assert(DtorFunc->hasThisPointer()); + assert(DtorFunc->getNumParams() == 1); + if (!this->emitDupPtr(SourceInfo{})) + return false; + if (!this->emitCall(DtorFunc, 0, SourceInfo{})) + return false; } for (const Record::Base &Base : llvm::reverse(R->bases())) { diff --git a/clang/lib/AST/Interp/ByteCodeExprGen.h b/clang/lib/AST/Interp/ByteCodeExprGen.h index 5b3b533dba387..acbbcc3dc9619 100644 --- a/clang/lib/AST/Interp/ByteCodeExprGen.h +++ b/clang/lib/AST/Interp/ByteCodeExprGen.h @@ -332,7 +332,7 @@ template class VariableScope { } virtual void emitDestruction() {} - virtual void emitDestructors() {} + virtual bool emitDestructors() { return true; } VariableScope *getParent() const { return Parent; } protected: @@ -356,13 +356,18 @@ template class LocalScope : public VariableScope { } /// Overriden to support explicit destruction. - void emitDestruction() override { + void emitDestruction() override { destroyLocals(); } + + /// Explicit destruction of local variables. + bool destroyLocals() { if (!Idx) - return; - this->emitDestructors(); + return true; + + bool Success = this->emitDestructors(); this->Ctx->emitDestroy(*Idx, SourceInfo{}); removeStoredOpaqueValues(); this->Idx = std::nullopt; + return Success; } void addLocal(const Scope::Local &Local) override { @@ -374,19 +379,25 @@ template class LocalScope : public VariableScope { this->Ctx->Descriptors[*Idx].emplace_back(Local); } - void emitDestructors() override { + bool emitDestructors() override { if (!Idx) - return; + 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()) { - this->Ctx->emitGetPtrLocal(Local.Offset, SourceInfo{}); - this->Ctx->emitDestruction(Local.Desc); - this->Ctx->emitPopPtr(SourceInfo{}); + if (!this->Ctx->emitGetPtrLocal(Local.Offset, SourceInfo{})) + return false; + + if (!this->Ctx->emitDestruction(Local.Desc)) + return false; + + if (!this->Ctx->emitPopPtr(SourceInfo{})) + return false; removeIfStoredOpaqueValue(Local); } } + return true; } void removeStoredOpaqueValues() { diff --git a/clang/lib/AST/Interp/EvalEmitter.cpp b/clang/lib/AST/Interp/EvalEmitter.cpp index 9cae25f5c4d64..c9c2bf9b145b2 100644 --- a/clang/lib/AST/Interp/EvalEmitter.cpp +++ b/clang/lib/AST/Interp/EvalEmitter.cpp @@ -38,8 +38,11 @@ EvaluationResult EvalEmitter::interpretExpr(const Expr *E, this->ConvertResultToRValue = ConvertResultToRValue; EvalResult.setSource(E); - if (!this->visitExpr(E) && EvalResult.empty()) + if (!this->visitExpr(E)) { + // EvalResult may already have a result set, but something failed + // after that (e.g. evaluating destructors). EvalResult.setInvalid(); + } return std::move(this->EvalResult); } diff --git a/clang/lib/AST/Interp/EvaluationResult.h b/clang/lib/AST/Interp/EvaluationResult.h index 28e1ae6ba3e7a..ecf2250074cc9 100644 --- a/clang/lib/AST/Interp/EvaluationResult.h +++ b/clang/lib/AST/Interp/EvaluationResult.h @@ -72,7 +72,8 @@ class EvaluationResult final { Kind = LValue; } void setInvalid() { - assert(empty()); + // We are NOT asserting empty() here, since setting it to invalid + // is allowed even if there is already a result. Kind = Invalid; } void setValid() { diff --git a/clang/test/AST/Interp/cxx20.cpp b/clang/test/AST/Interp/cxx20.cpp index 5c9c625796510..b24b0c8a3ba0e 100644 --- a/clang/test/AST/Interp/cxx20.cpp +++ b/clang/test/AST/Interp/cxx20.cpp @@ -1,5 +1,5 @@ -// RUN: %clang_cc1 -fcxx-exceptions -fexperimental-new-constant-interpreter -std=c++20 -verify %s -// RUN: %clang_cc1 -fcxx-exceptions -std=c++20 -verify=ref %s +// RUN: %clang_cc1 -fcxx-exceptions -fexperimental-new-constant-interpreter -std=c++20 -verify=both,expected -fcxx-exceptions %s +// RUN: %clang_cc1 -fcxx-exceptions -std=c++20 -verify=both,ref -fcxx-exceptions %s void test_alignas_operand() { alignas(8) char dummy; @@ -799,3 +799,21 @@ void f2() { // access info for unnamed bit-field } } + +namespace FailingDestructor { + struct D { + int n; + bool can_destroy; + + constexpr ~D() { + if (!can_destroy) + throw "oh no"; + } + }; + template + void f() {} // both-note {{invalid explicitly-specified argument}} + + void g() { + f(); // both-error {{no matching function}} + } +}