From 457226e02a6e8533eaaa864a3fd7c8eeccd2bf58 Mon Sep 17 00:00:00 2001 From: Richard Smith Date: Mon, 23 Sep 2019 03:48:44 +0000 Subject: [PATCH] For P0784R7: add support for constexpr destructors, and call them as appropriate during constant evaluation. Note that the evaluator is sometimes invoked on incomplete expressions. In such cases, if an object is constructed but we never reach the point where it would be destroyed (and it has non-trivial destruction), we treat the expression as having an unmodeled side-effect. llvm-svn: 372538 --- clang/include/clang/AST/DeclCXX.h | 29 +- clang/include/clang/Basic/DiagnosticASTKinds.td | 2 + clang/include/clang/Basic/DiagnosticSemaKinds.td | 10 +- clang/lib/AST/ASTImporter.cpp | 2 +- clang/lib/AST/DeclCXX.cpp | 39 +- clang/lib/AST/ExprConstant.cpp | 549 +++++++++++++++++---- clang/lib/Sema/SemaDecl.cpp | 12 +- clang/lib/Sema/SemaDeclCXX.cpp | 70 ++- clang/lib/Sema/SemaTemplateInstantiateDecl.cpp | 7 +- clang/lib/Sema/SemaType.cpp | 26 +- .../test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p1.cpp | 8 +- .../test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3.cpp | 5 +- .../dcl.fct.def/dcl.fct.def.default/p2.cpp | 2 +- clang/test/CXX/drs/dr2xx.cpp | 5 +- .../attr-require-constant-initialization.cpp | 7 +- clang/test/SemaCXX/constant-expression-cxx2a.cpp | 119 +++++ clang/test/SemaCXX/cxx2a-consteval.cpp | 8 +- 17 files changed, 741 insertions(+), 159 deletions(-) diff --git a/clang/include/clang/AST/DeclCXX.h b/clang/include/clang/AST/DeclCXX.h index a5dc837..d99c31e 100644 --- a/clang/include/clang/AST/DeclCXX.h +++ b/clang/include/clang/AST/DeclCXX.h @@ -445,6 +445,9 @@ class CXXRecordDecl : public RecordDecl { /// or an implicitly declared constexpr default constructor. unsigned HasConstexprDefaultConstructor : 1; + /// True if a defaulted destructor for this class would be constexpr. + unsigned DefaultedDestructorIsConstexpr : 1; + /// True when this class contains at least one non-static data /// member or base class of non-literal or volatile type. unsigned HasNonLiteralTypeFieldsOrBases : 1; @@ -1441,6 +1444,16 @@ public: !(data().HasTrivialSpecialMembers & SMF_MoveAssignment)); } + /// Determine whether a defaulted default constructor for this class + /// would be constexpr. + bool defaultedDestructorIsConstexpr() const { + return data().DefaultedDestructorIsConstexpr && + getASTContext().getLangOpts().CPlusPlus2a; + } + + /// Determine whether this class has a constexpr destructor. + bool hasConstexprDestructor() const; + /// Determine whether this class has a trivial destructor /// (C++ [class.dtor]p3) bool hasTrivialDestructor() const { @@ -1532,8 +1545,10 @@ public: /// /// Only in C++17 and beyond, are lambdas literal types. bool isLiteral() const { - return hasTrivialDestructor() && - (!isLambda() || getASTContext().getLangOpts().CPlusPlus17) && + ASTContext &Ctx = getASTContext(); + return (Ctx.getLangOpts().CPlusPlus2a ? hasConstexprDestructor() + : hasTrivialDestructor()) && + (!isLambda() || Ctx.getLangOpts().CPlusPlus17) && !hasNonLiteralTypeFieldsOrBases() && (isAggregate() || isLambda() || hasConstexprNonCopyMoveConstructor() || @@ -2802,9 +2817,9 @@ class CXXDestructorDecl : public CXXMethodDecl { CXXDestructorDecl(ASTContext &C, CXXRecordDecl *RD, SourceLocation StartLoc, const DeclarationNameInfo &NameInfo, QualType T, TypeSourceInfo *TInfo, bool isInline, - bool isImplicitlyDeclared) + bool isImplicitlyDeclared, ConstexprSpecKind ConstexprKind) : CXXMethodDecl(CXXDestructor, C, RD, StartLoc, NameInfo, T, TInfo, - SC_None, isInline, CSK_unspecified, SourceLocation()) { + SC_None, isInline, ConstexprKind, SourceLocation()) { setImplicit(isImplicitlyDeclared); } @@ -2814,9 +2829,9 @@ public: static CXXDestructorDecl *Create(ASTContext &C, CXXRecordDecl *RD, SourceLocation StartLoc, const DeclarationNameInfo &NameInfo, - QualType T, TypeSourceInfo* TInfo, - bool isInline, - bool isImplicitlyDeclared); + QualType T, TypeSourceInfo *TInfo, + bool isInline, bool isImplicitlyDeclared, + ConstexprSpecKind ConstexprKind); static CXXDestructorDecl *CreateDeserialized(ASTContext & C, unsigned ID); void setOperatorDelete(FunctionDecl *OD, Expr *ThisArg); diff --git a/clang/include/clang/Basic/DiagnosticASTKinds.td b/clang/include/clang/Basic/DiagnosticASTKinds.td index a0acb08..f11cd75 100644 --- a/clang/include/clang/Basic/DiagnosticASTKinds.td +++ b/clang/include/clang/Basic/DiagnosticASTKinds.td @@ -193,6 +193,8 @@ def note_constexpr_baa_insufficient_alignment : Note< def note_constexpr_baa_value_insufficient_alignment : Note< "value of the aligned pointer (%0) is not a multiple of the asserted %1 " "%plural{1:byte|:bytes}1">; +def note_constexpr_unsupported_destruction : Note< + "non-trivial destruction of type %0 in a constant expression is not supported">; def note_constexpr_unsupported_unsized_array : Note< "array-to-pointer decay of array member without known bound is not supported">; def note_constexpr_unsized_array_indexed : Note< diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 965cccc..5f47da0 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -2366,7 +2366,13 @@ def err_constexpr_tag : Error< "%select{class|struct|interface|union|enum}0 " "cannot be marked %sub{select_constexpr_spec_kind}1">; def err_constexpr_dtor : Error< - "destructor cannot be marked %sub{select_constexpr_spec_kind}0">; + "destructor cannot be declared %sub{select_constexpr_spec_kind}0">; +def err_constexpr_dtor_subobject : Error< + "destructor cannot be declared %sub{select_constexpr_spec_kind}0 because " + "%select{data member %2|base class %3}1 does not have a " + "constexpr destructor">; +def note_constexpr_dtor_subobject : Note< + "%select{data member %1|base class %2}0 declared here">; def err_constexpr_wrong_decl_kind : Error< "%sub{select_constexpr_spec_kind}0 can only be used " "in %select{|variable and function|function|variable}0 declarations">; @@ -2507,6 +2513,8 @@ def note_non_literal_user_provided_dtor : Note< "%0 is not literal because it has a user-provided destructor">; def note_non_literal_nontrivial_dtor : Note< "%0 is not literal because it has a non-trivial destructor">; +def note_non_literal_non_constexpr_dtor : Note< + "%0 is not literal because its destructor is not constexpr">; def note_non_literal_lambda : Note< "lambda closure types are non-literal types before C++17">; def warn_private_extern : Warning< diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp index 7be3502..057444a 100644 --- a/clang/lib/AST/ASTImporter.cpp +++ b/clang/lib/AST/ASTImporter.cpp @@ -3244,7 +3244,7 @@ ExpectedDecl ASTNodeImporter::VisitFunctionDecl(FunctionDecl *D) { if (GetImportedOrCreateDecl( ToFunction, D, Importer.getToContext(), cast(DC), ToInnerLocStart, NameInfo, T, TInfo, D->isInlineSpecified(), - D->isImplicit())) + D->isImplicit(), D->getConstexprKind())) return ToFunction; CXXDestructorDecl *ToDtor = cast(ToFunction); diff --git a/clang/lib/AST/DeclCXX.cpp b/clang/lib/AST/DeclCXX.cpp index bcc2238..297a598 100644 --- a/clang/lib/AST/DeclCXX.cpp +++ b/clang/lib/AST/DeclCXX.cpp @@ -95,6 +95,7 @@ CXXRecordDecl::DefinitionData::DefinitionData(CXXRecordDecl *D) HasDefaultedDefaultConstructor(false), DefaultedDefaultConstructorIsConstexpr(true), HasConstexprDefaultConstructor(false), + DefaultedDestructorIsConstexpr(true), HasNonLiteralTypeFieldsOrBases(false), ComputedVisibleConversions(false), UserProvidedDefaultConstructor(false), DeclaredSpecialMembers(0), ImplicitCopyConstructorCanHaveConstParamForVBase(true), @@ -325,10 +326,12 @@ CXXRecordDecl::setBases(CXXBaseSpecifier const * const *Bases, data().IsStandardLayout = false; data().IsCXX11StandardLayout = false; - // C++11 [dcl.constexpr]p4: - // In the definition of a constexpr constructor [...] - // -- the class shall not have any virtual base classes + // C++20 [dcl.constexpr]p3: + // In the definition of a constexpr function [...] + // -- if the function is a constructor or destructor, + // its class shall not have any virtual base classes data().DefaultedDefaultConstructorIsConstexpr = false; + data().DefaultedDestructorIsConstexpr = false; // C++1z [class.copy]p8: // The implicitly-declared copy constructor for a class X will have @@ -520,6 +523,19 @@ void CXXRecordDecl::addedClassSubobject(CXXRecordDecl *Subobj) { data().NeedOverloadResolutionForMoveConstructor = true; data().NeedOverloadResolutionForDestructor = true; } + + // C++2a [dcl.constexpr]p4: + // The definition of a constexpr destructor [shall] satisfy the + // following requirement: + // -- for every subobject of class type or (possibly multi-dimensional) + // array thereof, that class type shall have a constexpr destructor + if (!Subobj->hasConstexprDestructor()) + data().DefaultedDestructorIsConstexpr = false; +} + +bool CXXRecordDecl::hasConstexprDestructor() const { + auto *Dtor = getDestructor(); + return Dtor ? Dtor->isConstexpr() : defaultedDestructorIsConstexpr(); } bool CXXRecordDecl::hasAnyDependentBases() const { @@ -2571,20 +2587,19 @@ CXXDestructorDecl * CXXDestructorDecl::CreateDeserialized(ASTContext &C, unsigned ID) { return new (C, ID) CXXDestructorDecl(C, nullptr, SourceLocation(), DeclarationNameInfo(), - QualType(), nullptr, false, false); + QualType(), nullptr, false, false, CSK_unspecified); } -CXXDestructorDecl * -CXXDestructorDecl::Create(ASTContext &C, CXXRecordDecl *RD, - SourceLocation StartLoc, - const DeclarationNameInfo &NameInfo, - QualType T, TypeSourceInfo *TInfo, - bool isInline, bool isImplicitlyDeclared) { +CXXDestructorDecl *CXXDestructorDecl::Create( + ASTContext &C, CXXRecordDecl *RD, SourceLocation StartLoc, + const DeclarationNameInfo &NameInfo, QualType T, TypeSourceInfo *TInfo, + bool isInline, bool isImplicitlyDeclared, ConstexprSpecKind ConstexprKind) { assert(NameInfo.getName().getNameKind() == DeclarationName::CXXDestructorName && "Name must refer to a destructor"); - return new (C, RD) CXXDestructorDecl(C, RD, StartLoc, NameInfo, T, TInfo, - isInline, isImplicitlyDeclared); + return new (C, RD) + CXXDestructorDecl(C, RD, StartLoc, NameInfo, T, TInfo, isInline, + isImplicitlyDeclared, ConstexprKind); } void CXXDestructorDecl::setOperatorDelete(FunctionDecl *OD, Expr *ThisArg) { diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp index 4e371d7..fbec3ae 100644 --- a/clang/lib/AST/ExprConstant.cpp +++ b/clang/lib/AST/ExprConstant.cpp @@ -134,6 +134,14 @@ namespace { return E.getAsBaseOrMember().getInt(); } + /// Given an expression, determine the type used to store the result of + /// evaluating that expression. + static QualType getStorageType(ASTContext &Ctx, Expr *E) { + if (E->isRValue()) + return E->getType(); + return Ctx.getLValueReferenceType(E->getType()); + } + /// Given a CallExpr, try to get the alloc_size attribute. May return null. static const AllocSizeAttr *getAllocSizeAttr(const CallExpr *CE) { const FunctionDecl *Callee = CE->getDirectCallee(); @@ -572,7 +580,13 @@ namespace { return 0; } - APValue &createTemporary(const void *Key, bool IsLifetimeExtended); + /// Allocate storage for an object of type T in this stack frame. + /// Populates LV with a handle to the created object. Key identifies + /// the temporary within the stack frame, and must not be reused without + /// bumping the temporary version number. + template + APValue &createTemporary(const KeyT *Key, QualType T, + bool IsLifetimeExtended, LValue &LV); void describe(llvm::raw_ostream &OS) override; @@ -596,18 +610,33 @@ namespace { CallStackFrame &Frame; const LValue *OldThis; }; +} + +static bool HandleDestructorCall(EvalInfo &Info, APValue::LValueBase LVBase, + APValue &Value, QualType T); +namespace { /// A cleanup, and a flag indicating whether it is lifetime-extended. class Cleanup { llvm::PointerIntPair Value; + APValue::LValueBase Base; + QualType T; public: - Cleanup(APValue *Val, bool IsLifetimeExtended) - : Value(Val, IsLifetimeExtended) {} + Cleanup(APValue *Val, APValue::LValueBase Base, QualType T, + bool IsLifetimeExtended) + : Value(Val, IsLifetimeExtended), Base(Base), T(T) {} bool isLifetimeExtended() const { return Value.getInt(); } - void endLifetime() { + bool endLifetime(EvalInfo &Info, bool RunDestructors) { + if (RunDestructors && T.isDestructedType()) + return HandleDestructorCall(Info, Base, *Value.getPointer(), T); *Value.getPointer() = APValue(); + return true; + } + + bool hasSideEffect() { + return T.isDestructedType(); } }; @@ -623,7 +652,13 @@ namespace { return llvm::hash_combine(Obj.Base, Obj.Path); } }; - enum class ConstructionPhase { None, Bases, AfterBases }; + enum class ConstructionPhase { + None, + Bases, + AfterBases, + Destroying, + DestroyingBases + }; } namespace llvm { @@ -728,9 +763,29 @@ namespace { } }; + struct EvaluatingDestructorRAII { + EvalInfo &EI; + ObjectUnderConstruction Object; + EvaluatingDestructorRAII(EvalInfo &EI, ObjectUnderConstruction Object) + : EI(EI), Object(Object) { + bool DidInsert = EI.ObjectsUnderConstruction + .insert({Object, ConstructionPhase::Destroying}) + .second; + (void)DidInsert; + assert(DidInsert && "destroyed object multiple times"); + } + void startedDestroyingBases() { + EI.ObjectsUnderConstruction[Object] = + ConstructionPhase::DestroyingBases; + } + ~EvaluatingDestructorRAII() { + EI.ObjectsUnderConstruction.erase(Object); + } + }; + ConstructionPhase - isEvaluatingConstructor(APValue::LValueBase Base, - ArrayRef Path) { + isEvaluatingCtorDtor(APValue::LValueBase Base, + ArrayRef Path) { return ObjectsUnderConstruction.lookup({Base, Path}); } @@ -811,6 +866,10 @@ namespace { HasFoldFailureDiagnostic(false), InConstantContext(false), EvalMode(Mode) {} + ~EvalInfo() { + discardCleanups(); + } + void setEvaluatingDecl(APValue::LValueBase Base, APValue &Value) { EvaluatingDecl = Base; EvaluatingDeclValue = &Value; @@ -858,6 +917,25 @@ namespace { return true; } + void performLifetimeExtension() { + // Disable the cleanups for lifetime-extended temporaries. + CleanupStack.erase( + std::remove_if(CleanupStack.begin(), CleanupStack.end(), + [](Cleanup &C) { return C.isLifetimeExtended(); }), + CleanupStack.end()); + } + + /// Throw away any remaining cleanups at the end of evaluation. If any + /// cleanups would have had a side-effect, note that as an unmodeled + /// side-effect and return false. Otherwise, return true. + bool discardCleanups() { + for (Cleanup &C : CleanupStack) + if (C.hasSideEffect()) + if (!noteSideEffect()) + return false; + return true; + } + private: interp::Frame *getCurrentFrame() override { return CurrentCall; } const interp::Frame *getBottomFrame() const override { return &BottomFrame; } @@ -1101,29 +1179,37 @@ namespace { // temporaries created in different iterations of a loop. Info.CurrentCall->pushTempVersion(); } + bool destroy(bool RunDestructors = true) { + bool OK = cleanup(Info, RunDestructors, OldStackSize); + OldStackSize = -1U; + return OK; + } ~ScopeRAII() { + if (OldStackSize != -1U) + destroy(false); // Body moved to a static method to encourage the compiler to inline away // instances of this class. - cleanup(Info, OldStackSize); Info.CurrentCall->popTempVersion(); } private: - static void cleanup(EvalInfo &Info, unsigned OldStackSize) { - unsigned NewEnd = OldStackSize; - for (unsigned I = OldStackSize, N = Info.CleanupStack.size(); - I != N; ++I) { - if (IsFullExpression && Info.CleanupStack[I].isLifetimeExtended()) { - // Full-expression cleanup of a lifetime-extended temporary: nothing - // to do, just move this cleanup to the right place in the stack. - std::swap(Info.CleanupStack[I], Info.CleanupStack[NewEnd]); - ++NewEnd; - } else { - // End the lifetime of the object. - Info.CleanupStack[I].endLifetime(); + static bool cleanup(EvalInfo &Info, bool RunDestructors, unsigned OldStackSize) { + // Run all cleanups for a block scope, and non-lifetime-extended cleanups + // for a full-expression scope. + for (unsigned I = Info.CleanupStack.size(); I > OldStackSize; --I) { + if (!(IsFullExpression && Info.CleanupStack[I-1].isLifetimeExtended())) { + if (!Info.CleanupStack[I-1].endLifetime(Info, RunDestructors)) + return false; } } - Info.CleanupStack.erase(Info.CleanupStack.begin() + NewEnd, - Info.CleanupStack.end()); + + // Compact lifetime-extended cleanups. + auto NewEnd = Info.CleanupStack.begin() + OldStackSize; + if (IsFullExpression) + NewEnd = + std::remove_if(NewEnd, Info.CleanupStack.end(), + [](Cleanup &C) { return !C.isLifetimeExtended(); }); + Info.CleanupStack.erase(NewEnd, Info.CleanupStack.end()); + return true; } }; typedef ScopeRAII BlockScopeRAII; @@ -1183,15 +1269,6 @@ CallStackFrame::~CallStackFrame() { Info.CurrentCall = Caller; } -APValue &CallStackFrame::createTemporary(const void *Key, - bool IsLifetimeExtended) { - unsigned Version = Info.CurrentCall->getTempVersion(); - APValue &Result = Temporaries[MapKeyTy(Key, Version)]; - assert(Result.isAbsent() && "temporary created multiple times"); - Info.CleanupStack.push_back(Cleanup(&Result, IsLifetimeExtended)); - return Result; -} - static bool isRead(AccessKinds AK) { return AK == AK_Read || AK == AK_ReadObjectRepresentation; } @@ -1544,15 +1621,6 @@ static bool EvaluateFixedPoint(const Expr *E, APFixedPoint &Result, // Misc utilities //===----------------------------------------------------------------------===// -/// A helper function to create a temporary and set an LValue. -template -static APValue &createTemporary(const KeyTy *Key, bool IsLifetimeExtended, - LValue &LV, CallStackFrame &Frame) { - LV.set({Key, Frame.Info.CurrentCall->Index, - Frame.Info.CurrentCall->getTempVersion()}); - return Frame.createTemporary(Key, IsLifetimeExtended); -} - /// Negate an APSInt in place, converting it to a signed form if necessary, and /// preserving its value (by extending by up to one bit as needed). static void negateAsSigned(APSInt &Int) { @@ -1563,6 +1631,18 @@ static void negateAsSigned(APSInt &Int) { Int = -Int; } +template +APValue &CallStackFrame::createTemporary(const KeyT *Key, QualType T, + bool IsLifetimeExtended, LValue &LV) { + unsigned Version = Info.CurrentCall->getTempVersion(); + APValue::LValueBase Base(Key, Index, Version); + LV.set(Base); + APValue &Result = Temporaries[MapKeyTy(Key, Version)]; + assert(Result.isAbsent() && "temporary created multiple times"); + Info.CleanupStack.push_back(Cleanup(&Result, Base, T, IsLifetimeExtended)); + return Result; +} + /// Produce a string describing the given constexpr call. void CallStackFrame::describe(raw_ostream &Out) { unsigned ArgIndex = 0; @@ -2858,12 +2938,12 @@ findSubobject(EvalInfo &Info, const Expr *E, const CompleteObject &Obj, return handler.failed(); } - // C++ [class.ctor]p5: + // C++ [class.ctor]p5, C++ [class.dtor]p5: // const and volatile semantics are not applied on an object under - // construction. + // {con,de}struction. if ((ObjType.isConstQualified() || ObjType.isVolatileQualified()) && ObjType->isRecordType() && - Info.isEvaluatingConstructor( + Info.isEvaluatingCtorDtor( Obj.Base, llvm::makeArrayRef(Sub.Entries.begin(), Sub.Entries.begin() + I)) != ConstructionPhase::None) { @@ -3938,7 +4018,8 @@ static bool EvaluateVarDecl(EvalInfo &Info, const VarDecl *VD) { return true; LValue Result; - APValue &Val = createTemporary(VD, true, Result, *Info.CurrentCall); + APValue &Val = + Info.CurrentCall->createTemporary(VD, VD->getType(), true, Result); const Expr *InitE = VD->getInit(); if (!InitE) { @@ -3980,7 +4061,9 @@ static bool EvaluateCond(EvalInfo &Info, const VarDecl *CondDecl, FullExpressionRAII Scope(Info); if (CondDecl && !EvaluateDecl(Info, CondDecl)) return false; - return EvaluateAsBooleanCondition(Cond, Result, Info); + if (!EvaluateAsBooleanCondition(Cond, Result, Info)) + return false; + return Scope.destroy(); } namespace { @@ -4016,7 +4099,12 @@ static EvalStmtResult EvaluateLoopBody(StmtResult &Result, EvalInfo &Info, const Stmt *Body, const SwitchCase *Case = nullptr) { BlockScopeRAII Scope(Info); - switch (EvalStmtResult ESR = EvaluateStmt(Result, Info, Body, Case)) { + + EvalStmtResult ESR = EvaluateStmt(Result, Info, Body, Case); + if (ESR != ESR_Failed && ESR != ESR_CaseNotFound && !Scope.destroy()) + ESR = ESR_Failed; + + switch (ESR) { case ESR_Break: return ESR_Succeeded; case ESR_Succeeded: @@ -4038,17 +4126,23 @@ static EvalStmtResult EvaluateSwitch(StmtResult &Result, EvalInfo &Info, // Evaluate the switch condition. APSInt Value; { - FullExpressionRAII Scope(Info); if (const Stmt *Init = SS->getInit()) { EvalStmtResult ESR = EvaluateStmt(Result, Info, Init); - if (ESR != ESR_Succeeded) + if (ESR != ESR_Succeeded) { + if (ESR != ESR_Failed && !Scope.destroy()) + ESR = ESR_Failed; return ESR; + } } + + FullExpressionRAII CondScope(Info); if (SS->getConditionVariable() && !EvaluateDecl(Info, SS->getConditionVariable())) return ESR_Failed; if (!EvaluateInteger(SS->getCond(), Value, Info)) return ESR_Failed; + if (!CondScope.destroy()) + return ESR_Failed; } // Find the switch case corresponding to the value of the condition. @@ -4072,10 +4166,14 @@ static EvalStmtResult EvaluateSwitch(StmtResult &Result, EvalInfo &Info, } if (!Found) - return ESR_Succeeded; + return Scope.destroy() ? ESR_Failed : ESR_Succeeded; // Search the switch body for the switch case and evaluate it from there. - switch (EvalStmtResult ESR = EvaluateStmt(Result, Info, SS->getBody(), Found)) { + EvalStmtResult ESR = EvaluateStmt(Result, Info, SS->getBody(), Found); + if (ESR != ESR_Failed && ESR != ESR_CaseNotFound && !Scope.destroy()) + return ESR_Failed; + + switch (ESR) { case ESR_Break: return ESR_Succeeded; case ESR_Succeeded: @@ -4143,9 +4241,19 @@ static EvalStmtResult EvaluateStmt(StmtResult &Result, EvalInfo &Info, // (The same is true for 'for' statements.) EvalStmtResult ESR = EvaluateStmt(Result, Info, IS->getThen(), Case); - if (ESR != ESR_CaseNotFound || !IS->getElse()) + if (ESR == ESR_Failed) + return ESR; + if (ESR != ESR_CaseNotFound) + return Scope.destroy() ? ESR : ESR_Failed; + if (!IS->getElse()) + return ESR_CaseNotFound; + + ESR = EvaluateStmt(Result, Info, IS->getElse(), Case); + if (ESR == ESR_Failed) return ESR; - return EvaluateStmt(Result, Info, IS->getElse(), Case); + if (ESR != ESR_CaseNotFound) + return Scope.destroy() ? ESR : ESR_Failed; + return ESR_CaseNotFound; } case Stmt::WhileStmtClass: { @@ -4176,7 +4284,7 @@ static EvalStmtResult EvaluateStmt(StmtResult &Result, EvalInfo &Info, return ESR; if (FS->getInc()) { FullExpressionRAII IncScope(Info); - if (!EvaluateIgnoredValue(Info, FS->getInc())) + if (!EvaluateIgnoredValue(Info, FS->getInc()) || !IncScope.destroy()) return ESR_Failed; } break; @@ -4209,8 +4317,10 @@ static EvalStmtResult EvaluateStmt(StmtResult &Result, EvalInfo &Info, if (const Expr *E = dyn_cast(S)) { // Don't bother evaluating beyond an expression-statement which couldn't // be evaluated. + // FIXME: Do we need the FullExpressionRAII object here? + // VisitExprWithCleanups should create one when necessary. FullExpressionRAII Scope(Info); - if (!EvaluateIgnoredValue(Info, E)) + if (!EvaluateIgnoredValue(Info, E) || !Scope.destroy()) return ESR_Failed; return ESR_Succeeded; } @@ -4228,6 +4338,8 @@ static EvalStmtResult EvaluateStmt(StmtResult &Result, EvalInfo &Info, FullExpressionRAII Scope(Info); if (!EvaluateDecl(Info, D) && !Info.noteFailure()) return ESR_Failed; + if (!Scope.destroy()) + return ESR_Failed; } return ESR_Succeeded; } @@ -4240,7 +4352,7 @@ static EvalStmtResult EvaluateStmt(StmtResult &Result, EvalInfo &Info, ? EvaluateInPlace(Result.Value, Info, *Result.Slot, RetExpr) : Evaluate(Result.Value, Info, RetExpr))) return ESR_Failed; - return ESR_Returned; + return Scope.destroy() ? ESR_Returned : ESR_Failed; } case Stmt::CompoundStmtClass: { @@ -4251,10 +4363,15 @@ static EvalStmtResult EvaluateStmt(StmtResult &Result, EvalInfo &Info, EvalStmtResult ESR = EvaluateStmt(Result, Info, BI, Case); if (ESR == ESR_Succeeded) Case = nullptr; - else if (ESR != ESR_CaseNotFound) + else if (ESR != ESR_CaseNotFound) { + if (ESR != ESR_Failed && !Scope.destroy()) + return ESR_Failed; return ESR; + } } - return Case ? ESR_CaseNotFound : ESR_Succeeded; + if (Case) + return ESR_CaseNotFound; + return Scope.destroy() ? ESR_Succeeded : ESR_Failed; } case Stmt::IfStmtClass: { @@ -4264,8 +4381,11 @@ static EvalStmtResult EvaluateStmt(StmtResult &Result, EvalInfo &Info, BlockScopeRAII Scope(Info); if (const Stmt *Init = IS->getInit()) { EvalStmtResult ESR = EvaluateStmt(Result, Info, Init); - if (ESR != ESR_Succeeded) + if (ESR != ESR_Succeeded) { + if (ESR != ESR_Failed && !Scope.destroy()) + return ESR_Failed; return ESR; + } } bool Cond; if (!EvaluateCond(Info, IS->getConditionVariable(), IS->getCond(), Cond)) @@ -4273,10 +4393,13 @@ static EvalStmtResult EvaluateStmt(StmtResult &Result, EvalInfo &Info, if (const Stmt *SubStmt = Cond ? IS->getThen() : IS->getElse()) { EvalStmtResult ESR = EvaluateStmt(Result, Info, SubStmt); - if (ESR != ESR_Succeeded) + if (ESR != ESR_Succeeded) { + if (ESR != ESR_Failed && !Scope.destroy()) + return ESR_Failed; return ESR; + } } - return ESR_Succeeded; + return Scope.destroy() ? ESR_Succeeded : ESR_Failed; } case Stmt::WhileStmtClass: { @@ -4291,8 +4414,13 @@ static EvalStmtResult EvaluateStmt(StmtResult &Result, EvalInfo &Info, break; EvalStmtResult ESR = EvaluateLoopBody(Result, Info, WS->getBody()); - if (ESR != ESR_Continue) + if (ESR != ESR_Continue) { + if (ESR != ESR_Failed && !Scope.destroy()) + return ESR_Failed; return ESR; + } + if (!Scope.destroy()) + return ESR_Failed; } return ESR_Succeeded; } @@ -4307,7 +4435,8 @@ static EvalStmtResult EvaluateStmt(StmtResult &Result, EvalInfo &Info, Case = nullptr; FullExpressionRAII CondScope(Info); - if (!EvaluateAsBooleanCondition(DS->getCond(), Continue, Info)) + if (!EvaluateAsBooleanCondition(DS->getCond(), Continue, Info) || + !CondScope.destroy()) return ESR_Failed; } while (Continue); return ESR_Succeeded; @@ -4315,14 +4444,17 @@ static EvalStmtResult EvaluateStmt(StmtResult &Result, EvalInfo &Info, case Stmt::ForStmtClass: { const ForStmt *FS = cast(S); - BlockScopeRAII Scope(Info); + BlockScopeRAII ForScope(Info); if (FS->getInit()) { EvalStmtResult ESR = EvaluateStmt(Result, Info, FS->getInit()); - if (ESR != ESR_Succeeded) + if (ESR != ESR_Succeeded) { + if (ESR != ESR_Failed && !ForScope.destroy()) + return ESR_Failed; return ESR; + } } while (true) { - BlockScopeRAII Scope(Info); + BlockScopeRAII IterScope(Info); bool Continue = true; if (FS->getCond() && !EvaluateCond(Info, FS->getConditionVariable(), FS->getCond(), Continue)) @@ -4331,16 +4463,22 @@ static EvalStmtResult EvaluateStmt(StmtResult &Result, EvalInfo &Info, break; EvalStmtResult ESR = EvaluateLoopBody(Result, Info, FS->getBody()); - if (ESR != ESR_Continue) + if (ESR != ESR_Continue) { + if (ESR != ESR_Failed && (!IterScope.destroy() || !ForScope.destroy())) + return ESR_Failed; return ESR; + } if (FS->getInc()) { FullExpressionRAII IncScope(Info); - if (!EvaluateIgnoredValue(Info, FS->getInc())) + if (!EvaluateIgnoredValue(Info, FS->getInc()) || !IncScope.destroy()) return ESR_Failed; } + + if (!IterScope.destroy()) + return ESR_Failed; } - return ESR_Succeeded; + return ForScope.destroy() ? ESR_Succeeded : ESR_Failed; } case Stmt::CXXForRangeStmtClass: { @@ -4350,22 +4488,34 @@ static EvalStmtResult EvaluateStmt(StmtResult &Result, EvalInfo &Info, // Evaluate the init-statement if present. if (FS->getInit()) { EvalStmtResult ESR = EvaluateStmt(Result, Info, FS->getInit()); - if (ESR != ESR_Succeeded) + if (ESR != ESR_Succeeded) { + if (ESR != ESR_Failed && !Scope.destroy()) + return ESR_Failed; return ESR; + } } // Initialize the __range variable. EvalStmtResult ESR = EvaluateStmt(Result, Info, FS->getRangeStmt()); - if (ESR != ESR_Succeeded) + if (ESR != ESR_Succeeded) { + if (ESR != ESR_Failed && !Scope.destroy()) + return ESR_Failed; return ESR; + } // Create the __begin and __end iterators. ESR = EvaluateStmt(Result, Info, FS->getBeginStmt()); - if (ESR != ESR_Succeeded) + if (ESR != ESR_Succeeded) { + if (ESR != ESR_Failed && !Scope.destroy()) + return ESR_Failed; return ESR; + } ESR = EvaluateStmt(Result, Info, FS->getEndStmt()); - if (ESR != ESR_Succeeded) + if (ESR != ESR_Succeeded) { + if (ESR != ESR_Failed && !Scope.destroy()) + return ESR_Failed; return ESR; + } while (true) { // Condition: __begin != __end. @@ -4381,20 +4531,29 @@ static EvalStmtResult EvaluateStmt(StmtResult &Result, EvalInfo &Info, // User's variable declaration, initialized by *__begin. BlockScopeRAII InnerScope(Info); ESR = EvaluateStmt(Result, Info, FS->getLoopVarStmt()); - if (ESR != ESR_Succeeded) + if (ESR != ESR_Succeeded) { + if (ESR != ESR_Failed && (!InnerScope.destroy() || !Scope.destroy())) + return ESR_Failed; return ESR; + } // Loop body. ESR = EvaluateLoopBody(Result, Info, FS->getBody()); - if (ESR != ESR_Continue) + if (ESR != ESR_Continue) { + if (ESR != ESR_Failed && (!InnerScope.destroy() || !Scope.destroy())) + return ESR_Failed; return ESR; + } // Increment: ++__begin if (!EvaluateIgnoredValue(Info, FS->getInc())) return ESR_Failed; + + if (!InnerScope.destroy()) + return ESR_Failed; } - return ESR_Succeeded; + return Scope.destroy() ? ESR_Succeeded : ESR_Failed; } case Stmt::SwitchStmtClass: @@ -4619,16 +4778,19 @@ static Optional ComputeDynamicType(EvalInfo &Info, const Expr *E, ArrayRef Path = This.Designator.Entries; for (unsigned PathLength = This.Designator.MostDerivedPathLength; PathLength <= Path.size(); ++PathLength) { - switch (Info.isEvaluatingConstructor(This.getLValueBase(), - Path.slice(0, PathLength))) { + switch (Info.isEvaluatingCtorDtor(This.getLValueBase(), + Path.slice(0, PathLength))) { case ConstructionPhase::Bases: - // We're constructing a base class. This is not the dynamic type. + case ConstructionPhase::DestroyingBases: + // We're constructing or destroying a base class. This is not the dynamic + // type. break; case ConstructionPhase::None: case ConstructionPhase::AfterBases: - // We've finished constructing the base classes, so this is the dynamic - // type. + case ConstructionPhase::Destroying: + // We've finished constructing the base classes and not yet started + // destroying them again, so this is the dynamic type. return DynamicType{getBaseClassType(This.Designator, PathLength), PathLength}; } @@ -5117,7 +5279,8 @@ static bool HandleConstructorCall(const Expr *E, const LValue &This, CXXConstructorDecl::init_const_iterator I = Definition->init_begin(); { FullExpressionRAII InitScope(Info); - if (!EvaluateInPlace(Result, Info, This, (*I)->getInit())) + if (!EvaluateInPlace(Result, Info, This, (*I)->getInit()) || + !InitScope.destroy()) return false; } return EvaluateStmt(Ret, Info, Definition->getBody()) != ESR_Failed; @@ -5280,7 +5443,8 @@ static bool HandleConstructorCall(const Expr *E, const LValue &This, } return Success && - EvaluateStmt(Ret, Info, Definition->getBody()) != ESR_Failed; + EvaluateStmt(Ret, Info, Definition->getBody()) != ESR_Failed && + LifetimeExtendedScope.destroy(); } static bool HandleConstructorCall(const Expr *E, const LValue &This, @@ -5295,6 +5459,158 @@ static bool HandleConstructorCall(const Expr *E, const LValue &This, Info, Result); } +static bool HandleDestructorCallImpl(EvalInfo &Info, SourceLocation CallLoc, + const LValue &This, APValue &Value, + QualType T) { + // Invent an expression for location purposes. + // FIXME: We shouldn't need to do this. + OpaqueValueExpr LocE(CallLoc, Info.Ctx.IntTy, VK_RValue); + + // For arrays, destroy elements right-to-left. + if (const ConstantArrayType *CAT = Info.Ctx.getAsConstantArrayType(T)) { + uint64_t Size = CAT->getSize().getZExtValue(); + QualType ElemT = CAT->getElementType(); + + LValue ElemLV = This; + ElemLV.addArray(Info, &LocE, CAT); + if (!HandleLValueArrayAdjustment(Info, &LocE, ElemLV, ElemT, Size)) + return false; + + for (; Size != 0; --Size) { + APValue &Elem = Value.getArrayInitializedElt(Size - 1); + if (!HandleDestructorCallImpl(Info, CallLoc, ElemLV, Elem, ElemT) || + !HandleLValueArrayAdjustment(Info, &LocE, ElemLV, ElemT, -1)) + return false; + } + + // End the lifetime of this array now. + Value = APValue(); + return true; + } + + const CXXRecordDecl *RD = T->getAsCXXRecordDecl(); + if (!RD) { + if (T.isDestructedType()) { + Info.FFDiag(CallLoc, diag::note_constexpr_unsupported_destruction) << T; + return false; + } + + Value = APValue(); + return true; + } + + if (RD->getNumVBases()) { + Info.FFDiag(CallLoc, diag::note_constexpr_virtual_base) << RD; + return false; + } + + const CXXDestructorDecl *DD = RD->getDestructor(); + if (!DD) { + // FIXME: Can we get here for a type with an irrelevant destructor? + Info.FFDiag(CallLoc); + return false; + } + + const FunctionDecl *Definition = nullptr; + const Stmt *Body = DD->getBody(Definition); + + if ((DD && DD->isTrivial()) || + (RD->isAnonymousStructOrUnion() && RD->isUnion())) { + // A trivial destructor just ends the lifetime of the object. Check for + // this case before checking for a body, because we might not bother + // building a body for a trivial destructor. Note that it doesn't matter + // whether the destructor is constexpr in this case; all trivial + // destructors are constexpr. + // + // If an anonymous union would be destroyed, some enclosing destructor must + // have been explicitly defined, and the anonymous union destruction should + // have no effect. + Value = APValue(); + return true; + } + + if (!Info.CheckCallLimit(CallLoc)) + return false; + + if (!CheckConstexprFunction(Info, CallLoc, DD, Definition, Body)) + return false; + + CallStackFrame Frame(Info, CallLoc, Definition, &This, nullptr); + + // We're now in the period of destruction of this object. + unsigned BasesLeft = RD->getNumBases(); + EvalInfo::EvaluatingDestructorRAII EvalObj( + Info, + ObjectUnderConstruction{This.getLValueBase(), This.Designator.Entries}); + + // FIXME: Creating an APValue just to hold a nonexistent return value is + // wasteful. + APValue RetVal; + StmtResult Ret = {RetVal, nullptr}; + if (EvaluateStmt(Ret, Info, Definition->getBody()) == ESR_Failed) + return false; + + // A union destructor does not implicitly destroy its members. + if (RD->isUnion()) + return true; + + const ASTRecordLayout &Layout = Info.Ctx.getASTRecordLayout(RD); + + // We don't have a good way to iterate fields in reverse, so collect all the + // fields first and then walk them backwards. + SmallVector Fields(RD->field_begin(), RD->field_end()); + for (const FieldDecl *FD : llvm::reverse(Fields)) { + if (FD->isUnnamedBitfield()) + continue; + + LValue Subobject = This; + if (!HandleLValueMember(Info, &LocE, Subobject, FD, &Layout)) + return false; + + APValue *SubobjectValue = &Value.getStructField(FD->getFieldIndex()); + if (!HandleDestructorCallImpl(Info, CallLoc, Subobject, *SubobjectValue, + FD->getType())) + return false; + } + + if (BasesLeft != 0) + EvalObj.startedDestroyingBases(); + + // Destroy base classes in reverse order. + for (const CXXBaseSpecifier &Base : llvm::reverse(RD->bases())) { + --BasesLeft; + + QualType BaseType = Base.getType(); + LValue Subobject = This; + if (!HandleLValueDirectBase(Info, &LocE, Subobject, RD, + BaseType->getAsCXXRecordDecl(), &Layout)) + return false; + + APValue *SubobjectValue = &Value.getStructBase(BasesLeft); + if (!HandleDestructorCallImpl(Info, CallLoc, Subobject, *SubobjectValue, + BaseType)) + return false; + } + assert(BasesLeft == 0 && "NumBases was wrong?"); + + // The period of destruction ends now. The object is gone. + Value = APValue(); + return true; +} + +static bool HandleDestructorCall(EvalInfo &Info, APValue::LValueBase LVBase, + APValue &Value, QualType T) { + SourceLocation Loc; + if (const ValueDecl *VD = LVBase.dyn_cast()) + Loc = VD->getLocation(); + else if (const Expr *E = LVBase.dyn_cast()) + Loc = E->getExprLoc(); + + LValue LV; + LV.set({LVBase}); + return HandleDestructorCallImpl(Info, Loc, LV, Value, T); +} + //===----------------------------------------------------------------------===// // Generic Evaluation //===----------------------------------------------------------------------===// @@ -5905,10 +6221,10 @@ public: return StmtVisitorTy::Visit(E->getExpr()); } - // We cannot create any objects for which cleanups are required, so there is - // nothing to do here; all cleanups must come from unevaluated subexpressions. - bool VisitExprWithCleanups(const ExprWithCleanups *E) - { return StmtVisitorTy::Visit(E->getSubExpr()); } + bool VisitExprWithCleanups(const ExprWithCleanups *E) { + FullExpressionRAII Scope(Info); + return StmtVisitorTy::Visit(E->getSubExpr()) && Scope.destroy(); + } bool VisitCXXReinterpretCastExpr(const CXXReinterpretCastExpr *E) { CCEDiag(E, diag::note_constexpr_invalid_cast) << 0; @@ -5948,7 +6264,10 @@ public: bool VisitBinaryConditionalOperator(const BinaryConditionalOperator *E) { // Evaluate and cache the common expression. We treat it as a temporary, // even though it's not quite the same thing. - if (!Evaluate(Info.CurrentCall->createTemporary(E->getOpaqueValue(), false), + LValue CommonLV; + if (!Evaluate(Info.CurrentCall->createTemporary( + E->getOpaqueValue(), getStorageType(Info.Ctx, E->getOpaqueValue()), + false, CommonLV), Info, E->getCommon())) return false; @@ -6255,11 +6574,11 @@ public: if (Info.checkingForUndefinedBehavior()) return Error(E); - BlockScopeRAII Scope(Info); const CompoundStmt *CS = E->getSubStmt(); if (CS->body_empty()) return true; + BlockScopeRAII Scope(Info); for (CompoundStmt::const_body_iterator BI = CS->body_begin(), BE = CS->body_end(); /**/; ++BI) { @@ -6270,7 +6589,7 @@ public: diag::note_constexpr_stmt_expr_unsupported); return false; } - return this->Visit(FinalExpr); + return this->Visit(FinalExpr) && Scope.destroy(); } APValue ReturnValue; @@ -6619,8 +6938,8 @@ bool LValueExprEvaluator::VisitMaterializeTemporaryExpr( *Value = APValue(); Result.set(E); } else { - Value = &createTemporary(E, E->getStorageDuration() == SD_Automatic, Result, - *Info.CurrentCall); + Value = &Info.CurrentCall->createTemporary( + E, E->getType(), E->getStorageDuration() == SD_Automatic, Result); } QualType Type = Inner->getType(); @@ -7152,8 +7471,8 @@ bool PointerExprEvaluator::VisitCastExpr(const CastExpr *E) { if (!evaluateLValue(SubExpr, Result)) return false; } else { - APValue &Value = createTemporary(SubExpr, false, Result, - *Info.CurrentCall); + APValue &Value = Info.CurrentCall->createTemporary( + SubExpr, SubExpr->getType(), false, Result); if (!EvaluateInPlace(Value, Info, Result, SubExpr)) return false; } @@ -7705,6 +8024,12 @@ namespace { bool VisitCXXConstructExpr(const CXXConstructExpr *E, QualType T); bool VisitCXXStdInitializerListExpr(const CXXStdInitializerListExpr *E); + // Temporaries are registered when created, so we don't care about + // CXXBindTemporaryExpr. + bool VisitCXXBindTemporaryExpr(const CXXBindTemporaryExpr *E) { + return Visit(E->getSubExpr()); + } + bool VisitBinCmp(const BinaryOperator *E); }; } @@ -8104,7 +8429,8 @@ public: /// Visit an expression which constructs the value of this temporary. bool VisitConstructExpr(const Expr *E) { - APValue &Value = createTemporary(E, false, Result, *Info.CurrentCall); + APValue &Value = + Info.CurrentCall->createTemporary(E, E->getType(), false, Result); return EvaluateInPlace(Value, Info, Result, E); } @@ -8464,8 +8790,12 @@ bool ArrayExprEvaluator::VisitInitListExpr(const InitListExpr *E) { } bool ArrayExprEvaluator::VisitArrayInitLoopExpr(const ArrayInitLoopExpr *E) { + LValue CommonLV; if (E->getCommonExpr() && - !Evaluate(Info.CurrentCall->createTemporary(E->getCommonExpr(), false), + !Evaluate(Info.CurrentCall->createTemporary( + E->getCommonExpr(), + getStorageType(Info.Ctx, E->getCommonExpr()), false, + CommonLV), Info, E->getCommonExpr()->getSourceExpr())) return false; @@ -12124,13 +12454,14 @@ static bool Evaluate(APValue &Result, EvalInfo &Info, const Expr *E) { return true; } else if (T->isArrayType()) { LValue LV; - APValue &Value = createTemporary(E, false, LV, *Info.CurrentCall); + APValue &Value = + Info.CurrentCall->createTemporary(E, T, false, LV); if (!EvaluateArray(E, LV, Value, Info)) return false; Result = Value; } else if (T->isRecordType()) { LValue LV; - APValue &Value = createTemporary(E, false, LV, *Info.CurrentCall); + APValue &Value = Info.CurrentCall->createTemporary(E, T, false, LV); if (!EvaluateRecord(E, LV, Value, Info)) return false; Result = Value; @@ -12144,7 +12475,7 @@ static bool Evaluate(APValue &Result, EvalInfo &Info, const Expr *E) { QualType Unqual = T.getAtomicUnqualifiedType(); if (Unqual->isArrayType() || Unqual->isRecordType()) { LValue LV; - APValue &Value = createTemporary(E, false, LV, *Info.CurrentCall); + APValue &Value = Info.CurrentCall->createTemporary(E, Unqual, false, LV); if (!EvaluateAtomic(E, &LV, Value, Info)) return false; } else { @@ -12372,7 +12703,8 @@ bool Expr::EvaluateAsLValue(EvalResult &Result, const ASTContext &Ctx, EvalInfo Info(Ctx, Result, EvalInfo::EM_ConstantFold); Info.InConstantContext = InConstantContext; LValue LV; - if (!EvaluateLValue(this, LV, Info) || Result.HasSideEffects || + if (!EvaluateLValue(this, LV, Info) || !Info.discardCleanups() || + Result.HasSideEffects || !CheckLValueConstantExpression(Info, getExprLoc(), Ctx.getLValueReferenceType(getType()), LV, Expr::EvaluateForCodeGen)) @@ -12394,6 +12726,9 @@ bool Expr::EvaluateAsConstantExpr(EvalResult &Result, ConstExprUsage Usage, if (!::Evaluate(Result.Val, Info, this)) return false; + if (!Info.discardCleanups()) + llvm_unreachable("Unhandled cleanup; missing full expression marker?"); + return CheckConstantExpression(Info, getExprLoc(), getType(), Result.Val, Usage); } @@ -12457,6 +12792,13 @@ bool Expr::EvaluateAsInitializer(APValue &Value, const ASTContext &Ctx, EStatus.HasSideEffects) return false; + // At this point, any lifetime-extended temporaries are completely + // initialized. + Info.performLifetimeExtension(); + + if (!Info.discardCleanups()) + llvm_unreachable("Unhandled cleanup; missing full expression marker?"); + return CheckConstantExpression(Info, DeclLoc, DeclTy, Value); } @@ -13063,7 +13405,11 @@ bool Expr::isCXX11ConstantExpr(const ASTContext &Ctx, APValue *Result, EvalInfo Info(Ctx, Status, EvalInfo::EM_ConstantExpression); APValue Scratch; - bool IsConstExpr = ::EvaluateAsRValue(Info, this, Result ? *Result : Scratch); + bool IsConstExpr = + ::EvaluateAsRValue(Info, this, Result ? *Result : Scratch) && + // FIXME: We don't produce a diagnostic for this, but the callers that + // call us on arbitrary full-expressions should generally not care. + Info.discardCleanups(); if (!Diags.empty()) { IsConstExpr = false; @@ -13115,7 +13461,8 @@ bool Expr::EvaluateWithSubstitution(APValue &Value, ASTContext &Ctx, // Build fake call to Callee. CallStackFrame Frame(Info, Callee->getLocation(), Callee, ThisPtr, ArgValues.data()); - return Evaluate(Value, Info, this) && !Info.EvalStatus.HasSideEffects; + return Evaluate(Value, Info, this) && Info.discardCleanups() && + !Info.EvalStatus.HasSideEffects; } bool Expr::isPotentialConstantExpr(const FunctionDecl *FD, diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp index dead3b6..42ce5f1 100644 --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -8172,10 +8172,10 @@ static FunctionDecl *CreateNewFunctionDecl(Sema &SemaRef, Declarator &D, if (DC->isRecord()) { R = SemaRef.CheckDestructorDeclarator(D, R, SC); CXXRecordDecl *Record = cast(DC); - CXXDestructorDecl *NewDD = - CXXDestructorDecl::Create(SemaRef.Context, Record, D.getBeginLoc(), - NameInfo, R, TInfo, isInline, - /*isImplicitlyDeclared=*/false); + CXXDestructorDecl *NewDD = CXXDestructorDecl::Create( + SemaRef.Context, Record, D.getBeginLoc(), NameInfo, R, TInfo, + isInline, + /*isImplicitlyDeclared=*/false, ConstexprKind); // If the destructor needs an implicit exception specification, set it // now. FIXME: It'd be nice to be able to create the right type to start @@ -8784,7 +8784,7 @@ Sema::ActOnFunctionDeclarator(Scope *S, Declarator &D, DeclContext *DC, // C++11 [dcl.constexpr]p3: functions declared constexpr are required to // be either constructors or to return a literal type. Therefore, // destructors cannot be declared constexpr. - if (isa(NewFD)) { + if (isa(NewFD) && !getLangOpts().CPlusPlus2a) { Diag(D.getDeclSpec().getConstexprSpecLoc(), diag::err_constexpr_dtor) << ConstexprKind; } @@ -10277,7 +10277,7 @@ bool Sema::CheckFunctionDeclaration(Scope *S, FunctionDecl *NewFD, CXXMethodDecl *MD = dyn_cast(NewFD); if (!getLangOpts().CPlusPlus14 && MD && MD->isConstexpr() && !MD->isStatic() && !isa(MD) && - !MD->getMethodQualifiers().hasConst()) { + !isa(MD) && !MD->getMethodQualifiers().hasConst()) { CXXMethodDecl *OldMD = nullptr; if (OldDecl) OldMD = dyn_cast_or_null(OldDecl->getAsFunction()); diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp index 9a1a9c0..ff2cc7f 100644 --- a/clang/lib/Sema/SemaDeclCXX.cpp +++ b/clang/lib/Sema/SemaDeclCXX.cpp @@ -1590,6 +1590,36 @@ static bool CheckLiteralType(Sema &SemaRef, Sema::CheckConstexprKind Kind, llvm_unreachable("unknown CheckConstexprKind"); } +/// Determine whether a destructor cannot be constexpr due to +static bool CheckConstexprDestructorSubobjects(Sema &SemaRef, + const CXXDestructorDecl *DD, + Sema::CheckConstexprKind Kind) { + auto Check = [&](SourceLocation Loc, QualType T, const FieldDecl *FD) { + const CXXRecordDecl *RD = + T->getBaseElementTypeUnsafe()->getAsCXXRecordDecl(); + if (!RD || RD->hasConstexprDestructor()) + return true; + + if (Kind == Sema::CheckConstexprKind::Diagnose) { + SemaRef.Diag(DD->getLocation(), diag::err_constexpr_dtor_subobject) + << DD->getConstexprKind() << !FD + << (FD ? FD->getDeclName() : DeclarationName()) << T; + SemaRef.Diag(Loc, diag::note_constexpr_dtor_subobject) + << !FD << (FD ? FD->getDeclName() : DeclarationName()) << T; + } + return false; + }; + + const CXXRecordDecl *RD = DD->getParent(); + for (const CXXBaseSpecifier &B : RD->bases()) + if (!Check(B.getBaseTypeLoc(), B.getType(), nullptr)) + return false; + for (const FieldDecl *FD : RD->fields()) + if (!Check(FD->getLocation(), FD->getType(), FD)) + return false; + return true; +} + // CheckConstexprParameterTypes - Check whether a function's parameter types // are all literal types. If so, return true. If not, produce a suitable // diagnostic and return false. @@ -1645,8 +1675,8 @@ bool Sema::CheckConstexprFunctionDefinition(const FunctionDecl *NewFD, // constraints: // - the class shall not have any virtual base classes; // - // FIXME: This only applies to constructors, not arbitrary member - // functions. + // FIXME: This only applies to constructors and destructors, not arbitrary + // member functions. const CXXRecordDecl *RD = MD->getParent(); if (RD->getNumVBases()) { if (Kind == CheckConstexprKind::CheckValid) @@ -1699,6 +1729,18 @@ bool Sema::CheckConstexprFunctionDefinition(const FunctionDecl *NewFD, return false; } + if (auto *Dtor = dyn_cast(NewFD)) { + // A destructor can be constexpr only if the defaulted destructor could be; + // we don't need to check the members and bases if we already know they all + // have constexpr destructors. + if (!Dtor->getParent()->defaultedDestructorIsConstexpr()) { + if (Kind == CheckConstexprKind::CheckValid) + return false; + if (!CheckConstexprDestructorSubobjects(*this, Dtor, Kind)) + return false; + } + } + // - each of its parameter types shall be a literal type; if (!CheckConstexprParameterTypes(*this, NewFD, Kind)) return false; @@ -6535,6 +6577,8 @@ specialMemberIsConstexpr(Sema &S, CXXRecordDecl *ClassDecl, if (CSM == Sema::CXXDefaultConstructor) return ClassDecl->hasConstexprDefaultConstructor(); + if (CSM == Sema::CXXDestructor) + return ClassDecl->hasConstexprDestructor(); Sema::SpecialMemberOverloadResult SMOR = lookupCallFromSpecialMember(S, ClassDecl, CSM, Quals, ConstRHS); @@ -6583,6 +6627,8 @@ static bool defaultedSpecialMemberIsConstexpr( break; case Sema::CXXDestructor: + return ClassDecl->defaultedDestructorIsConstexpr(); + case Sema::CXXInvalid: return false; } @@ -6835,13 +6881,14 @@ void Sema::CheckExplicitlyDefaultedSpecialMember(CXXMethodDecl *MD) { // Do not apply this rule to members of class templates, since core issue 1358 // makes such functions always instantiate to constexpr functions. For // functions which cannot be constexpr (for non-constructors in C++11 and for - // destructors in C++1y), this is checked elsewhere. + // destructors in C++14 and C++17), this is checked elsewhere. // // FIXME: This should not apply if the member is deleted. bool Constexpr = defaultedSpecialMemberIsConstexpr(*this, RD, CSM, HasConstParam); - if ((getLangOpts().CPlusPlus14 ? !isa(MD) - : isa(MD)) && + if ((getLangOpts().CPlusPlus2a || + (getLangOpts().CPlusPlus14 ? !isa(MD) + : isa(MD))) && MD->isConstexpr() && !Constexpr && MD->getTemplatedKind() == FunctionDecl::TK_NonTemplate) { Diag(MD->getBeginLoc(), MD->isConsteval() @@ -11464,6 +11511,10 @@ CXXDestructorDecl *Sema::DeclareImplicitDestructor(CXXRecordDecl *ClassDecl) { if (DSM.isAlreadyBeingDeclared()) return nullptr; + bool Constexpr = defaultedSpecialMemberIsConstexpr(*this, ClassDecl, + CXXDestructor, + false); + // Create the actual destructor declaration. CanQualType ClassType = Context.getCanonicalType(Context.getTypeDeclType(ClassDecl)); @@ -11471,10 +11522,11 @@ CXXDestructorDecl *Sema::DeclareImplicitDestructor(CXXRecordDecl *ClassDecl) { DeclarationName Name = Context.DeclarationNames.getCXXDestructorName(ClassType); DeclarationNameInfo NameInfo(Name, ClassLoc); - CXXDestructorDecl *Destructor - = CXXDestructorDecl::Create(Context, ClassDecl, ClassLoc, NameInfo, - QualType(), nullptr, /*isInline=*/true, - /*isImplicitlyDeclared=*/true); + CXXDestructorDecl *Destructor = + CXXDestructorDecl::Create(Context, ClassDecl, ClassLoc, NameInfo, + QualType(), nullptr, /*isInline=*/true, + /*isImplicitlyDeclared=*/true, + Constexpr ? CSK_constexpr : CSK_unspecified); Destructor->setAccess(AS_public); Destructor->setDefaulted(); diff --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp index 5338242..684254b 100644 --- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp +++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp @@ -2147,10 +2147,9 @@ Decl *TemplateDeclInstantiator::VisitCXXMethodDecl( Constructor->getConstexprKind()); Method->setRangeEnd(Constructor->getEndLoc()); } else if (CXXDestructorDecl *Destructor = dyn_cast(D)) { - Method = CXXDestructorDecl::Create(SemaRef.Context, Record, - StartLoc, NameInfo, T, TInfo, - Destructor->isInlineSpecified(), - false); + Method = CXXDestructorDecl::Create( + SemaRef.Context, Record, StartLoc, NameInfo, T, TInfo, + Destructor->isInlineSpecified(), false, Destructor->getConstexprKind()); Method->setRangeEnd(Destructor->getEndLoc()); } else if (CXXConversionDecl *Conversion = dyn_cast(D)) { Method = CXXConversionDecl::Create( diff --git a/clang/lib/Sema/SemaType.cpp b/clang/lib/Sema/SemaType.cpp index c892c62..a9459d9 100644 --- a/clang/lib/Sema/SemaType.cpp +++ b/clang/lib/Sema/SemaType.cpp @@ -8208,20 +8208,28 @@ bool Sema::RequireLiteralType(SourceLocation Loc, QualType T, return true; } } - } else if (!RD->hasTrivialDestructor()) { - // All fields and bases are of literal types, so have trivial destructors. - // If this class's destructor is non-trivial it must be user-declared. + } else if (getLangOpts().CPlusPlus2a ? !RD->hasConstexprDestructor() + : !RD->hasTrivialDestructor()) { + // All fields and bases are of literal types, so have trivial or constexpr + // destructors. If this class's destructor is non-trivial / non-constexpr, + // it must be user-declared. CXXDestructorDecl *Dtor = RD->getDestructor(); assert(Dtor && "class has literal fields and bases but no dtor?"); if (!Dtor) return true; - Diag(Dtor->getLocation(), Dtor->isUserProvided() ? - diag::note_non_literal_user_provided_dtor : - diag::note_non_literal_nontrivial_dtor) << RD; - if (!Dtor->isUserProvided()) - SpecialMemberIsTrivial(Dtor, CXXDestructor, TAH_IgnoreTrivialABI, - /*Diagnose*/true); + if (getLangOpts().CPlusPlus2a) { + Diag(Dtor->getLocation(), diag::note_non_literal_non_constexpr_dtor) + << RD; + } else { + Diag(Dtor->getLocation(), Dtor->isUserProvided() + ? diag::note_non_literal_user_provided_dtor + : diag::note_non_literal_nontrivial_dtor) + << RD; + if (!Dtor->isUserProvided()) + SpecialMemberIsTrivial(Dtor, CXXDestructor, TAH_IgnoreTrivialABI, + /*Diagnose*/ true); + } } return true; diff --git a/clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p1.cpp b/clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p1.cpp index f472e92..59e3dcf 100644 --- a/clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p1.cpp +++ b/clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p1.cpp @@ -1,6 +1,7 @@ // RUN: %clang_cc1 -triple %itanium_abi_triple -fsyntax-only -verify -std=c++11 %s // RUN: %clang_cc1 -triple %itanium_abi_triple -fsyntax-only -verify -std=c++14 %s -// RUN: %clang_cc1 -triple %itanium_abi_triple -fsyntax-only -verify -std=c++1z %s +// RUN: %clang_cc1 -triple %itanium_abi_triple -fsyntax-only -verify -std=c++17 %s +// RUN: %clang_cc1 -triple %itanium_abi_triple -fsyntax-only -verify -std=c++2a %s // MSVC always adopted the C++17 rule that implies that constexpr variables are // implicitly inline, so do the test again. @@ -75,7 +76,10 @@ template T f6(T); // expected-note {{here}} template constexpr T f6(T); // expected-error {{constexpr declaration of 'f6' follows non-constexpr declaration}} // destructor struct ConstexprDtor { - constexpr ~ConstexprDtor() = default; // expected-error {{destructor cannot be marked constexpr}} + constexpr ~ConstexprDtor() = default; +#if __cplusplus <= 201703L + // expected-error@-2 {{destructor cannot be declared constexpr}} +#endif }; // template stuff diff --git a/clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3.cpp b/clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3.cpp index 68196bc..59c2ee7 100644 --- a/clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3.cpp +++ b/clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3.cpp @@ -57,7 +57,10 @@ struct T : SS, NonLiteral { #ifndef CXX1Y // expected-error@-2 {{constexpr function's return type 'void' is not a literal type}} #endif - constexpr ~T(); // expected-error {{destructor cannot be marked constexpr}} + constexpr ~T(); +#ifndef CXX2A + // expected-error@-2 {{destructor cannot be declared constexpr}} +#endif typedef NonLiteral F() const; constexpr F NonLiteralReturn2; // ok until definition diff --git a/clang/test/CXX/dcl.decl/dcl.fct.def/dcl.fct.def.default/p2.cpp b/clang/test/CXX/dcl.decl/dcl.fct.def/dcl.fct.def.default/p2.cpp index 2b7886d..7274ee9 100644 --- a/clang/test/CXX/dcl.decl/dcl.fct.def/dcl.fct.def.default/p2.cpp +++ b/clang/test/CXX/dcl.decl/dcl.fct.def/dcl.fct.def.default/p2.cpp @@ -8,7 +8,7 @@ struct S1 { constexpr S1(S1&&) = default; constexpr S1 &operator=(const S1&) const = default; // expected-error {{explicitly-defaulted copy assignment operator may not have}} constexpr S1 &operator=(S1&&) const = default; // expected-error {{explicitly-defaulted move assignment operator may not have}} - constexpr ~S1() = default; // expected-error {{destructor cannot be marked constexpr}} + constexpr ~S1() = default; // expected-error {{destructor cannot be declared constexpr}} int n; }; struct NoCopyMove { diff --git a/clang/test/CXX/drs/dr2xx.cpp b/clang/test/CXX/drs/dr2xx.cpp index c7ffa77..89e1336 100644 --- a/clang/test/CXX/drs/dr2xx.cpp +++ b/clang/test/CXX/drs/dr2xx.cpp @@ -751,9 +751,12 @@ namespace dr263 { // dr263: yes #if __cplusplus < 201103L friend X::X() throw(); friend X::~X() throw(); -#else +#elif __cplusplus <= 201703L friend constexpr X::X() noexcept; friend X::~X(); +#else + friend constexpr X::X() noexcept; + friend constexpr X::~X(); #endif Y::Y(); // expected-error {{extra qualification}} Y::~Y(); // expected-error {{extra qualification}} diff --git a/clang/test/SemaCXX/attr-require-constant-initialization.cpp b/clang/test/SemaCXX/attr-require-constant-initialization.cpp index 12bae81..5584b4b 100644 --- a/clang/test/SemaCXX/attr-require-constant-initialization.cpp +++ b/clang/test/SemaCXX/attr-require-constant-initialization.cpp @@ -185,7 +185,7 @@ struct TT2 { ATTR static constexpr LitType lit = {}; ATTR static const NonLit non_lit; ATTR static const NonLit non_lit_list_init; - ATTR static const NonLit non_lit_copy_init; // expected-note {{required by 'require_constant_initialization' attribute here}} + ATTR static const NonLit non_lit_copy_init; #endif }; PODType TT2::pod_noinit; // expected-note 0+ {{declared here}} @@ -203,8 +203,9 @@ PODType TT2::pod_copy_init(TT2::pod_noinit); // expected-error {{variable does n #if __cplusplus >= 201402L const NonLit TT2::non_lit(42); const NonLit TT2::non_lit_list_init = {42}; -const NonLit TT2::non_lit_copy_init = 42; // expected-error {{variable does not have a constant initializer}} -// expected-note@-1 {{subexpression not valid in a constant expression}} +// FIXME: This is invalid, but we incorrectly elide the copy. It's OK if we +// start diagnosing this. +const NonLit TT2::non_lit_copy_init = 42; #endif #if __cplusplus >= 201103L diff --git a/clang/test/SemaCXX/constant-expression-cxx2a.cpp b/clang/test/SemaCXX/constant-expression-cxx2a.cpp index a1ab6e8..ae627b0 100644 --- a/clang/test/SemaCXX/constant-expression-cxx2a.cpp +++ b/clang/test/SemaCXX/constant-expression-cxx2a.cpp @@ -6,6 +6,10 @@ namespace std { struct type_info; }; +// Helper to print out values for debugging. +constexpr void not_defined(); +template constexpr void print(T) { not_defined(); } + namespace ThreeWayComparison { struct A { int n; @@ -219,15 +223,19 @@ static_assert(for_range_init()); namespace Virtual { struct NonZeroOffset { int padding = 123; }; + constexpr void assert(bool b) { if (!b) throw 0; } + // Ensure that we pick the right final overrider during construction. struct A { virtual constexpr char f() const { return 'A'; } char a = f(); + constexpr ~A() { assert(f() == 'A'); } }; struct NoOverrideA : A {}; struct B : NonZeroOffset, NoOverrideA { virtual constexpr char f() const { return 'B'; } char b = f(); + constexpr ~B() { assert(f() == 'B'); } }; struct NoOverrideB : B {}; struct C : NonZeroOffset, A { @@ -236,12 +244,15 @@ namespace Virtual { char c = ((A*)this)->f(); char ba = pba->f(); constexpr C(A *pba) : pba(pba) {} + constexpr ~C() { assert(f() == 'C'); } }; struct D : NonZeroOffset, NoOverrideB, C { // expected-warning {{inaccessible}} virtual constexpr char f() const { return 'D'; } char d = f(); constexpr D() : C((B*)this) {} + constexpr ~D() { assert(f() == 'D'); } }; + constexpr int n = (D(), 0); constexpr D d; static_assert(((B&)d).a == 'A'); static_assert(((C&)d).a == 'A'); @@ -634,3 +645,111 @@ namespace Uninit { } static_assert(switch_into_init_stmt()); } + +namespace dtor { + void lifetime_extension() { + struct X { constexpr ~X() {} }; + X &&a = X(); + } + + template constexpr T &&ref(T &&t) { return (T&&)t; } + + struct Buf { + char buf[64]; + int n = 0; + constexpr void operator+=(char c) { buf[n++] = c; } + constexpr bool operator==(const char *str) const { + return str[n] == 0 && __builtin_memcmp(str, buf, n) == 0; + } + constexpr bool operator!=(const char *str) const { return !operator==(str); } + }; + + struct A { + constexpr A(Buf &buf, char c) : buf(buf), c(c) { buf += c; } + constexpr ~A() { buf += c; } + constexpr operator bool() const { return true; } + Buf &buf; + char c; + }; + + constexpr bool dtor_calls_dtor() { + union U { + constexpr U(Buf &buf) : u(buf, 'u') { buf += 'U'; } + constexpr ~U() { u.buf += 'U'; } + A u, v; + }; + + struct B : A { + A c, &&d, e; + union { + A f; + }; + U u; + constexpr B(Buf &buf) + : A(buf, 'a'), c(buf, 'c'), d(ref(A(buf, 'd'))), e(A(buf, 'e')), f(buf, 'f'), u(buf) { + buf += 'b'; + } + constexpr ~B() { + buf += 'b'; + } + }; + + Buf buf; + { + B b(buf); + if (buf != "acddefuUb") + return false; + } + if (buf != "acddefuUbbUeca") + return false; + return true; + } + static_assert(dtor_calls_dtor()); + + constexpr void abnormal_termination(Buf &buf) { + struct Indestructible { + constexpr ~Indestructible(); // not defined + }; + + A a(buf, 'a'); + A(buf, 'b'); + int n = 0; + for (A &&c = A(buf, 'c'); A d = A(buf, 'd'); A(buf, 'e')) { + switch (A f(buf, 'f'); A g = A(buf, 'g')) { // expected-warning {{boolean}} + case false: { + A x(buf, 'x'); + } + + case true: { + A h(buf, 'h'); + switch (n++) { + case 0: + break; + case 1: + continue; + case 2: + return; + } + break; + } + + default: + Indestructible indest; + } + + A j = (A(buf, 'i'), A(buf, 'j')); + } + } + + constexpr bool check_abnormal_termination() { + Buf buf = {}; + abnormal_termination(buf); + return buf == + "abbc" + "dfgh" /*break*/ "hgfijijeed" + "dfgh" /*continue*/ "hgfeed" + "dfgh" /*return*/ "hgfd" + "ca"; + } + static_assert(check_abnormal_termination()); +} diff --git a/clang/test/SemaCXX/cxx2a-consteval.cpp b/clang/test/SemaCXX/cxx2a-consteval.cpp index c531b92..3b52ed4 100644 --- a/clang/test/SemaCXX/cxx2a-consteval.cpp +++ b/clang/test/SemaCXX/cxx2a-consteval.cpp @@ -27,7 +27,7 @@ struct A { } consteval A(int i); consteval A() = default; - consteval ~A() = default; // expected-error {{destructor cannot be marked consteval}} + consteval ~A() = default; }; consteval struct B {}; // expected-error {{struct cannot be marked consteval}} @@ -45,11 +45,17 @@ consteval int f1() {} // expected-error {{no return statement in consteval funct struct C { C() {} + ~C() {} }; struct D { C c; consteval D() = default; // expected-error {{cannot be consteval}} + consteval ~D() = default; // expected-error {{cannot be consteval}} +}; + +struct E : C { // expected-note {{here}} + consteval ~E() {} // expected-error {{cannot be declared consteval because base class 'basic_sema::C' does not have a constexpr destructor}} }; } -- 2.7.4