[CFG] [analyzer] Add construction context for CXXCtorInitializer.

[NoQ]

Summary

This patch builds on top of D42672 to add construction contexts for constructors that are initializing CXXCtorInitializer nodes (C::C(int x, struct S s): x(x), s(s) /* <== these little thingines */ {}). Because CXXCtorInitializer is not a Stmt, ConstructionContext needs to be extended to support such contexts. This patch refactors ConstructionContext into a "proper" class with fancy getters (we'd most likely need setters later as well).

[george.karpenkov]

include/clang/Analysis/CFG.h

167

I think we should get<> here instead, to clearly crash on incorrect operations.
I also don't understand what dyn_cast does in such cases: doxygen comment says it returns null, yet the code calls the default empty constructor, which I think is even more confusing.

lib/Analysis/CFG.cpp

1259

auto *CE?

1260

Could we have an explicit constructor call here?

[NoQ]

include/clang/Analysis/CFG.h

167

I think we should get<> here instead, to clearly crash on incorrect operations.

Yeah, i guess, it'd eventually evolve into a whole class hierarchy of construction contexts with RTTI and stuff. I'm trying to grow it slowly because for now i don't foresee exactly how much of this we'd need.

Still, to me it seems more natural to ask "hey does this thing have a trigger statement", "oh, so maybe it has an initializer", than to enumerate all possible reasons why it may have a trigger statement or an initializer and match for those. It seems easier for callers to use.

the code calls the default empty constructor

This is an empty constructor for T, where T is in our case CXXCtorInitializer *, so it boils down to returning nullptr.

[NoQ]

Remove the stack of contexts for now. Which, by the way, i forgot to pop in this patch.

[NoQ]

lib/Analysis/CFG.cpp

1259

Overwritten in D42719.

1260

Overwritten in D42719.

[dcoughlin]

include/clang/Analysis/CFG.h

184

Extra semi-colon here.

lib/Analysis/CFG.cpp

4649

This is pretty fragile if you ever add a third kind. Do you want some kind of a 'Kind' you can switch over and get a warning when you add a new one?

[NoQ]

Rebase. Add direct tests.

[NoQ]

Add an assertion to see if we can always print the construction trigger.

lib/Analysis/CFG.cpp

4649

Yep, this is indeed likely that i'd end up adding a whole RTTI of construction contexts.
Added a direct assertion for now.

[NoQ]

Rebase.

include/clang/Analysis/CFG.h

static bool isKind(const CFGElement &E) {
     return E.getKind() == Statement;
   }
 };
 
 // This is bulky data for CFGConstructor which would not fit into the
 // CFGElement's room (pair of pointers). Contains the information
 // necessary to express what memory is being initialized by
 // the construction.
