Index: include/clang/Analysis/ConstructionContext.h =================================================================== --- include/clang/Analysis/ConstructionContext.h +++ include/clang/Analysis/ConstructionContext.h @@ -22,74 +22,193 @@ namespace clang { -/// Construction context is a linked list of multiple layers. Layers are -/// created gradually while traversing the AST, and layers that represent -/// the outmost AST nodes are built first, while the node that immediately -/// contains the constructor would be built last and capture the previous -/// layers as its parents. Construction context captures the last layer -/// (which has links to the previous layers) and classifies the seemingly -/// arbitrary chain of layers into one of the possible ways of constructing -/// an object in C++ for user-friendly experience. -class ConstructionContextLayer { +/// Represents a single point (AST node) in the program that requires attention +/// during construction of an object. ConstructionContext would be represented +/// as a list of such items. +class ConstructionContextItem { public: - typedef llvm::PointerUnion TriggerTy; + enum ItemKind { + VariableKind, + NewAllocatorKind, + ReturnKind, + MaterializationKind, + TemporaryDestructorKind, + ElidedDestructorKind, + ElidableConstructorKind, + ArgumentKind, + STATEMENT_WITH_INDEX_KIND_BEGIN=ArgumentKind, + STATEMENT_WITH_INDEX_KIND_END=ArgumentKind, + STATEMENT_KIND_BEGIN = VariableKind, + STATEMENT_KIND_END = ArgumentKind, + InitializerKind, + INITIALIZER_KIND_BEGIN=InitializerKind, + INITIALIZER_KIND_END=InitializerKind + }; + + LLVM_DUMP_METHOD static StringRef getKindAsString(ItemKind K) { + switch (K) { + case VariableKind: return "construct into local variable"; + case NewAllocatorKind: return "construct into new-allocator"; + case ReturnKind: return "construct into return address"; + case MaterializationKind: return "materialize temporary"; + case TemporaryDestructorKind: return "destroy temporary"; + case ElidedDestructorKind: return "elide destructor"; + case ElidableConstructorKind: return "elide constructor"; + case ArgumentKind: return "construct into argument"; + case InitializerKind: return "construct into member variable"; + }; + } private: + const void *const Data; + const ItemKind Kind; + const unsigned Index = 0; + + bool hasStatement() const { + return Kind >= STATEMENT_KIND_BEGIN && + Kind <= STATEMENT_KIND_END; + } + + bool hasIndex() const { + return Kind >= STATEMENT_WITH_INDEX_KIND_BEGIN && + Kind >= STATEMENT_WITH_INDEX_KIND_END; + } + + bool hasInitializer() const { + return Kind >= INITIALIZER_KIND_BEGIN && + Kind <= INITIALIZER_KIND_END; + } + +public: + // ConstructionContextItem should be simple enough so that it was easy to + // re-construct it from the AST node it captures. For that reason we provide + // simple implicit conversions from all sorts of supported AST nodes. + ConstructionContextItem(const DeclStmt *DS) + : Data(DS), Kind(VariableKind) {} + + ConstructionContextItem(const CXXNewExpr *NE) + : Data(NE), Kind(NewAllocatorKind) {} + + ConstructionContextItem(const ReturnStmt *RS) + : Data(RS), Kind(ReturnKind) {} + + ConstructionContextItem(const MaterializeTemporaryExpr *MTE) + : Data(MTE), Kind(MaterializationKind) {} + + ConstructionContextItem(const CXXBindTemporaryExpr *BTE, + bool IsElided = false) + : Data(BTE), + Kind(IsElided ? ElidedDestructorKind : TemporaryDestructorKind) {} + + ConstructionContextItem(const CXXConstructExpr *CE) + : Data(CE), Kind(ElidableConstructorKind) {} + + ConstructionContextItem(const CallExpr *CE, unsigned Index) + : Data(CE), Kind(ArgumentKind), Index(Index) {} + + ConstructionContextItem(const CXXConstructExpr *CE, unsigned Index) + : Data(CE), Kind(ArgumentKind), Index(Index) {} + + ConstructionContextItem(const ObjCMessageExpr *ME, unsigned Index) + : Data(ME), Kind(ArgumentKind), Index(Index) {} + + ConstructionContextItem(const CXXCtorInitializer *Init) + : Data(Init), Kind(InitializerKind), Index(0) {} + + ItemKind getKind() const { return Kind; } + + LLVM_DUMP_METHOD StringRef getKindAsString() const { + return getKindAsString(getKind()); + } + /// The construction site - the statement that triggered the construction /// for one of its parts. For instance, stack variable declaration statement /// triggers construction of itself or its elements if it's an array, /// new-expression triggers construction of the newly allocated object(s). - TriggerTy Trigger; + const Stmt *getStmt() const { + assert(hasStatement()); + return static_cast(Data); + } + + const Stmt *getStmtOrNull() const { + return hasStatement() ? getStmt() : nullptr; + } + + /// The construction site is not necessarily a statement. It may also be a + /// CXXCtorInitializer, which means that a member variable is being + /// constructed during initialization of the object that contains it. + const CXXCtorInitializer *getCXXCtorInitializer() const { + assert(hasInitializer()); + return static_cast(Data); + } /// If a single trigger statement triggers multiple constructors, they are /// usually being enumerated. This covers function argument constructors /// triggered by a call-expression and items in an initializer list triggered /// by an init-list-expression. - unsigned Index; - - /// Sometimes a single trigger is not enough to describe the construction - /// site. In this case we'd have a chain of "partial" construction context - /// layers. - /// Some examples: - /// - A constructor within in an aggregate initializer list within a variable - /// would have a construction context of the initializer list with - /// the parent construction context of a variable. - /// - A constructor for a temporary that needs to be both destroyed - /// and materialized into an elidable copy constructor would have a - /// construction context of a CXXBindTemporaryExpr with the parent - /// construction context of a MaterializeTemproraryExpr. - /// Not all of these are currently supported. + unsigned getIndex() const { + // This is a fairly specific request. Let's make sure the user knows + // what he's doing. + assert(hasIndex()); + return Index; + } + + void Profile(llvm::FoldingSetNodeID &ID) const { + ID.AddPointer(Data); + ID.AddInteger(Kind); + ID.AddInteger(Index); + } + + bool operator==(const ConstructionContextItem &Other) const { + // For most kinds the Index comparison is trivially true, but + // checking kind separately doesn't seem to be less expensive + // than checking Index. Same in operator<(). + return std::make_tuple(Data, Kind, Index) == + std::make_tuple(Other.Data, Other.Kind, Other.Index); + } + + bool operator<(const ConstructionContextItem &Other) const { + return std::make_tuple(Data, Kind, Index) < + std::make_tuple(Other.Data, Other.Kind, Other.Index); + } +}; + +/// Construction context can be seen as a linked list of multiple layers. +/// Sometimes a single trigger is not enough to describe the construction +/// site. That's what causing us to have a chain of "partial" construction +/// context layers. Some examples: +/// - A constructor within in an aggregate initializer list within a variable +/// would have a construction context of the initializer list with +/// the parent construction context of a variable. +/// - A constructor for a temporary that needs to be both destroyed +/// and materialized into an elidable copy constructor would have a +/// construction context of a CXXBindTemporaryExpr with the parent +/// construction context of a MaterializeTemproraryExpr. +/// Not all of these are currently supported. +/// Layers are created gradually while traversing the AST, and layers that +/// represent the outmost AST nodes are built first, while the node that +/// immediately contains the constructor would be built last and capture the +/// previous layers as its parents. Construction context captures the last layer +/// (which has links to the previous layers) and classifies the seemingly +/// arbitrary chain of layers into one of the possible ways of constructing +/// an object in C++ for user-friendly experience. +class ConstructionContextLayer { const ConstructionContextLayer *Parent = nullptr; + ConstructionContextItem Item; - ConstructionContextLayer(TriggerTy Trigger, unsigned Index, + ConstructionContextLayer(ConstructionContextItem Item, const ConstructionContextLayer *Parent) - : Trigger(Trigger), Index(Index), Parent(Parent) {} + : Parent(Parent), Item(Item) {} public: static const ConstructionContextLayer * - create(BumpVectorContext &C, TriggerTy Trigger, unsigned Index = 0, + create(BumpVectorContext &C, const ConstructionContextItem &Item, const ConstructionContextLayer *Parent = nullptr); + const ConstructionContextItem &getItem() const { return Item; } const ConstructionContextLayer *getParent() const { return Parent; } bool isLast() const { return !Parent; } - const Stmt *getTriggerStmt() const { - return Trigger.dyn_cast(); - } - - const CXXCtorInitializer *getTriggerInit() const { - return Trigger.dyn_cast(); - } - - unsigned getIndex() const { return Index; } - - /// Returns true if these layers are equal as individual layers, even if - /// their parents are different. - bool isSameLayer(const ConstructionContextLayer *Other) const { - assert(Other); - return (Trigger == Other->Trigger && Index == Other->Index); - } - /// See if Other is a proper initial segment of this construction context /// in terms of the parent chain - i.e. a few first parents coincide and /// then the other context terminates but our context goes further - i.e., @@ -141,6 +260,23 @@ return new (CC) T(Args...); } + // A sub-routine of createFromLayers() that deals with temporary objects + // that need to be materialized. The BTE argument is for the situation when + // the object also needs to be bound for destruction. + static const ConstructionContext *createMaterializedTemporaryFromLayers( + BumpVectorContext &C, const MaterializeTemporaryExpr *MTE, + const CXXBindTemporaryExpr *BTE, + const ConstructionContextLayer *ParentLayer); + + // A sub-routine of createFromLayers() that deals with temporary objects + // that need to be bound for destruction. Automatically finds out if the + // object also needs to be materialized and delegates to + // createMaterializedTemporaryFromLayers() if necessary. + static const ConstructionContext * + createBoundTemporaryFromLayers( + BumpVectorContext &C, const CXXBindTemporaryExpr *BTE, + const ConstructionContextLayer *ParentLayer); + public: /// Consume the construction context layer, together with its parent layers, /// and wrap it up into a complete construction context. May return null Index: include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h =================================================================== --- include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h +++ include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h @@ -761,24 +761,24 @@ /// This allows, among other things, to keep bindings to variable's fields /// made within the constructor alive until its declaration actually /// goes into scope. - static ProgramStateRef addObjectUnderConstruction( - ProgramStateRef State, - llvm::PointerUnion P, - const LocationContext *LC, SVal V); + static ProgramStateRef + addObjectUnderConstruction(ProgramStateRef State, + const ConstructionContextItem &Item, + const LocationContext *LC, SVal V); /// Mark the object sa fully constructed, cleaning up the state trait /// that tracks objects under construction. - static ProgramStateRef finishObjectConstruction( - ProgramStateRef State, - llvm::PointerUnion P, - const LocationContext *LC); + static ProgramStateRef + finishObjectConstruction(ProgramStateRef State, + const ConstructionContextItem &Item, + const LocationContext *LC); /// If the given statement corresponds to an object under construction, /// being part of its construciton context, retrieve that object's location. - static Optional getObjectUnderConstruction( - ProgramStateRef State, - llvm::PointerUnion P, - const LocationContext *LC); + static Optional + getObjectUnderConstruction(ProgramStateRef State, + const ConstructionContextItem &Item, + const LocationContext *LC); /// If the given expression corresponds to a temporary that was used for /// passing into an elidable copy/move constructor and that constructor Index: lib/Analysis/CFG.cpp =================================================================== --- lib/Analysis/CFG.cpp +++ lib/Analysis/CFG.cpp @@ -697,7 +697,8 @@ Expr *Arg = E->getArg(i); if (Arg->getType()->getAsCXXRecordDecl() && !Arg->isGLValue()) findConstructionContexts( - ConstructionContextLayer::create(cfg->getBumpVectorContext(), E, i), + ConstructionContextLayer::create(cfg->getBumpVectorContext(), + ConstructionContextItem(E, i)), Arg); } } @@ -1286,9 +1287,9 @@ if (!Child) return; - auto withExtraLayer = [this, Layer](Stmt *S, unsigned Index = 0) { - return ConstructionContextLayer::create(cfg->getBumpVectorContext(), S, - Index, Layer); + auto withExtraLayer = [this, Layer](const ConstructionContextItem &Item) { + return ConstructionContextLayer::create(cfg->getBumpVectorContext(), Item, + Layer); }; switch(Child->getStmtClass()) { @@ -1348,18 +1349,17 @@ // it indicates the beginning of a temporary object construction context, // so it shouldn't be found in the middle. However, if it is the beginning // of an elidable copy or move construction context, we need to include it. - if (const auto *CE = - dyn_cast_or_null(Layer->getTriggerStmt())) { - if (CE->isElidable()) { - auto *MTE = cast(Child); - findConstructionContexts(withExtraLayer(MTE), MTE->GetTemporaryExpr()); - } + if (Layer->getItem().getKind() == + ConstructionContextItem::ElidableConstructorKind) { + auto *MTE = cast(Child); + findConstructionContexts(withExtraLayer(MTE), MTE->GetTemporaryExpr()); } break; } case Stmt::ConditionalOperatorClass: { auto *CO = cast(Child); - if (!dyn_cast_or_null(Layer->getTriggerStmt())) { + if (Layer->getItem().getKind() != + ConstructionContextItem::MaterializationKind) { // If the object returned by the conditional operator is not going to be a // temporary object that needs to be immediately materialized, then // it must be C++17 with its mandatory copy elision. Do not yet promise @@ -3221,8 +3221,7 @@ const DeclStmt *DS = F->getConditionVariableDeclStmt(); assert(DS->isSingleDecl()); findConstructionContexts( - ConstructionContextLayer::create(cfg->getBumpVectorContext(), - const_cast(DS)), + ConstructionContextLayer::create(cfg->getBumpVectorContext(), DS), Init); appendStmt(Block, DS); EntryConditionBlock = addStmt(Init); Index: lib/Analysis/ConstructionContext.cpp =================================================================== --- lib/Analysis/ConstructionContext.cpp +++ lib/Analysis/ConstructionContext.cpp @@ -20,12 +20,12 @@ using namespace clang; const ConstructionContextLayer * -ConstructionContextLayer::create(BumpVectorContext &C, TriggerTy Trigger, - unsigned Index, +ConstructionContextLayer::create(BumpVectorContext &C, + const ConstructionContextItem &Item, const ConstructionContextLayer *Parent) { ConstructionContextLayer *CC = C.getAllocator().Allocate(); - return new (CC) ConstructionContextLayer(Trigger, Index, Parent); + return new (CC) ConstructionContextLayer(Item, Parent); } bool ConstructionContextLayer::isStrictlyMoreSpecificThan( @@ -34,7 +34,7 @@ while (true) { if (!Other) return Self; - if (!Self || !Self->isSameLayer(Other)) + if (!Self || !(Self->Item == Other->Item)) return false; Self = Self->getParent(); Other = Other->getParent(); @@ -42,156 +42,176 @@ llvm_unreachable("The above loop can only be terminated via return!"); } +const ConstructionContext * +ConstructionContext::createMaterializedTemporaryFromLayers( + BumpVectorContext &C, const MaterializeTemporaryExpr *MTE, + const CXXBindTemporaryExpr *BTE, + const ConstructionContextLayer *ParentLayer) { + assert(MTE); + + // If the object requires destruction and is not lifetime-extended, + // then it must have a BTE within its MTE, otherwise it shouldn't. + // FIXME: This should be an assertion. + if (!BTE && !(MTE->getType().getCanonicalType()->getAsCXXRecordDecl() + ->hasTrivialDestructor() || + MTE->getStorageDuration() != SD_FullExpression)) { + return nullptr; + } + + // If the temporary is lifetime-extended, don't save the BTE, + // because we don't need a temporary destructor, but an automatic + // destructor. + if (MTE->getStorageDuration() != SD_FullExpression) { + BTE = nullptr; + } + + // Handle pre-C++17 copy and move elision. + const CXXConstructExpr *ElidedCE = nullptr; + const ConstructionContext *ElidedCC = nullptr; + if (ParentLayer) { + const ConstructionContextItem &ElidedItem = ParentLayer->getItem(); + assert(ElidedItem.getKind() == + ConstructionContextItem::ElidableConstructorKind); + ElidedCE = cast(ElidedItem.getStmt()); + assert(ElidedCE->isElidable()); + // We're creating a construction context that might have already + // been created elsewhere. Maybe we should unique our construction + // contexts. That's what we often do, but in this case it's unlikely + // to bring any benefits. + ElidedCC = createFromLayers(C, ParentLayer->getParent()); + if (!ElidedCC) { + // We may fail to create the elided construction context. + // In this case, skip copy elision entirely. + return create(C, BTE, MTE); + } + return create( + C, BTE, MTE, ElidedCE, ElidedCC); + } + + // This is a normal temporary. + assert(!ParentLayer); + return create(C, BTE, MTE); +} + +const ConstructionContext *ConstructionContext::createBoundTemporaryFromLayers( + BumpVectorContext &C, const CXXBindTemporaryExpr *BTE, + const ConstructionContextLayer *ParentLayer) { + if (!ParentLayer) { + // A temporary object that doesn't require materialization. + // In particular, it shouldn't require copy elision, because + // copy/move constructors take a reference, which requires + // materialization to obtain the glvalue. + return create(C, BTE, + /*MTE=*/nullptr); + } + + const ConstructionContextItem &ParentItem = ParentLayer->getItem(); + switch (ParentItem.getKind()) { + case ConstructionContextItem::VariableKind: { + const auto *DS = cast(ParentItem.getStmt()); + assert(!cast(DS->getSingleDecl())->getType().getCanonicalType() + ->getAsCXXRecordDecl()->hasTrivialDestructor()); + return create(C, DS, BTE); + } + case ConstructionContextItem::NewAllocatorKind: { + llvm_unreachable("This context does not accept a bound temporary!"); + } + case ConstructionContextItem::ReturnKind: { + assert(ParentLayer->isLast()); + const auto *RS = cast(ParentItem.getStmt()); + assert(!RS->getRetValue()->getType().getCanonicalType() + ->getAsCXXRecordDecl()->hasTrivialDestructor()); + return create(C, RS, + BTE); + } + + case ConstructionContextItem::MaterializationKind: { + // No assert. We may have an elidable copy on the grandparent layer. + const auto *MTE = cast(ParentItem.getStmt()); + return createMaterializedTemporaryFromLayers(C, MTE, BTE, + ParentLayer->getParent()); + } + case ConstructionContextItem::TemporaryDestructorKind: { + llvm_unreachable("Duplicate CXXBindTemporaryExpr in the AST!"); + } + case ConstructionContextItem::ElidedDestructorKind: { + llvm_unreachable("Elided destructor items are not produced by the CFG!"); + } + case ConstructionContextItem::ElidableConstructorKind: { + llvm_unreachable("Materialization is necessary to put temporary into a " + "copy or move constructor!"); + } + case ConstructionContextItem::ArgumentKind: { + assert(ParentLayer->isLast()); + const auto *E = cast(ParentItem.getStmt()); + assert(isa(E) || isa(E) || + isa(E)); + return create(C, E, ParentItem.getIndex(), + BTE); + } + case ConstructionContextItem::InitializerKind: { + assert(ParentLayer->isLast()); + const auto *I = ParentItem.getCXXCtorInitializer(); + assert(!I->getAnyMember()->getType().getCanonicalType() + ->getAsCXXRecordDecl()->hasTrivialDestructor()); + return create( + C, I, BTE); + } + } // switch (ParentItem.getKind()) + + llvm_unreachable("Unexpected construction context with destructor!"); +} + const ConstructionContext *ConstructionContext::createFromLayers( BumpVectorContext &C, const ConstructionContextLayer *TopLayer) { // Before this point all we've had was a stockpile of arbitrary layers. // Now validate that it is shaped as one of the finite amount of expected // patterns. - if (const Stmt *S = TopLayer->getTriggerStmt()) { - if (const auto *DS = dyn_cast(S)) { - assert(TopLayer->isLast()); - return create(C, DS); - } - if (const auto *NE = dyn_cast(S)) { - assert(TopLayer->isLast()); - return create(C, NE); - } - if (const auto *BTE = dyn_cast(S)) { - const MaterializeTemporaryExpr *MTE = nullptr; - assert(BTE->getType().getCanonicalType() - ->getAsCXXRecordDecl()->hasNonTrivialDestructor()); - // For temporaries with destructors, there may or may not be - // lifetime extension on the parent layer. - if (const ConstructionContextLayer *ParentLayer = TopLayer->getParent()) { - // C++17 *requires* elision of the constructor at the return site - // and at variable/member initialization site, while previous standards - // were allowing an optional elidable constructor. - // This is the C++17 copy-elided construction into a ctor initializer. - if (const CXXCtorInitializer *I = ParentLayer->getTriggerInit()) { - return create< - CXX17ElidedCopyConstructorInitializerConstructionContext>(C, - I, BTE); - } - assert(ParentLayer->getTriggerStmt() && - "Non-statement-based layers have been handled above!"); - // This is the normal, non-C++17 case: a temporary object which has - // both destruction and materialization info attached to it in the AST. - if ((MTE = dyn_cast( - ParentLayer->getTriggerStmt()))) { - if (MTE->getStorageDuration() != SD_FullExpression) { - // If the temporary is lifetime-extended, don't save the BTE, - // because we don't need a temporary destructor, but an automatic - // destructor. - BTE = nullptr; - } - - // Handle pre-C++17 copy and move elision. - const CXXConstructExpr *ElidedCE = nullptr; - const ConstructionContext *ElidedCC = nullptr; - if (const ConstructionContextLayer *ElidedLayer = - ParentLayer->getParent()) { - ElidedCE = cast(ElidedLayer->getTriggerStmt()); - assert(ElidedCE->isElidable()); - // We're creating a construction context that might have already - // been created elsewhere. Maybe we should unique our construction - // contexts. That's what we often do, but in this case it's unlikely - // to bring any benefits. - ElidedCC = createFromLayers(C, ElidedLayer->getParent()); - if (!ElidedCC) { - // We may fail to create the elided construction context. - // In this case, skip copy elision entirely. - return create(C, BTE, - MTE); - } else { - return create( - C, BTE, MTE, ElidedCE, ElidedCC); - } - } - assert(ParentLayer->isLast()); - return create(C, BTE, MTE); - } - assert(ParentLayer->isLast()); - - // This is a constructor into a function argument. - if (isa(ParentLayer->getTriggerStmt()) || - isa(ParentLayer->getTriggerStmt()) || - isa(ParentLayer->getTriggerStmt())) { - return create( - C, cast(ParentLayer->getTriggerStmt()), - ParentLayer->getIndex(), BTE); - } - // This is C++17 copy-elided construction into return statement. - if (auto *RS = dyn_cast(ParentLayer->getTriggerStmt())) { - assert(!RS->getRetValue()->getType().getCanonicalType() - ->getAsCXXRecordDecl()->hasTrivialDestructor()); - return create(C, - RS, BTE); - } - // This is C++17 copy-elided construction into a simple variable. - if (auto *DS = dyn_cast(ParentLayer->getTriggerStmt())) { - assert(!cast(DS->getSingleDecl())->getType() - .getCanonicalType()->getAsCXXRecordDecl() - ->hasTrivialDestructor()); - return create(C, DS, BTE); - } - llvm_unreachable("Unexpected construction context with destructor!"); - } - // A temporary object that doesn't require materialization. - // In particular, it shouldn't require copy elision, because - // copy/move constructors take a reference, which requires - // materialization to obtain the glvalue. - return create(C, BTE, - /*MTE=*/nullptr); - } - if (const auto *MTE = dyn_cast(S)) { - // If the object requires destruction and is not lifetime-extended, - // then it must have a BTE within its MTE. - // FIXME: This should be an assertion. - if (!(MTE->getType().getCanonicalType() - ->getAsCXXRecordDecl()->hasTrivialDestructor() || - MTE->getStorageDuration() != SD_FullExpression)) - return nullptr; - - // Handle pre-C++17 copy and move elision. - const CXXConstructExpr *ElidedCE = nullptr; - const ConstructionContext *ElidedCC = nullptr; - if (const ConstructionContextLayer *ElidedLayer = TopLayer->getParent()) { - ElidedCE = cast(ElidedLayer->getTriggerStmt()); - assert(ElidedCE->isElidable()); - // We're creating a construction context that might have already - // been created elsewhere. Maybe we should unique our construction - // contexts. That's what we often do, but in this case it's unlikely - // to bring any benefits. - ElidedCC = createFromLayers(C, ElidedLayer->getParent()); - if (!ElidedCC) { - // We may fail to create the elided construction context. - // In this case, skip copy elision entirely. - return create(C, nullptr, - MTE); - } - return create( - C, nullptr, MTE, ElidedCE, ElidedCC); - } - assert(TopLayer->isLast()); - return create(C, nullptr, MTE); - } - if (const auto *RS = dyn_cast(S)) { - assert(TopLayer->isLast()); - return create(C, RS); - } - // This is a constructor into a function argument. - if (isa(TopLayer->getTriggerStmt()) || - isa(TopLayer->getTriggerStmt()) || - isa(TopLayer->getTriggerStmt())) { - assert(TopLayer->isLast()); - return create( - C, cast(TopLayer->getTriggerStmt()), TopLayer->getIndex(), - /*BTE=*/nullptr); - } - llvm_unreachable("Unexpected construction context with statement!"); - } else if (const CXXCtorInitializer *I = TopLayer->getTriggerInit()) { + const ConstructionContextItem &TopItem = TopLayer->getItem(); + switch (TopItem.getKind()) { + case ConstructionContextItem::VariableKind: { assert(TopLayer->isLast()); + const auto *DS = cast(TopItem.getStmt()); + return create(C, DS); + } + case ConstructionContextItem::NewAllocatorKind: { + assert(TopLayer->isLast()); + const auto *NE = cast(TopItem.getStmt()); + return create(C, NE); + } + case ConstructionContextItem::ReturnKind: { + assert(TopLayer->isLast()); + const auto *RS = cast(TopItem.getStmt()); + return create(C, RS); + } + case ConstructionContextItem::MaterializationKind: { + const auto *MTE = cast(TopItem.getStmt()); + return createMaterializedTemporaryFromLayers(C, MTE, /*BTE=*/nullptr, + TopLayer->getParent()); + } + case ConstructionContextItem::TemporaryDestructorKind: { + const auto *BTE = cast(TopItem.getStmt()); + assert(BTE->getType().getCanonicalType()->getAsCXXRecordDecl() + ->hasNonTrivialDestructor()); + return createBoundTemporaryFromLayers(C, BTE, TopLayer->getParent()); + } + case ConstructionContextItem::ElidedDestructorKind: { + llvm_unreachable("Elided destructor items are not produced by the CFG!"); + } + case ConstructionContextItem::ElidableConstructorKind: { + llvm_unreachable("The argument needs to be materialized first!"); + } + case ConstructionContextItem::InitializerKind: { + assert(TopLayer->isLast()); + const CXXCtorInitializer *I = TopItem.getCXXCtorInitializer(); return create(C, I); } + case ConstructionContextItem::ArgumentKind: { + assert(TopLayer->isLast()); + const auto *E = cast(TopItem.getStmt()); + return create(C, E, TopItem.getIndex(), + /*BTE=*/nullptr); + } + } // switch (TopItem.getKind()) llvm_unreachable("Unexpected construction context!"); } Index: lib/StaticAnalyzer/Core/ExprEngine.cpp =================================================================== --- lib/StaticAnalyzer/Core/ExprEngine.cpp +++ lib/StaticAnalyzer/Core/ExprEngine.cpp @@ -117,56 +117,42 @@ /// the construction context was present and contained references to these /// AST nodes. class ConstructedObjectKey { - typedef std::pair< - llvm::PointerUnion, - const LocationContext *> ConstructedObjectKeyImpl; + typedef std::pair + ConstructedObjectKeyImpl; - ConstructedObjectKeyImpl Impl; + const ConstructedObjectKeyImpl Impl; const void *getAnyASTNodePtr() const { - if (const Stmt *S = getStmt()) + if (const Stmt *S = getItem().getStmt()) return S; else - return getCXXCtorInitializer(); + return getItem().getCXXCtorInitializer(); } public: - ConstructedObjectKey( - llvm::PointerUnion P, - const LocationContext *LC) - : Impl(P, LC) { - // This is the full list of statements that require additional actions when - // encountered. This list may be expanded when new actions are implemented. - assert(getCXXCtorInitializer() || isa(getStmt()) || - isa(getStmt()) || isa(getStmt()) || - isa(getStmt()) || - isa(getStmt())); - } - - const Stmt *getStmt() const { - return Impl.first.dyn_cast(); - } - - const CXXCtorInitializer *getCXXCtorInitializer() const { - return Impl.first.dyn_cast(); - } + explicit ConstructedObjectKey(const ConstructionContextItem &Item, + const LocationContext *LC) + : Impl(Item, LC) {} - const LocationContext *getLocationContext() const { - return Impl.second; - } + const ConstructionContextItem &getItem() const { return Impl.first; } + const LocationContext *getLocationContext() const { return Impl.second; } void print(llvm::raw_ostream &OS, PrinterHelper *Helper, PrintingPolicy &PP) { - OS << '(' << getLocationContext() << ',' << getAnyASTNodePtr() << ") "; - if (const Stmt *S = getStmt()) { + OS << '(' << getLocationContext() << ',' << getAnyASTNodePtr() << ',' + << getItem().getKindAsString(); + if (getItem().getKind() == ConstructionContextItem::ArgumentKind) + OS << " #" << getItem().getIndex(); + OS << ") "; + if (const Stmt *S = getItem().getStmtOrNull()) { S->printPretty(OS, Helper, PP); } else { - const CXXCtorInitializer *I = getCXXCtorInitializer(); + const CXXCtorInitializer *I = getItem().getCXXCtorInitializer(); OS << I->getAnyMember()->getNameAsString(); } } void Profile(llvm::FoldingSetNodeID &ID) const { - ID.AddPointer(Impl.first.getOpaqueValue()); + ID.Add(Impl.first); ID.AddPointer(Impl.second); } @@ -184,15 +170,6 @@ REGISTER_TRAIT_WITH_PROGRAMSTATE(ObjectsUnderConstruction, ObjectsUnderConstructionMap) -// Additionally, track a set of destructors that correspond to elided -// constructors when copy elision occurs. -typedef std::pair - ElidedDestructorItem; -typedef llvm::ImmutableSet - ElidedDestructorSet; -REGISTER_TRAIT_WITH_PROGRAMSTATE(ElidedDestructors, - ElidedDestructorSet) - //===----------------------------------------------------------------------===// // Engine construction and deletion. //===----------------------------------------------------------------------===// @@ -449,31 +426,32 @@ return State; } -ProgramStateRef ExprEngine::addObjectUnderConstruction( - ProgramStateRef State, - llvm::PointerUnion P, - const LocationContext *LC, SVal V) { - ConstructedObjectKey Key(P, LC->getStackFrame()); +ProgramStateRef +ExprEngine::addObjectUnderConstruction(ProgramStateRef State, + const ConstructionContextItem &Item, + const LocationContext *LC, SVal V) { + ConstructedObjectKey Key(Item, LC->getStackFrame()); // FIXME: Currently the state might already contain the marker due to // incorrect handling of temporaries bound to default parameters. assert(!State->get(Key) || - isa(Key.getStmt())); + Key.getItem().getKind() == + ConstructionContextItem::TemporaryDestructorKind); return State->set(Key, V); } -Optional ExprEngine::getObjectUnderConstruction( - ProgramStateRef State, - llvm::PointerUnion P, - const LocationContext *LC) { - ConstructedObjectKey Key(P, LC->getStackFrame()); +Optional +ExprEngine::getObjectUnderConstruction(ProgramStateRef State, + const ConstructionContextItem &Item, + const LocationContext *LC) { + ConstructedObjectKey Key(Item, LC->getStackFrame()); return Optional::create(State->get(Key)); } -ProgramStateRef ExprEngine::finishObjectConstruction( - ProgramStateRef State, - llvm::PointerUnion P, - const LocationContext *LC) { - ConstructedObjectKey Key(P, LC->getStackFrame()); +ProgramStateRef +ExprEngine::finishObjectConstruction(ProgramStateRef State, + const ConstructionContextItem &Item, + const LocationContext *LC) { + ConstructedObjectKey Key(Item, LC->getStackFrame()); assert(State->contains(Key)); return State->remove(Key); } @@ -481,25 +459,25 @@ ProgramStateRef ExprEngine::elideDestructor(ProgramStateRef State, const CXXBindTemporaryExpr *BTE, const LocationContext *LC) { - ElidedDestructorItem I(BTE, LC); - assert(!State->contains(I)); - return State->add(I); + ConstructedObjectKey Key({BTE, /*IsElided=*/true}, LC); + assert(!State->contains(Key)); + return State->set(Key, UnknownVal()); } ProgramStateRef ExprEngine::cleanupElidedDestructor(ProgramStateRef State, const CXXBindTemporaryExpr *BTE, const LocationContext *LC) { - ElidedDestructorItem I(BTE, LC); - assert(State->contains(I)); - return State->remove(I); + ConstructedObjectKey Key({BTE, /*IsElided=*/true}, LC); + assert(State->contains(Key)); + return State->remove(Key); } bool ExprEngine::isDestructorElided(ProgramStateRef State, const CXXBindTemporaryExpr *BTE, const LocationContext *LC) { - ElidedDestructorItem I(BTE, LC); - return State->contains(I); + ConstructedObjectKey Key({BTE, /*IsElided=*/true}, LC); + return State->contains(Key); } bool ExprEngine::areAllObjectsFullyConstructed(ProgramStateRef State, @@ -512,10 +490,6 @@ if (I.first.getLocationContext() == LC) return false; - for (auto I: State->get()) - if (I.second == LC) - return false; - LC = LC->getParent(); } return true; @@ -560,14 +534,6 @@ Key.print(Out, nullptr, PP); Out << " : " << Value << NL; } - - for (auto I : State->get()) { - if (I.second != LC) - continue; - Out << '(' << I.second << ',' << (const void *)I.first << ") "; - I.first->printPretty(Out, nullptr, PP); - Out << " : (constructor elided)" << NL; - } } void ExprEngine::printState(raw_ostream &Out, ProgramStateRef State, @@ -2252,25 +2218,14 @@ for (auto I : State->get()) if (I.first.getLocationContext() == LC) { // The comment above only pardons us for not cleaning up a - // CXXBindTemporaryExpr. If any other statements are found here, + // temporary destructor. If any other statements are found here, // it must be a separate problem. - assert(isa(I.first.getStmt())); + assert(I.first.getItem().getKind() == + ConstructionContextItem::TemporaryDestructorKind || + I.first.getItem().getKind() == + ConstructionContextItem::ElidedDestructorKind); State = State->remove(I.first); - // Also cleanup the elided destructor info. - ElidedDestructorItem Item( - cast(I.first.getStmt()), - I.first.getLocationContext()); - State = State->remove(Item); } - - // Also suppress the assertion for elided destructors when temporary - // destructors are not provided at all by the CFG, because there's no - // good place to clean them up. - if (!AMgr.getAnalyzerOptions().includeTemporaryDtorsInCFG()) - for (auto I : State->get()) - if (I.second == LC) - State = State->remove(I); - LC = LC->getParent(); } if (State != Pred->getState()) {