[CFG] [analyzer] NFC: Enumerate construction context layer kinds and re-use their code for ExprEngine keys.

[NoQ]

Summary

This is a refactoring patch; no functional change intended.

ConstructionContext is composed of ConstructionContextLayers, but the layer information is explicitly discarded when ConstructionContext is created, in favor of a more user-friendly and explicit interface for accessing the interesting items in the ConstructionContext. Later, however - in the Analyzer, which is currently the only user of this API - there appears ConstructedObjectKey which helps tracking path-sensitive information about the constructed object. Apparently, ConstructedObjectKey has a lot in common with ConstructionContextLayer: it captures a single item that requires attention during object construction.

The common part of these two structures is factored out into a new structure, ConstructionContextItem. With that, ConstructionContextLayer becomes a wrapper around ConstructionContextItem that additionally temporarily organizes the items as a linked list, and ConstructedObjectKey becomes a wrapper around ConstructionContextItemthat additionally carries path-sensitive stack frame information.

Once ConstructionContextItem is factored out, i also made it a bit more explicit when it comes to what sort of information does it carry. It now contains an enumeration of item kinds ("variable", "destructor", "materialization", "elidable copy"). Such clarification was necessary for supporting argument constructors because otherwise it's hard to discriminate between an elidable copy layer (identified by a construct-expression and, occasionally, index 0) and a layer for the first argument of a constructor (identified by a construct-expression and, intentionally, index 0). I believe it was a good idea in general to be more explicit about what the layer means, rather than give its clients an ad-hoc pile of raw data to work with. It also allows fancy switches in createFromLayers() that check that all cases were handled, and many assertions become more explicit.

I did not, however, want to introduce a full-scale class hierarchy of ConstructionContextItems because it'd result in a ridiculous amount of boilerplate. Having them easy to construct via implicit conversion, on the other hand, seems very user-friendly and i'm not seeing much error surface.

One of the questionable ideas here is to use a special item kind ElidedDestructor that is only used by the analyzer (to track destructors that correspond to elided constructors) and is never provided by the CFG (so unlike other items it has only one use). This allows us to stop using an extra program state set for elided destructors, but i'm not sure it was a beautiful thing to pursue.

[NoQ]

A better ConstructedObjectKey::print().

[NoQ]

Committed as rC338439 but Phabricator didn't pick it up because i put a . after the phabricator link.