-struct ConstructionContext {
+class ConstructionContext {
   CXXConstructExpr *Constructor;
-  Stmt *Trigger;
+  llvm::PointerUnion<Stmt *, CXXCtorInitializer *> Trigger;
+
+public:
+  ConstructionContext(CXXConstructExpr *Constructor,
+                      Stmt *Trigger)
+      : Constructor(Constructor), Trigger(Trigger) {}
+
+  ConstructionContext(CXXConstructExpr *Constructor,
+                      CXXCtorInitializer *Trigger)
+      : Constructor(Constructor), Trigger(Trigger) {}
 
   const ConstructionContext *getPersistentCopy(BumpVectorContext &C) const {
     ConstructionContext *CC = C.getAllocator().Allocate<ConstructionContext>();
     *CC = *this;
     return CC;
   }
+
+  const CXXConstructExpr *getConstructor() const {
+    return Constructor;
+  }
+  const Stmt *getTriggerStmt() const {
+    return Trigger.dyn_cast<Stmt *>();
+  }
+  const CXXCtorInitializer *getTriggerInit() const {
+    return Trigger.dyn_cast<CXXCtorInitializer *>();
+  }
 };
 
 /// CFGConstructor - Represents C++ constructor call. Maintains information
 /// necessary to figure out what memory is being initialized by the
 /// constructor expression.
 class CFGConstructor : public CFGElement {
 public:
   explicit CFGConstructor(const ConstructionContext *C)
       : CFGElement(Constructor, C) {}
 
   const ConstructionContext *getConstructionContext() const  {
     return static_cast<ConstructionContext *>(Data1.getPointer());
   }
 
-  CXXConstructExpr *getConstructor() const {
-    return getConstructionContext()->Constructor;
+  const CXXConstructExpr *getConstructor() const {
+    return getConstructionContext()->getConstructor();;
+  }
+
+  QualType getType() const {
+    return getConstructor()->getType();
   }
 
-  QualType getType() const { return getConstructor()->getType(); }
+  const Stmt *getTriggerStmt() const {
+    return getConstructionContext()->getTriggerStmt();
+  }
 
-  Stmt *getTriggerStmt() const { return getConstructionContext()->Trigger; }
+  const CXXCtorInitializer *getTriggerInit() const {
+    return getConstructionContext()->getTriggerInit();
+  }
 
 private:
   friend class CFGElement;
 
   CFGConstructor() = default;
 
   static bool isKind(const CFGElement &E) {
     return E.getKind() == Constructor;

lib/Analysis/CFG.cpp

class CFGBuilder {
   // resolved targets upon completion of CFG construction.
   using BackpatchBlocksTy = std::vector<JumpSource>;
   BackpatchBlocksTy BackpatchBlocks;
 
   // A list of labels whose address has been taken (for indirect gotos).
   using LabelSetTy = llvm::SmallSetVector<LabelDecl *, 8>;
   LabelSetTy AddressTakenLabels;
 
-  ConstructionContext CurrentConstructionContext = {nullptr, nullptr};
+  ConstructionContext CurrentConstructionContext = {nullptr, (Stmt *)nullptr};
 
   bool badCFG = false;
   const CFG::BuildOptions &BuildOpts;
   
   // State to track for building switch statements.
   bool switchExclusivelyCovered = false;
   Expr::EvalResult *switchCond = nullptr;
   
if (alwaysAdd(S) && cachedEntry)
       cachedEntry->second = B;
 
     // All block-level expressions should have already been IgnoreParens()ed.
     assert(!isa<Expr>(S) || cast<Expr>(S)->IgnoreParens() == S);
     B->appendStmt(const_cast<Stmt*>(S), cfg->getBumpVectorContext());
   }
 
   void appendConstructor(CFGBlock *B, CXXConstructExpr *CE) {
-    if (CurrentConstructionContext.Constructor == CE) {
+    if (CurrentConstructionContext.getConstructor() == CE) {
       B->appendConstructor(CurrentConstructionContext,
                            cfg->getBumpVectorContext());
       return;
     }
 
     // No valid construction context found. Fall back to statement.
     B->appendStmt(CE, cfg->getBumpVectorContext());
   }
if (BuildOpts.AddTemporaryDtors && HasTemporaries) {
                              /*BindToTemporary=*/false, Context);
     }
   }
 
   autoCreateBlock();
   appendInitializer(Block, I);
 
   if (Init) {
+    if (CXXConstructExpr *CE = dyn_cast<CXXConstructExpr>(Init))
+      CurrentConstructionContext = {/*Constructor=*/CE, /*Trigger=*/I};
+
     if (HasTemporaries) {
       // For expression with temporaries go directly to subexpression to omit
       // generating destructors for the second time.
       return Visit(cast<ExprWithCleanups>(Init)->getSubExpr());
     }
     if (BuildOpts.AddCXXDefaultInitExprInCtors) {
       if (CXXDefaultInitExpr *Default = dyn_cast<CXXDefaultInitExpr>(Init)) {
         // In general, appending the expression wrapped by a CXXDefaultInitExpr
void print(CFGTerminator T) {
     if (T.isTemporaryDtorsBranch())
       OS << "(Temp Dtor) ";
     Visit(T.getStmt());
   }
 };
 
 } // namespace
 
+static void print_initializer(raw_ostream &OS, StmtPrinterHelper &Helper,
+                              const CXXCtorInitializer *I) {
+  if (I->isBaseInitializer())
+    OS << I->getBaseClass()->getAsCXXRecordDecl()->getName();
+  else if (I->isDelegatingInitializer())
+    OS << I->getTypeSourceInfo()->getType()->getAsCXXRecordDecl()->getName();
+  else
+    OS << I->getAnyMember()->getName();
+  OS << "(";
+  if (Expr *IE = I->getInit())
+    IE->printPretty(OS, &Helper, PrintingPolicy(Helper.getLangOpts()));
+  OS << ")";
+
+  if (I->isBaseInitializer())
+    OS << " (Base initializer)";
+  else if (I->isDelegatingInitializer())
+    OS << " (Delegating initializer)";
+  else
+    OS << " (Member initializer)";
+}
+
 static void print_elem(raw_ostream &OS, StmtPrinterHelper &Helper,
                        const CFGElement &E) {
   if (Optional<CFGStmt> CS = E.getAs<CFGStmt>()) {
     const Stmt *S = CS->getStmt();
     assert(S != nullptr && "Expecting non-null Stmt");
 
     // special printing for statement-expressions.
     if (const StmtExpr *SE = dyn_cast<StmtExpr>(S)) {
if (Optional<CFGStmt> CS = E.getAs<CFGStmt>()) {
 
     // Expressions need a newline.
     if (isa<Expr>(S))
       OS << '\n';
   } else if (Optional<CFGConstructor> CE = E.getAs<CFGConstructor>()) {
     CE->getConstructor()->printPretty(OS, &Helper,
                                       PrintingPolicy(Helper.getLangOpts()));
     OS << " (CXXConstructExpr, ";
-    Helper.handledStmt((CE->getTriggerStmt()), OS);
+    if (const Stmt *S = CE->getTriggerStmt())
+      Helper.handledStmt((const_cast<Stmt *>(S)), OS);
+    else
+      print_initializer(OS, Helper, CE->getTriggerInit());
     OS << ", " << CE->getType().getAsString() << ")\n";
   } else if (Optional<CFGInitializer> IE = E.getAs<CFGInitializer>()) {
-    const CXXCtorInitializer *I = IE->getInitializer();
-    if (I->isBaseInitializer())
-      OS << I->getBaseClass()->getAsCXXRecordDecl()->getName();
-    else if (I->isDelegatingInitializer())
-      OS << I->getTypeSourceInfo()->getType()->getAsCXXRecordDecl()->getName();
-    else OS << I->getAnyMember()->getName();
-
-    OS << "(";
-    if (Expr *IE = I->getInit())
-      IE->printPretty(OS, &Helper, PrintingPolicy(Helper.getLangOpts()));
-    OS << ")";
-
-    if (I->isBaseInitializer())
-      OS << " (Base initializer)\n";
-    else if (I->isDelegatingInitializer())
-      OS << " (Delegating initializer)\n";
-    else OS << " (Member initializer)\n";
+    print_initializer(OS, Helper, IE->getInitializer());
+    OS << '\n';
   } else if (Optional<CFGAutomaticObjDtor> DE =
                  E.getAs<CFGAutomaticObjDtor>()) {
     const VarDecl *VD = DE->getVarDecl();
     Helper.handleDecl(VD, OS);
 
     const Type* T = VD->getType().getTypePtr();
     if (const ReferenceType* RT = T->getAs<ReferenceType>())
       T = RT->getPointeeType().getTypePtr();

test/Analysis/initializers-cfg-output.cpp

  public:
   TestDelegating() : TestDelegating(2, 3) {}
   TestDelegating(int x, int z) : x(x), z(z) {}
 };
 
 // CHECK:  [B2 (ENTRY)]
 // CHECK:    Succs (1): B1
 // CHECK:  [B1]
-// CHECK:    1:  (CXXConstructExpr, class A)
+// CHECK:    1:  (CXXConstructExpr, A() (Base initializer), class A)
 // CHECK:    2: A([B1.1]) (Base initializer)
-// CHECK:    3:  (CXXConstructExpr, class C)
+// CHECK:    3:  (CXXConstructExpr, C() (Base initializer), class C)
 // CHECK:    4: C([B1.3]) (Base initializer)
-// CHECK:    5:  (CXXConstructExpr, class B)
+// CHECK:    5:  (CXXConstructExpr, B() (Base initializer), class B)
 // CHECK:    6: B([B1.5]) (Base initializer)
-// CHECK:    7:  (CXXConstructExpr, class A)
+// CHECK:    7:  (CXXConstructExpr, A() (Base initializer), class A)
 // CHECK:    8: A([B1.7]) (Base initializer)
 // CHECK:    9: /*implicit*/(int)0
 // CHECK:   10: i([B1.9]) (Member initializer)
 // CHECK:   11: this
 // CHECK:   12: [B1.11]->i
 // CHECK:   13: r([B1.12]) (Member initializer)
 // CHECK:   14:  (CXXConstructExpr, [B1.15], class A)
 // CHECK:   15: A a;

 // CHECK:    Succs (2): B2 B3
 // CHECK:  [B0 (EXIT)]
 // CHECK:    Preds (1): B1
 // CHECK:  [B2 (ENTRY)]
 // CHECK:    Succs (1): B1
 // CHECK:  [B1]
 // CHECK:    1: 2
 // CHECK:    2: 3
-// CHECK:    3: [B1.1], [B1.2] (CXXConstructExpr, class TestDelegating)
+// CHECK:    3: [B1.1], [B1.2] (CXXConstructExpr, TestDelegating([B1.1], [B1.2]) (Delegating initializer), class TestDelegating)
 // CHECK:    4: TestDelegating([B1.3]) (Delegating initializer)
 // CHECK:    Preds (1): B2
 // CHECK:    Succs (1): B0
 // CHECK:  [B0 (EXIT)]
 // CHECK:    Preds (1): B1