[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 ↗

(On Diff #132015)

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

1262 ↗

(On Diff #132015)

auto *CE?

1263 ↗

(On Diff #132015)

Could we have an explicit constructor call here?

[NoQ]

include/clang/Analysis/CFG.h

167 ↗

(On Diff #132015)

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

1262 ↗

(On Diff #132015)

Overwritten in D42719.

1263 ↗

(On Diff #132015)

Overwritten in D42719.

[dcoughlin]

include/clang/Analysis/CFG.h

184 ↗

(On Diff #132056)

Extra semi-colon here.

lib/Analysis/CFG.cpp

4649 ↗

(On Diff #132056)

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 ↗

(On Diff #132056)

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.

cfe/trunk/include/clang/Analysis/CFG.h

protected:
   CFGStmt() = default;
 };
 
 // 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.
 class ConstructionContext {
+public:
+  typedef llvm::PointerUnion<Stmt *, CXXCtorInitializer *> TriggerTy;
+
+private:
   // 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).
-  Stmt *Trigger = nullptr;
+  TriggerTy Trigger;
 
 public:
   ConstructionContext() = default;
-  ConstructionContext(Stmt *Trigger) : Trigger(Trigger) {}
+  ConstructionContext(TriggerTy Trigger)
+      : Trigger(Trigger) {}
 
-  bool isNull() const { return Trigger == nullptr; }
+  bool isNull() const { return Trigger.isNull(); }
 
-  const Stmt *getTriggerStmt() const { return Trigger; }
+  const Stmt *getTriggerStmt() const {
+    return Trigger.dyn_cast<Stmt *>();
+  }
+
+  const CXXCtorInitializer *getTriggerInit() const {
+    return Trigger.dyn_cast<CXXCtorInitializer *>();
+  }
 
   const ConstructionContext *getPersistentCopy(BumpVectorContext &C) const {
     ConstructionContext *CC = C.getAllocator().Allocate<ConstructionContext>();
     *CC = *this;
     return CC;
   }
 };
 
public:
   const ConstructionContext *getConstructionContext() const {
     return static_cast<ConstructionContext *>(Data2.getPointer());
   }
 
   const Stmt *getTriggerStmt() const {
     return getConstructionContext()->getTriggerStmt();
   }
 
+  const CXXCtorInitializer *getTriggerInit() const {
+    return getConstructionContext()->getTriggerInit();
+  }
+
 private:
   friend class CFGElement;
 
   CFGConstructor() = default;
 
   static bool isKind(const CFGElement &E) {
     return E.getKind() == Constructor;
   }

cfe/trunk/lib/Analysis/CFG.cpp

private:
   }
 
   // Scan the child statement \p Child to find the constructor that might
   // have been directly triggered by the current node, \p Trigger. If such
   // constructor has been found, set current construction context to point
   // to the trigger statement. The construction context will be unset once
   // it is consumed when the CFG building procedure processes the
   // construct-expression and adds the respective CFGConstructor element.
-  void EnterConstructionContextIfNecessary(Stmt *Trigger, Stmt *Child);
+  void EnterConstructionContextIfNecessary(
+      ConstructionContext::TriggerTy Trigger, Stmt *Child);
   // Unset the construction context after consuming it. This is done immediately
   // after adding the CFGConstructor element, so there's no need to
   // do this manually in every Visit... function.
   void ExitConstructionContext();
 
   void autoCreateBlock() { if (!Block) Block = createBlock(); }
   CFGBlock *createBlock(bool add_successor = true);
   CFGBlock *createNoReturnBlock();
if (const VariableArrayType *vat = dyn_cast<VariableArrayType>(vt))
         return vat;
 
     t = vt->getElementType().getTypePtr();
   }
 
   return nullptr;
 }
 
-void CFGBuilder::EnterConstructionContextIfNecessary(Stmt *Trigger,
-                                                     Stmt *Child) {
+void CFGBuilder::EnterConstructionContextIfNecessary(
+    ConstructionContext::TriggerTy Trigger, Stmt *Child) {
   if (!BuildOpts.AddRichCXXConstructors)
     return;
   if (!Child)
     return;
   if (isa<CXXConstructExpr>(Child)) {
     assert(CurrentConstructionContext.isNull() &&
            "Already within a construction context!");
     CurrentConstructionContext = ConstructionContext(Trigger);
if (BuildOpts.AddTemporaryDtors && HasTemporaries) {
                              /*BindToTemporary=*/false, Context);
     }
   }
 
   autoCreateBlock();
   appendInitializer(Block, I);
 
   if (Init) {
+    EnterConstructionContextIfNecessary(I, Init);
+
     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 (isa<CXXOperatorCallExpr>(S)) {
       OS << " (OperatorCall)";
     } else if (isa<CXXBindTemporaryExpr>(S)) {
       OS << " (BindTemporary)";
     } else if (const CXXConstructExpr *CCE = dyn_cast<CXXConstructExpr>(S)) {
       OS << " (CXXConstructExpr, ";
       if (Optional<CFGConstructor> CE = E.getAs<CFGConstructor>()) {
         if (const Stmt *S = CE->getTriggerStmt())
           Helper.handledStmt((const_cast<Stmt *>(S)), OS);
+        else if (const CXXCtorInitializer *I = CE->getTriggerInit())
+          print_initializer(OS, Helper, I);
         else
           llvm_unreachable("Unexpected trigger kind!");
         OS << ", ";
       }
       OS << CCE->getType().getAsString() << ")";
     } else if (const CastExpr *CE = dyn_cast<CastExpr>(S)) {
       OS << " (" << CE->getStmtClassName() << ", "
          << CE->getCastKindName()
          << ", " << CE->getType().getAsString()
          << ")";
     }
 
     // Expressions need a newline.
     if (isa<Expr>(S))
       OS << '\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();

cfe/trunk/test/Analysis/cfg-rich-constructors.cpp

 // CHECK-NEXT:     2: [B1.1] (ImplicitCastExpr, NoOp, const class C)
 // CHECK-NEXT:     3: [B1.2]
 // CHECK-NEXT:     4: const C &c = C();
 void referenceVariableWithInitializer() {
   const C &c = C();
 }
 
 } // end namespace decl_stmt
+
+namespace ctor_initializers {
+
+class D: public C {
+  C c1;
+
+public:
+
+// CHECK: D()
+// CHECK:          1:  (CXXConstructExpr, C() (Base initializer), class C)
+// CHECK-NEXT:     2: C([B1.1]) (Base initializer)
+// CHECK-NEXT:     3: CFGNewAllocator(C *)
+// CHECK-NEXT:     4:  (CXXConstructExpr, [B1.5], class C)
+// CHECK-NEXT:     5: new C([B1.4])
+// CHECK-NEXT:     6: [B1.5] (CXXConstructExpr, c1([B1.5]) (Member initializer), class C)
+// CHECK-NEXT:     7: c1([B1.6]) (Member initializer)
+  D(): C(), c1(new C()) {}
+
+// CHECK: D(int)
+// CHECK:          1:  (CXXConstructExpr, D() (Delegating initializer), class ctor_initializers::D)
+// CHECK-NEXT:     2: D([B1.1]) (Delegating initializer)
+  D(int): D() {}
+};
+
+} // end namespace ctor_initializers

cfe/trunk/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)
+// WARNINGS:    1:  (CXXConstructExpr, class A)
+// ANALYZER:    1:  (CXXConstructExpr, A() (Base initializer), class A)
 // CHECK:    2: A([B1.1]) (Base initializer)
-// CHECK:    3:  (CXXConstructExpr, class C)
+// WARNINGS:    3:  (CXXConstructExpr, class C)
+// ANALYZER:    3:  (CXXConstructExpr, C() (Base initializer), class C)
 // CHECK:    4: C([B1.3]) (Base initializer)
-// CHECK:    5:  (CXXConstructExpr, class B)
+// WARNINGS:    5:  (CXXConstructExpr, class B)
+// ANALYZER:    5:  (CXXConstructExpr, B() (Base initializer), class B)
 // CHECK:    6: B([B1.5]) (Base initializer)
-// CHECK:    7:  (CXXConstructExpr, class A)
+// WARNINGS:    7:  (CXXConstructExpr, class A)
+// ANALYZER:    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)
 // WARNINGS:   14:  (CXXConstructExpr, class A)
 // ANALYZER:   14:  (CXXConstructExpr, [B1.15], class 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)
+// WARNINGS:    3: [B1.1], [B1.2] (CXXConstructExpr, class TestDelegating)
+// ANALYZER:    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