include/clang/Analysis/ConstructionContext.h

 #ifndef LLVM_CLANG_ANALYSIS_CONSTRUCTIONCONTEXT_H
 #define LLVM_CLANG_ANALYSIS_CONSTRUCTIONCONTEXT_H
 
 #include "clang/Analysis/Support/BumpVector.h"
 #include "clang/AST/ExprCXX.h"
 
 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 {
-public:
-  typedef llvm::PointerUnion<Stmt *, CXXCtorInitializer *> TriggerTy;
+/// 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:
+  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<const Stmt *>(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<const CXXCtorInitializer *>(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;
+  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);
+  }
 
-  /// 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.
+  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.
+/// site. That's what causing us to have a chain of "partial" construction
+/// context layers. Some examples:
+/// 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<Stmt *>();
-  }
-
-  const CXXCtorInitializer *getTriggerInit() const {
-    return Trigger.dyn_cast<CXXCtorInitializer *>();
-  }
-
-  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.,
   /// we are providing the same context that the other context provides,
   /// and a bit more above that.
   bool isStrictlyMoreSpecificThan(const ConstructionContextLayer *Other) const;
 };
 

 private:
   // A helper function for constructing an instance into a bump vector context.
   template <typename T, typename... ArgTypes>
   static T *create(BumpVectorContext &C, ArgTypes... Args) {
     auto *CC = C.getAllocator().Allocate<T>();
     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
   /// if layers do not form any supported construction context.
   static const ConstructionContext *
   createFromLayers(BumpVectorContext &C,
                    const ConstructionContextLayer *TopLayer);
 

include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h

private:
 
   /// Store the location of a C++ object corresponding to a statement
   /// until the statement is actually encountered. For example, if a DeclStmt
   /// has CXXConstructExpr as its initializer, the object would be considered
   /// to be "under construction" between CXXConstructExpr and DeclStmt.
   /// 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<const Stmt *, const CXXCtorInitializer *> P,
+  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<const Stmt *, const CXXCtorInitializer *> P,
+  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<SVal> getObjectUnderConstruction(
-      ProgramStateRef State,
-      llvm::PointerUnion<const Stmt *, const CXXCtorInitializer *> P,
+  static Optional<SVal>
+  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
   /// was actually elided, track that we also need to elide the destructor.
   static ProgramStateRef elideDestructor(ProgramStateRef State,
                                          const CXXBindTemporaryExpr *BTE,
                                          const LocationContext *LC);
 

lib/Analysis/CFG.cpp

template <typename CallLikeExpr,
                 std::is_same<CallLikeExpr, CallExpr>::value ||
                 std::is_same<CallLikeExpr, CXXConstructExpr>::value ||
                 std::is_same<CallLikeExpr, ObjCMessageExpr>::value>>
   void findConstructionContextsForArguments(CallLikeExpr *E) {
     for (unsigned i = 0, e = E->getNumArgs(); i != e; ++i) {
       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);
     }
   }
 
   // Unset the construction context after consuming it. This is done immediately
   // after adding the CFGConstructor or CFGCXXRecordTypedCall element, so
   // there's no need to do this manually in every Visit... function.
   void cleanupConstructionContext(Expr *E);

 void CFGBuilder::findConstructionContexts(
     const ConstructionContextLayer *Layer, Stmt *Child) {
   if (!BuildOpts.AddRichCXXConstructors)
     return;
 
   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()) {
   case Stmt::CXXConstructExprClass:
   case Stmt::CXXTemporaryObjectExprClass: {
     // Support pre-C++17 copy elision AST.
     auto *CE = cast<CXXConstructExpr>(Child);
     if (BuildOpts.MarkElidedCXXConstructors && CE->isElidable()) {
case Stmt::CXXBindTemporaryExprClass: {
     findConstructionContexts(withExtraLayer(BTE), BTE->getSubExpr());
     break;
   }
   case Stmt::MaterializeTemporaryExprClass: {
     // Normally we don't want to search in MaterializeTemporaryExpr because
     // 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<CXXConstructExpr>(Layer->getTriggerStmt())) {
-      if (CE->isElidable()) {
-        auto *MTE = cast<MaterializeTemporaryExpr>(Child);
-        findConstructionContexts(withExtraLayer(MTE), MTE->GetTemporaryExpr());
-      }
-    }
+    if (Layer->getItem().getKind() ==
+        ConstructionContextItem::ElidableConstructorKind) {
     break;
   }
   case Stmt::ConditionalOperatorClass: {
     auto *CO = cast<ConditionalOperator>(Child);
-    if (!dyn_cast_or_null<MaterializeTemporaryExpr>(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
       // to support this case.
       assert(!CO->getType()->getAsCXXRecordDecl() || CO->isGLValue() ||
              Context->getLangOpts().CPlusPlus17);
       break;
     }
if (C) {
       // If this block contains a condition variable, add both the condition
       // variable and initializer to the CFG.
       if (VarDecl *VD = F->getConditionVariable()) {
         if (Expr *Init = VD->getInit()) {
           autoCreateBlock();
           const DeclStmt *DS = F->getConditionVariableDeclStmt();
           assert(DS->isSingleDecl());
           findConstructionContexts(
-              ConstructionContextLayer::create(cfg->getBumpVectorContext(),
-                                               const_cast<DeclStmt *>(DS)),
+              ConstructionContextLayer::create(cfg->getBumpVectorContext(), DS),
               Init);
           appendStmt(Block, DS);
           EntryConditionBlock = addStmt(Init);
           assert(Block == EntryConditionBlock);
           maybeAddScopeBeginForVarDecl(EntryConditionBlock, VD, C);
         }
       }
 

lib/Analysis/ConstructionContext.cpp

 //===----------------------------------------------------------------------===//
 
 #include "clang/Analysis/ConstructionContext.h"
 #include "clang/AST/ExprObjC.h"
 
 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<ConstructionContextLayer>();
-  return new (CC) ConstructionContextLayer(Trigger, Index, Parent);
+  return new (CC) ConstructionContextLayer(Item, Parent);
 }
 
 bool ConstructionContextLayer::isStrictlyMoreSpecificThan(
     const ConstructionContextLayer *Other) const {
   const ConstructionContextLayer *Self = this;
   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();
   }
   llvm_unreachable("The above loop can only be terminated via return!");
 }
 
-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<DeclStmt>(S)) {
-      assert(TopLayer->isLast());
-      return create<SimpleVariableConstructionContext>(C, DS);
-    }
-    if (const auto *NE = dyn_cast<CXXNewExpr>(S)) {
-      assert(TopLayer->isLast());
-      return create<NewAllocatedObjectConstructionContext>(C, NE);
-    }
-    if (const auto *BTE = dyn_cast<CXXBindTemporaryExpr>(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<MaterializeTemporaryExpr>(
-                 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;
-          }
+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 (MTE->getStorageDuration() != SD_FullExpression) {
 
-          // Handle pre-C++17 copy and move elision.
-          const CXXConstructExpr *ElidedCE = nullptr;
-          const ConstructionContext *ElidedCC = nullptr;
-          if (const ConstructionContextLayer *ElidedLayer =
-                  ParentLayer->getParent()) {
-            ElidedCE = cast<CXXConstructExpr>(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<SimpleTemporaryObjectConstructionContext>(C, BTE,
-                                                                      MTE);
-            } else {
-              return create<ElidedTemporaryObjectConstructionContext>(
-                  C, BTE, MTE, ElidedCE, ElidedCC);
-            }
-          }
-          assert(ParentLayer->isLast());
-          return create<SimpleTemporaryObjectConstructionContext>(C, BTE, MTE);
-        }
-        assert(ParentLayer->isLast());
+  if (ParentLayer) {
+    const ConstructionContextItem &ElidedItem = ParentLayer->getItem();
+    assert(ElidedItem.getKind() ==
+           ConstructionContextItem::ElidableConstructorKind);
+    ElidedCE = cast<CXXConstructExpr>(ElidedItem.getStmt());
+    ElidedCC = createFromLayers(C, ParentLayer->getParent());
+      return create<SimpleTemporaryObjectConstructionContext>(C, BTE, MTE);
+    }
+
+  // This is a normal temporary.
+  assert(!ParentLayer);
 
-        // This is a constructor into a function argument.
-        if (isa<CallExpr>(ParentLayer->getTriggerStmt()) ||
-            isa<CXXConstructExpr>(ParentLayer->getTriggerStmt()) ||
-            isa<ObjCMessageExpr>(ParentLayer->getTriggerStmt())) {
-          return create<ArgumentConstructionContext>(
-              C, cast<Expr>(ParentLayer->getTriggerStmt()),
-              ParentLayer->getIndex(), BTE);
-        }
-        // This is C++17 copy-elided construction into return statement.
-        if (auto *RS = dyn_cast<ReturnStmt>(ParentLayer->getTriggerStmt())) {
-          assert(!RS->getRetValue()->getType().getCanonicalType()
-                    ->getAsCXXRecordDecl()->hasTrivialDestructor());
-          return create<CXX17ElidedCopyReturnedValueConstructionContext>(C,
-                                                                       RS, BTE);
-        }
-        // This is C++17 copy-elided construction into a simple variable.
-        if (auto *DS = dyn_cast<DeclStmt>(ParentLayer->getTriggerStmt())) {
-          assert(!cast<VarDecl>(DS->getSingleDecl())->getType()
-                      .getCanonicalType()->getAsCXXRecordDecl()
-                      ->hasTrivialDestructor());
-          return create<CXX17ElidedCopyVariableConstructionContext>(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<SimpleTemporaryObjectConstructionContext>(C, BTE,
-                                                              /*MTE=*/nullptr);
-    }
-    if (const auto *MTE = dyn_cast<MaterializeTemporaryExpr>(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;
+const ConstructionContext *ConstructionContext::createBoundTemporaryFromLayers(
+    BumpVectorContext &C, const CXXBindTemporaryExpr *BTE,
+    const ConstructionContextLayer *ParentLayer) {
+  if (!ParentLayer) {
 
-      // Handle pre-C++17 copy and move elision.
-      const CXXConstructExpr *ElidedCE = nullptr;
-      const ConstructionContext *ElidedCC = nullptr;
-      if (const ConstructionContextLayer *ElidedLayer = TopLayer->getParent()) {
-        ElidedCE = cast<CXXConstructExpr>(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<SimpleTemporaryObjectConstructionContext>(C, nullptr,
-                                                                  MTE);
-        }
-        return create<ElidedTemporaryObjectConstructionContext>(
-            C, nullptr, MTE, ElidedCE, ElidedCC);
-      }
-      assert(TopLayer->isLast());
-      return create<SimpleTemporaryObjectConstructionContext>(C, nullptr, MTE);
-    }
-    if (const auto *RS = dyn_cast<ReturnStmt>(S)) {
-      assert(TopLayer->isLast());
-      return create<SimpleReturnedValueConstructionContext>(C, RS);
-    }
-    // This is a constructor into a function argument.
-    if (isa<CallExpr>(TopLayer->getTriggerStmt()) ||
-        isa<CXXConstructExpr>(TopLayer->getTriggerStmt()) ||
-        isa<ObjCMessageExpr>(TopLayer->getTriggerStmt())) {
-      assert(TopLayer->isLast());
-      return create<ArgumentConstructionContext>(
-          C, cast<Expr>(TopLayer->getTriggerStmt()), TopLayer->getIndex(),
-          /*BTE=*/nullptr);
-    }
-    llvm_unreachable("Unexpected construction context with statement!");
-  } else if (const CXXCtorInitializer *I = TopLayer->getTriggerInit()) {
+  const ConstructionContextItem &ParentItem = ParentLayer->getItem();
+  switch (ParentItem.getKind()) {
+  case ConstructionContextItem::VariableKind: {
+    const auto *DS = cast<DeclStmt>(ParentItem.getStmt());
+    assert(!cast<VarDecl>(DS->getSingleDecl())->getType().getCanonicalType()
+                            ->getAsCXXRecordDecl()->hasTrivialDestructor());
+    return create<CXX17ElidedCopyVariableConstructionContext>(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<ReturnStmt>(ParentItem.getStmt());
+    assert(!RS->getRetValue()->getType().getCanonicalType()
+              ->getAsCXXRecordDecl()->hasTrivialDestructor());
+    return create<CXX17ElidedCopyReturnedValueConstructionContext>(C, RS,
+                                                                   BTE);
+  }
+
+  case ConstructionContextItem::MaterializationKind: {
+    // No assert. We may have an elidable copy on the grandparent layer.
+    const auto *MTE = cast<MaterializeTemporaryExpr>(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<Expr>(ParentItem.getStmt());
+    assert(isa<CallExpr>(E) || isa<CXXConstructExpr>(E) ||
+           isa<ObjCMessageExpr>(E));
+    return create<ArgumentConstructionContext>(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<CXX17ElidedCopyConstructorInitializerConstructionContext>(
+        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.
+  const ConstructionContextItem &TopItem = TopLayer->getItem();
+  switch (TopItem.getKind()) {
+  case ConstructionContextItem::VariableKind: {
+    const auto *DS = cast<DeclStmt>(TopItem.getStmt());
+    return create<SimpleVariableConstructionContext>(C, DS);
+  case ConstructionContextItem::NewAllocatorKind: {
+    const auto *NE = cast<CXXNewExpr>(TopItem.getStmt());
+    return create<NewAllocatedObjectConstructionContext>(C, NE);
+  case ConstructionContextItem::ReturnKind: {
+    const auto *RS = cast<ReturnStmt>(TopItem.getStmt());
+    return create<SimpleReturnedValueConstructionContext>(C, RS);
+  case ConstructionContextItem::MaterializationKind: {
+    const auto *MTE = cast<MaterializeTemporaryExpr>(TopItem.getStmt());
+    return createMaterializedTemporaryFromLayers(C, MTE, /*BTE=*/nullptr,
+                                                 TopLayer->getParent());
+  }
+  case ConstructionContextItem::TemporaryDestructorKind: {
+    const auto *BTE = cast<CXXBindTemporaryExpr>(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<SimpleConstructorInitializerConstructionContext>(C, I);
   }
+  case ConstructionContextItem::ArgumentKind: {
+    assert(TopLayer->isLast());
+    const auto *E = cast<Expr>(TopItem.getStmt());
+    return create<ArgumentConstructionContext>(C, E, TopItem.getIndex(),
+                                               /*BTE=*/nullptr);
+  }
+  } // switch (TopItem.getKind())
   llvm_unreachable("Unexpected construction context!");
 }

lib/StaticAnalyzer/Core/ExprEngine.cpp

 /// ConstructedObjectKey is used for being able to find the path-sensitive
 /// memory region of a freshly constructed object while modeling the AST node
 /// that syntactically represents the object that is being constructed.
 /// Semantics of such nodes may sometimes require access to the region that's
 /// not otherwise present in the program state, or to the very fact that
 /// the construction context was present and contained references to these
 /// AST nodes.
 class ConstructedObjectKey {
-  typedef std::pair<
-      llvm::PointerUnion<const Stmt *, const CXXCtorInitializer *>,
-      const LocationContext *> ConstructedObjectKeyImpl;
+  typedef std::pair<ConstructionContextItem, const LocationContext *>
+      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<const Stmt *, const CXXCtorInitializer *> P,
+  explicit ConstructedObjectKey(const ConstructionContextItem &Item,
                        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<DeclStmt>(getStmt()) ||
-           isa<CXXNewExpr>(getStmt()) || isa<CXXBindTemporaryExpr>(getStmt()) ||
-           isa<MaterializeTemporaryExpr>(getStmt()) ||
-           isa<CXXConstructExpr>(getStmt()));
-  }
-
-  const Stmt *getStmt() const {
-    return Impl.first.dyn_cast<const Stmt *>();
-  }
+      : Impl(Item, LC) {}
 
-  const CXXCtorInitializer *getCXXCtorInitializer() const {
-    return Impl.first.dyn_cast<const CXXCtorInitializer *>();
-  }
-
-  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);
   }
 
   bool operator==(const ConstructedObjectKey &RHS) const {
     return Impl == RHS.Impl;
   }
 
   bool operator<(const ConstructedObjectKey &RHS) const {
     return Impl < RHS.Impl;
   }
 };
 
 typedef llvm::ImmutableMap<ConstructedObjectKey, SVal>
     ObjectsUnderConstructionMap;
 REGISTER_TRAIT_WITH_PROGRAMSTATE(ObjectsUnderConstruction,
                                  ObjectsUnderConstructionMap)
 
-// Additionally, track a set of destructors that correspond to elided
-// constructors when copy elision occurs.
-typedef std::pair<const CXXBindTemporaryExpr *, const LocationContext *>
-    ElidedDestructorItem;
-typedef llvm::ImmutableSet<ElidedDestructorItem>
-    ElidedDestructorSet;
-REGISTER_TRAIT_WITH_PROGRAMSTATE(ElidedDestructors,
-                                 ElidedDestructorSet)
-
 //===----------------------------------------------------------------------===//
 // Engine construction and deletion.
 //===----------------------------------------------------------------------===//
 
 static const char* TagProviderName = "ExprEngine";
 
 ExprEngine::ExprEngine(cross_tu::CrossTranslationUnitContext &CTU,
                        AnalysisManager &mgr, bool gcEnabled,
ExprEngine::createTemporaryRegionIfNeeded(ProgramStateRef State,
   State = State->BindExpr(Result, LC, Reg);
 
   // Notify checkers once for two bindLoc()s.
   State = processRegionChange(State, TR, LC);
 
   return State;
 }
 
-ProgramStateRef ExprEngine::addObjectUnderConstruction(
-    ProgramStateRef State,
-    llvm::PointerUnion<const Stmt *, const CXXCtorInitializer *> P,
+ProgramStateRef
+ExprEngine::addObjectUnderConstruction(ProgramStateRef State,
+                                       const ConstructionContextItem &Item,
                                        const LocationContext *LC, SVal V) {
-  ConstructedObjectKey Key(P, LC->getStackFrame());
+  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<ObjectsUnderConstruction>(Key) ||
-         isa<CXXBindTemporaryExpr>(Key.getStmt()));
+         Key.getItem().getKind() ==
+             ConstructionContextItem::TemporaryDestructorKind);
   return State->set<ObjectsUnderConstruction>(Key, V);
 }
 
-Optional<SVal> ExprEngine::getObjectUnderConstruction(
-    ProgramStateRef State,
-    llvm::PointerUnion<const Stmt *, const CXXCtorInitializer *> P,
+Optional<SVal>
+ExprEngine::getObjectUnderConstruction(ProgramStateRef State,
+                                       const ConstructionContextItem &Item,
                                        const LocationContext *LC) {
-  ConstructedObjectKey Key(P, LC->getStackFrame());
+  ConstructedObjectKey Key(Item, LC->getStackFrame());
   return Optional<SVal>::create(State->get<ObjectsUnderConstruction>(Key));
 }
 
-ProgramStateRef ExprEngine::finishObjectConstruction(
-    ProgramStateRef State,
-    llvm::PointerUnion<const Stmt *, const CXXCtorInitializer *> P,
+ProgramStateRef
+ExprEngine::finishObjectConstruction(ProgramStateRef State,
+                                     const ConstructionContextItem &Item,
                                      const LocationContext *LC) {
-  ConstructedObjectKey Key(P, LC->getStackFrame());
+  ConstructedObjectKey Key(Item, LC->getStackFrame());
   assert(State->contains<ObjectsUnderConstruction>(Key));
   return State->remove<ObjectsUnderConstruction>(Key);
 }
 
 ProgramStateRef ExprEngine::elideDestructor(ProgramStateRef State,
                                             const CXXBindTemporaryExpr *BTE,
                                             const LocationContext *LC) {
-  ElidedDestructorItem I(BTE, LC);
-  assert(!State->contains<ElidedDestructors>(I));
-  return State->add<ElidedDestructors>(I);
+  ConstructedObjectKey Key({BTE, /*IsElided=*/true}, LC);
+  assert(!State->contains<ObjectsUnderConstruction>(Key));
+  return State->set<ObjectsUnderConstruction>(Key, UnknownVal());
 }
 
 ProgramStateRef
 ExprEngine::cleanupElidedDestructor(ProgramStateRef State,
                                     const CXXBindTemporaryExpr *BTE,
                                     const LocationContext *LC) {
-  ElidedDestructorItem I(BTE, LC);
-  assert(State->contains<ElidedDestructors>(I));
-  return State->remove<ElidedDestructors>(I);
+  ConstructedObjectKey Key({BTE, /*IsElided=*/true}, LC);
+  assert(State->contains<ObjectsUnderConstruction>(Key));
+  return State->remove<ObjectsUnderConstruction>(Key);
 }
 
 bool ExprEngine::isDestructorElided(ProgramStateRef State,
                                     const CXXBindTemporaryExpr *BTE,
                                     const LocationContext *LC) {
-  ElidedDestructorItem I(BTE, LC);
-  return State->contains<ElidedDestructors>(I);
+  ConstructedObjectKey Key({BTE, /*IsElided=*/true}, LC);
+  return State->contains<ObjectsUnderConstruction>(Key);
 }
 
 bool ExprEngine::areAllObjectsFullyConstructed(ProgramStateRef State,
                                                const LocationContext *FromLC,
                                                const LocationContext *ToLC) {
   const LocationContext *LC = FromLC;
   while (LC != ToLC) {
     assert(LC && "ToLC must be a parent of FromLC!");
     for (auto I : State->get<ObjectsUnderConstruction>())
       if (I.first.getLocationContext() == LC)
         return false;
 
-    for (auto I: State->get<ElidedDestructors>())
-      if (I.second == LC)
-        return false;
-
     LC = LC->getParent();
   }
   return true;
 }
 
 
 //===----------------------------------------------------------------------===//
 // Top-level transfer function logic (Dispatcher).
static void printObjectsUnderConstructionForContext(raw_ostream &Out,
   for (auto I : State->get<ObjectsUnderConstruction>()) {
     ConstructedObjectKey Key = I.first;
     SVal Value = I.second;
     if (Key.getLocationContext() != LC)
       continue;
     Key.print(Out, nullptr, PP);
     Out << " : " << Value << NL;
   }
-
-  for (auto I : State->get<ElidedDestructors>()) {
-    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,
                             const char *NL, const char *Sep,
                             const LocationContext *LCtx) {
   if (LCtx) {
     if (!State->get<ObjectsUnderConstruction>().isEmpty()) {
       Out << Sep << "Objects under construction:" << NL;
// should go away, but the assertion should remain.
     const LocationContext *FromLC = Pred->getLocationContext();
     const LocationContext *ToLC = FromLC->getStackFrame()->getParent();
     const LocationContext *LC = FromLC;
     while (LC != ToLC) {
       assert(LC && "ToLC must be a parent of FromLC!");
       for (auto I : State->get<ObjectsUnderConstruction>())
         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<CXXBindTemporaryExpr>(I.first.getStmt()));
+          assert(I.first.getItem().getKind() ==
+                     ConstructionContextItem::TemporaryDestructorKind ||
+                 I.first.getItem().getKind() ==
+                     ConstructionContextItem::ElidedDestructorKind);
           State = State->remove<ObjectsUnderConstruction>(I.first);
-          // Also cleanup the elided destructor info.
-          ElidedDestructorItem Item(
-              cast<CXXBindTemporaryExpr>(I.first.getStmt()),
-              I.first.getLocationContext());
-          State = State->remove<ElidedDestructors>(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<ElidedDestructors>())
-          if (I.second == LC)
-            State = State->remove<ElidedDestructors>(I);
-
       LC = LC->getParent();
     }
     if (State != Pred->getState()) {
       Pred = Bldr.generateNode(Pred->getLocation(), State, Pred);
       if (!Pred) {
         // The node with clean temporaries already exists. We might have reached
         // it on a path on which we initialize different temporaries.
         return;