Author: Timm Baeder Date: 2026-06-25T15:29:21+02:00 New Revision: c834d1acf130b8d98d818b60ab129f780855461f
URL: https://github.com/llvm/llvm-project/commit/c834d1acf130b8d98d818b60ab129f780855461f DIFF: https://github.com/llvm/llvm-project/commit/c834d1acf130b8d98d818b60ab129f780855461f.diff LOG: [clang][bytecode] Fix `evaluateDestruction()` (#205778) Me previous testing regarding this seems to have been insufficient. Or this regressed some time along the way. Now that `CLANG_USE_EXPERIMENTAL_CONST_INTERP` is used for testing I noticed a few regressions. We need to special-case the evaluating decl in a few places, since it's a global variable that we're allowed to modify. Added: clang/test/AST/ByteCode/evaluate-dtor.cpp Modified: clang/lib/AST/ByteCode/Compiler.cpp clang/lib/AST/ByteCode/Context.cpp clang/lib/AST/ByteCode/EvalEmitter.cpp clang/lib/AST/ByteCode/Interp.cpp clang/lib/AST/ByteCode/InterpFrame.cpp clang/lib/AST/ByteCode/InterpState.h clang/lib/AST/ExprConstant.cpp clang/test/CXX/expr/expr.const/p6-2a.cpp clang/test/CodeGenCXX/const-init-cxx2a.cpp clang/test/CodeGenCXX/non-const-init-cxx2a.cpp Removed: ################################################################################ diff --git a/clang/lib/AST/ByteCode/Compiler.cpp b/clang/lib/AST/ByteCode/Compiler.cpp index cadb040048384..389a0094509a9 100644 --- a/clang/lib/AST/ByteCode/Compiler.cpp +++ b/clang/lib/AST/ByteCode/Compiler.cpp @@ -5398,10 +5398,11 @@ bool Compiler<Emitter>::visitDtorCall(const VarDecl *VD, const APValue &Value) { DeclScope<Emitter> LocalScope(this, VD); // Create a local variable to use as the instance. QualType Ty = VD->getType(); - Descriptor *D = P.createDescriptor( - VD, Ty.getTypePtr(), Descriptor::InlineDescMD, /*IsConst=*/false, - /*IsTemporary=*/false, /*IsMutable=*/false, - /*IsVolatile=*/Ty.isVolatileQualified(), nullptr); + Descriptor *D = + P.createDescriptor(VD, Ty.getTypePtr(), Descriptor::InlineDescMD, + /*IsConst=*/Ty.isConstQualified(), + /*IsTemporary=*/false, /*IsMutable=*/false, + /*IsVolatile=*/Ty.isVolatileQualified(), nullptr); if (!D) return false; diff --git a/clang/lib/AST/ByteCode/Context.cpp b/clang/lib/AST/ByteCode/Context.cpp index 4beb35a9a7b43..40cf29efcfb4f 100644 --- a/clang/lib/AST/ByteCode/Context.cpp +++ b/clang/lib/AST/ByteCode/Context.cpp @@ -174,6 +174,8 @@ bool Context::evaluateDestruction(State &Parent, const VarDecl *VD, return false; } + assert(Stk.empty()); + return true; } diff --git a/clang/lib/AST/ByteCode/EvalEmitter.cpp b/clang/lib/AST/ByteCode/EvalEmitter.cpp index 7aeac16b9639e..e862d945e020f 100644 --- a/clang/lib/AST/ByteCode/EvalEmitter.cpp +++ b/clang/lib/AST/ByteCode/EvalEmitter.cpp @@ -77,11 +77,14 @@ EvaluationResult EvalEmitter::interpretDestructor(const VarDecl *VD, const APValue &Value) { assert(VD); S.setEvalLocation(VD->getLocation()); + S.EvaluatingDecl = VD; + S.EvalKind = EvaluationKind::Dtor; EvalResult.setSource(VD); if (!this->visitDtorCall(VD, Value)) EvalResult.setInvalid(); + S.EvaluatingDecl = nullptr; return std::move(this->EvalResult); } diff --git a/clang/lib/AST/ByteCode/Interp.cpp b/clang/lib/AST/ByteCode/Interp.cpp index 3d5fda7ddf3c7..71021815baeef 100644 --- a/clang/lib/AST/ByteCode/Interp.cpp +++ b/clang/lib/AST/ByteCode/Interp.cpp @@ -205,14 +205,16 @@ static bool CheckTemporary(InterpState &S, CodePtr OpPC, const Block *B, // FIXME(perf): Since we do this check on every Load from a static // temporary, it might make sense to cache the value of the // isUsableInConstantExpressions call. - if (B->getEvalID() != S.EvalID && - !MTE->isUsableInConstantExpressions(S.getASTContext())) { + if (S.checkingConstantDestruction() || + (B->getEvalID() != S.EvalID && + !MTE->isUsableInConstantExpressions(S.getASTContext()))) { const SourceInfo &E = S.Current->getSource(OpPC); S.FFDiag(E, diag::note_constexpr_access_static_temporary, 1) << AK; noteValueLocation(S, B); return false; } } + return true; } @@ -459,7 +461,12 @@ bool CheckConstant(InterpState &S, CodePtr OpPC, const Descriptor *Desc, assert(Desc); const auto *D = Desc->asVarDecl(); - if (!D || D == S.EvaluatingDecl || D->isConstexpr()) + if (S.checkingConstantDestruction(D)) { + // If we're checking for a constant destructor for this variable, we can + // only read from it if it is constant. + if (D->getType().isConstQualified()) + return true; + } else if (!D || D == S.EvaluatingDecl || D->isConstexpr()) return true; // If we're evaluating the initializer for a constexpr variable in C23, we may @@ -509,6 +516,9 @@ bool CheckConstant(InterpState &S, CodePtr OpPC, const Descriptor *Desc, static bool CheckConstant(InterpState &S, CodePtr OpPC, const Pointer &Ptr, AccessKinds AK = AK_Read) { + if (S.checkingConstantDestruction(Ptr)) + return CheckConstant(S, OpPC, Ptr.getDeclDesc(), AK); + if (!Ptr.isStatic() || !Ptr.isBlockPointer()) return true; if (!Ptr.getDeclID()) @@ -648,10 +658,24 @@ bool CheckMutable(InterpState &S, CodePtr OpPC, PtrView Ptr, AccessKinds AK) { if (!Ptr.isMutable()) return true; - // In C++14 onwards, it is permitted to read a mutable member whose - // lifetime began within the evaluation. - if (S.getLangOpts().CPlusPlus14 && Ptr.getEvalID() == S.EvalID) + if (S.checkingConstantDestruction()) { + // Never allowed when checking for constant destruction. + // Find the reason this pointer is mutable. + PtrView MutablePtr = Ptr; + while (!MutablePtr.isRoot() && MutablePtr.getBase().isMutable()) + MutablePtr = MutablePtr.getBase(); + + const FieldDecl *Field = MutablePtr.getField(); + S.FFDiag(S.Current->getSource(OpPC), diag::note_constexpr_access_mutable, 1) + << AK << Field; + S.Note(Field->getLocation(), diag::note_declared_at); + return false; + } else if (S.getLangOpts().CPlusPlus14 && + S.lifetimeStartedInEvaluation(Ptr.block())) { + // In C++14 onwards, it is permitted to read a mutable member whose + // lifetime began within the evaluation. return true; + } const SourceInfo &Loc = S.Current->getSource(OpPC); const FieldDecl *Field = Ptr.getField(); @@ -985,21 +1009,23 @@ bool CheckStore(InterpState &S, CodePtr OpPC, const Pointer &Ptr, return false; if (!CheckVolatile(S, OpPC, Ptr, AK_Assign)) return false; + if (!CheckMutable(S, OpPC, Ptr, AK_Assign)) + return false; if (isConstexprUnknown(Ptr)) return false; return true; } static bool CheckInvoke(InterpState &S, CodePtr OpPC, const Pointer &Ptr, - bool IsCtorDtor = false) { + bool IsCtor, bool IsDtor) { if (!Ptr.isDummy() && !isConstexprUnknown(Ptr)) { if (!CheckLive(S, OpPC, Ptr, AK_MemberCall)) return false; if (!CheckRange(S, OpPC, Ptr, AK_MemberCall)) return false; - if (!IsCtorDtor && !CheckLifetime(S, OpPC, Ptr, AK_MemberCall)) + if (!(IsCtor || IsDtor) && !CheckLifetime(S, OpPC, Ptr, AK_MemberCall)) return false; - if (!CheckMutable(S, OpPC, Ptr)) + if (!IsDtor && !CheckMutable(S, OpPC, Ptr)) return false; } return true; @@ -1708,6 +1734,11 @@ bool CheckDestructor(InterpState &S, CodePtr OpPC, const Pointer &Ptr) { if (Ptr.getLifetime() == Lifetime::Ended) return CheckLifetime(S, OpPC, Ptr, AK_Destroy); + // We _can_ call the destructor on the global variable we're checking constant + // destruction for. + if (S.checkingConstantDestruction(Ptr)) + return true; + // Can't call a dtor on a global variable. if (Ptr.block()->isStatic()) { const SourceInfo &E = S.Current->getSource(OpPC); @@ -1803,8 +1834,8 @@ bool CallVar(InterpState &S, CodePtr OpPC, const Function *Func, if (!(S.Current->getFunction() && S.Current->getFunction()->isLambdaStaticInvoker() && Func->isLambdaCallOperator())) { - if (!CheckInvoke(S, OpPC, ThisPtr, - Func->isConstructor() || Func->isDestructor())) + if (!CheckInvoke(S, OpPC, ThisPtr, Func->isConstructor(), + Func->isDestructor())) return false; } @@ -1873,14 +1904,14 @@ bool Call(InterpState &S, CodePtr OpPC, const Function *Func, Func->isLambdaCallOperator()) { assert(ThisPtr.isZero()); } else { - if (!CheckInvoke(S, OpPC, ThisPtr, - Func->isConstructor() || Func->isDestructor())) + if (!CheckInvoke(S, OpPC, ThisPtr, Func->isConstructor(), + Func->isDestructor())) return cleanup(); if (Func->isCopyOrMoveOperator() || Func->isCopyOrMoveConstructor()) { const Pointer &RVOPtr = S.Stk.peek<Pointer>(ThisOffset - align(sizeof(Pointer))); - if (!CheckInvoke(S, OpPC, RVOPtr, /*IsCtorDtor=*/true)) + if (!CheckInvoke(S, OpPC, RVOPtr, /*IsCtor=*/true, /*IsDtor=*/false)) return cleanup(); } diff --git a/clang/lib/AST/ByteCode/InterpFrame.cpp b/clang/lib/AST/ByteCode/InterpFrame.cpp index 5eb8273af17f2..7366aca114ca0 100644 --- a/clang/lib/AST/ByteCode/InterpFrame.cpp +++ b/clang/lib/AST/ByteCode/InterpFrame.cpp @@ -287,6 +287,7 @@ SourceInfo InterpFrame::getSource(CodePtr PC) const { SourceInfo Result = S.getSource(Func, PC); if (Result.getLoc().isInvalid() && Caller) return Caller->getSource(RetPC); + return Result; } diff --git a/clang/lib/AST/ByteCode/InterpState.h b/clang/lib/AST/ByteCode/InterpState.h index 92073cebcd490..4e4a053d6bbed 100644 --- a/clang/lib/AST/ByteCode/InterpState.h +++ b/clang/lib/AST/ByteCode/InterpState.h @@ -32,6 +32,13 @@ struct StdAllocatorCaller { explicit operator bool() { return Call; } }; +// FIXME: Create one for the "checking potential constant expression" +// evaluation. +enum class EvaluationKind : uint8_t { + None, + Dtor, /// We're checking for constant destruction of a global variable. +}; + /// Interpreter context. class InterpState final : public State, public SourceMapper { public: @@ -132,6 +139,31 @@ class InterpState final : public State, public SourceMapper { return false; } + bool lifetimeStartedInEvaluation(const Block *B) const { + if (EvalKind == EvaluationKind::None) + return B->getEvalID() == EvalID; + + if (EvalKind == EvaluationKind::Dtor) { + assert(EvaluatingDecl); + if (B->getDescriptor()->asVarDecl() == EvaluatingDecl) + return EvaluatingDecl->getType().isConstQualified(); + } + return false; + } + + /// Return if we're checking if a global variable has a constant destructor. + bool checkingConstantDestruction() const { + return EvalKind == EvaluationKind::Dtor; + } + /// Return if we're checking if a global variable has a constant destructor + /// and the given pointer is pointing to the variable we're checking that for. + bool checkingConstantDestruction(const Pointer &Ptr) const { + return checkingConstantDestruction(Ptr.getDeclDesc()->asVarDecl()); + } + bool checkingConstantDestruction(const VarDecl *VD) const { + return EvalKind == EvaluationKind::Dtor && VD == EvaluatingDecl; + } + private: friend class EvaluationResult; friend class InterpStateCCOverride; @@ -167,6 +199,8 @@ class InterpState final : public State, public SourceMapper { /// ID identifying this evaluation. const unsigned EvalID; + EvaluationKind EvalKind = EvaluationKind::None; + /// Things needed to do speculative execution. SmallVectorImpl<PartialDiagnosticAt> *PrevDiags = nullptr; bool PrevDiagsEmitted = false; diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp index 28ac44edd800c..1d359339b9104 100644 --- a/clang/lib/AST/ExprConstant.cpp +++ b/clang/lib/AST/ExprConstant.cpp @@ -21748,8 +21748,6 @@ bool VarDecl::evaluateDestruction( EvalInfo Info(Ctx, EStatus, IsConstantDestruction ? EvaluationMode::ConstantExpression : EvaluationMode::ConstantFold); - Info.setEvaluatingDecl(this, DestroyedValue, - EvalInfo::EvaluatingDeclKind::Dtor); Info.InConstantContext = IsConstantDestruction; if (!Ctx.getInterpContext().evaluateDestruction(Info, this, std::move(DestroyedValue))) diff --git a/clang/test/AST/ByteCode/evaluate-dtor.cpp b/clang/test/AST/ByteCode/evaluate-dtor.cpp new file mode 100644 index 0000000000000..da0d1c2d4cafb --- /dev/null +++ b/clang/test/AST/ByteCode/evaluate-dtor.cpp @@ -0,0 +1,56 @@ +// RUN: %clang_cc1 -std=c++23 -verify=both,expected %s -fexperimental-new-constant-interpreter +// RUN: %clang_cc1 -std=c++23 -verify=both,ref %s + +struct MutableConst { + struct HasConstMember { + const int n = 4; + }; + mutable HasConstMember hcm; + constexpr ~MutableConst() { + /// This is _not_ a read. + auto *p = &hcm.n; + } +}; +constexpr MutableConst mc; + + +struct C { + static int m; + bool b; + constexpr C(bool b) : b(b){ + } + constexpr ~C() { + if (b) + m++; // both-note {{a constant expression cannot modify an object that is visible outside that expression}} + } +}; + +int C::m = 0; +constexpr C c1(false); +constexpr C c2(true); // both-error {{must have constant destruction}} \ + // both-note {{in call to}} + + + +namespace LocalVariables { + struct S { + constexpr ~S() { + int a = 0; + ++a; + } + }; + constexpr S s; +} + +namespace GlobalVariables { + struct S {}; + constexpr S s; + + struct K { + constexpr ~K() { + s.~S(); // both-note {{a constant expression cannot modify an object that is visible outside that expression}} + } + }; + constexpr K k{}; // both-error {{must have constant destruction}} \ + // both-note {{in call to}} +} diff --git a/clang/test/CXX/expr/expr.const/p6-2a.cpp b/clang/test/CXX/expr/expr.const/p6-2a.cpp index a937474d53b22..e97f35c891717 100644 --- a/clang/test/CXX/expr/expr.const/p6-2a.cpp +++ b/clang/test/CXX/expr/expr.const/p6-2a.cpp @@ -1,4 +1,5 @@ // RUN: %clang_cc1 -std=c++2a -verify %s +// RUN: %clang_cc1 -std=c++2a -verify %s -fexperimental-new-constant-interpreter constexpr int non_class = 42; constexpr int arr_non_class[5] = {1, 2, 3}; diff --git a/clang/test/CodeGenCXX/const-init-cxx2a.cpp b/clang/test/CodeGenCXX/const-init-cxx2a.cpp index 3c83a9c94adec..765f5b8d46e6f 100644 --- a/clang/test/CodeGenCXX/const-init-cxx2a.cpp +++ b/clang/test/CodeGenCXX/const-init-cxx2a.cpp @@ -1,4 +1,5 @@ -// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -o - %s -std=c++2a | FileCheck %s --implicit-check-not=cxx_global_var_init --implicit-check-not=cxa_atexit +// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -o - %s -std=c++2a | FileCheck %s --implicit-check-not=cxx_global_var_init --implicit-check-not=cxa_atexit +// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -o - %s -std=c++2a -fexperimental-new-constant-interpreter | FileCheck %s --implicit-check-not=cxx_global_var_init --implicit-check-not=cxa_atexit // RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-pch -o %t.pch %s -std=c++2a // RUN: %clang_cc1 -triple x86_64-linux-gnu -include-pch %t.pch -x c++ /dev/null -emit-llvm -o - -std=c++2a | FileCheck %s --implicit-check-not=cxx_global_var_init --implicit-check-not=cxa_atexit diff --git a/clang/test/CodeGenCXX/non-const-init-cxx2a.cpp b/clang/test/CodeGenCXX/non-const-init-cxx2a.cpp index 1ebb595897b31..48594632d4d06 100644 --- a/clang/test/CodeGenCXX/non-const-init-cxx2a.cpp +++ b/clang/test/CodeGenCXX/non-const-init-cxx2a.cpp @@ -1,4 +1,5 @@ // RUN: %clang_cc1 -triple x86_64-apple-darwin -emit-llvm -o - %s -std=c++2a | FileCheck %s +// RUN: %clang_cc1 -triple x86_64-apple-darwin -emit-llvm -o - %s -std=c++2a -fexperimental-new-constant-interpreter | FileCheck %s // RUN: %clang_cc1 -triple x86_64-apple-darwin -emit-pch -o %t.pch %s -std=c++2a // RUN: %clang_cc1 -triple x86_64-apple-darwin -include-pch %t.pch -x c++ /dev/null -emit-llvm -o - -std=c++2a | FileCheck %s _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
