[analyzer] Fix liveness calculation for C++17 structured bindings

C++ structured bindings for non-tuple-types are defined in a peculiar
way, where the resulting declaration is not a VarDecl, but a
BindingDecl.
That means a lot of existing machinery stops working.

rdar://36912381

Differential Revision: https://reviews.llvm.org/D44956

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@328910 91177308-0d34-0410-b5e6-96231b3b80d8

include/clang/Analysis/Analyses/LiveVariables.h

@@ -33,15 +33,18 @@ public:
 
     llvm::ImmutableSet<const Stmt *> liveStmts;
     llvm::ImmutableSet<const VarDecl *> liveDecls;
+    llvm::ImmutableSet<const BindingDecl *> liveBindings;
     
     bool equals(const LivenessValues &V) const;
 
     LivenessValues()
-      : liveStmts(nullptr), liveDecls(nullptr) {}
+      : liveStmts(nullptr), liveDecls(nullptr), liveBindings(nullptr) {}
 
     LivenessValues(llvm::ImmutableSet<const Stmt *> LiveStmts,
-                   llvm::ImmutableSet<const VarDecl *> LiveDecls)
-      : liveStmts(LiveStmts), liveDecls(LiveDecls) {}
+                   llvm::ImmutableSet<const VarDecl *> LiveDecls,
+                   llvm::ImmutableSet<const BindingDecl *> LiveBindings)
+        : liveStmts(LiveStmts), liveDecls(LiveDecls),
+          liveBindings(LiveBindings) {}
 
     bool isLive(const Stmt *S) const;
     bool isLive(const VarDecl *D) const;

lib/Analysis/LiveVariables.cpp

@@ -77,6 +77,7 @@ public:
   AnalysisDeclContext &analysisContext;
   llvm::ImmutableSet<const Stmt *>::Factory SSetFact;
   llvm::ImmutableSet<const VarDecl *>::Factory DSetFact;
+  llvm::ImmutableSet<const BindingDecl *>::Factory BSetFact;
   llvm::DenseMap<const CFGBlock *, LiveVariables::LivenessValues> blocksEndToLiveness;
   llvm::DenseMap<const CFGBlock *, LiveVariables::LivenessValues> blocksBeginToLiveness;
   llvm::DenseMap<const Stmt *, LiveVariables::LivenessValues> stmtsToLiveness;
@@ -97,6 +98,7 @@ public:
     : analysisContext(ac),
       SSetFact(false), // Do not canonicalize ImmutableSets by default.
       DSetFact(false), // This is a *major* performance win.
+      BSetFact(false),
       killAtAssign(KillAtAssign) {}
 };
 }
@@ -114,6 +116,12 @@ bool LiveVariables::LivenessValues::isLive(const Stmt *S) const {
 }
 
 bool LiveVariables::LivenessValues::isLive(const VarDecl *D) const {
+  if (const auto *DD = dyn_cast<DecompositionDecl>(D)) {
+    bool alive = false;
+    for (const BindingDecl *BD : DD->bindings())
+      alive |= liveBindings.contains(BD);
+    return alive;
+  }
   return liveDecls.contains(D);
 }
 
@@ -145,14 +153,19 @@ LiveVariablesImpl::merge(LiveVariables::LivenessValues valsA,
     DSetRefA(valsA.liveDecls.getRootWithoutRetain(), DSetFact.getTreeFactory()),
     DSetRefB(valsB.liveDecls.getRootWithoutRetain(), DSetFact.getTreeFactory());
   
+  llvm::ImmutableSetRef<const BindingDecl *>
+    BSetRefA(valsA.liveBindings.getRootWithoutRetain(), BSetFact.getTreeFactory()),
+    BSetRefB(valsB.liveBindings.getRootWithoutRetain(), BSetFact.getTreeFactory());
 
   SSetRefA = mergeSets(SSetRefA, SSetRefB);
   DSetRefA = mergeSets(DSetRefA, DSetRefB);
+  BSetRefA = mergeSets(BSetRefA, BSetRefB);
   
   // asImmutableSet() canonicalizes the tree, allowing us to do an easy
   // comparison afterwards.
   return LiveVariables::LivenessValues(SSetRefA.asImmutableSet(),
-                                       DSetRefA.asImmutableSet());  
+                                       DSetRefA.asImmutableSet(),
+                                       BSetRefA.asImmutableSet());  
 }
 
 bool LiveVariables::LivenessValues::equals(const LivenessValues &V) const {
@@ -322,6 +335,11 @@ void TransferFunctions::Visit(Stmt *S) {
   }
 }
 
+static bool writeShouldKill(const VarDecl *VD) {
+  return VD && !VD->getType()->isReferenceType() &&
+    !isAlwaysAlive(VD);
+}
+
 void TransferFunctions::VisitBinaryOperator(BinaryOperator *B) {
   if (B->isAssignmentOp()) {
     if (!LV.killAtAssign)
@@ -329,21 +347,25 @@ void TransferFunctions::VisitBinaryOperator(BinaryOperator *B) {
     
     // Assigning to a variable?
     Expr *LHS = B->getLHS()->IgnoreParens();
-    
-    if (DeclRefExpr *DR = dyn_cast<DeclRefExpr>(LHS))
-      if (const VarDecl *VD = dyn_cast<VarDecl>(DR->getDecl())) {
-        // Assignments to references don't kill the ref's address
-        if (VD->getType()->isReferenceType())
-          return;
 
-        if (!isAlwaysAlive(VD)) {
-          // The variable is now dead.
+    if (DeclRefExpr *DR = dyn_cast<DeclRefExpr>(LHS)) {
+      const Decl* D = DR->getDecl();
+      bool Killed = false;
+
+      if (const BindingDecl* BD = dyn_cast<BindingDecl>(D)) {
+        Killed = !BD->getType()->isReferenceType();
+        if (Killed)
+          val.liveBindings = LV.BSetFact.remove(val.liveBindings, BD);
+      } else if (const auto *VD = dyn_cast<VarDecl>(D)) {
+        Killed = writeShouldKill(VD);
+        if (Killed)
           val.liveDecls = LV.DSetFact.remove(val.liveDecls, VD);
-        }
 
-        if (observer)
-          observer->observerKill(DR);
       }
+
+      if (Killed && observer)
+        observer->observerKill(DR);
+    }
   }
 }
 
@@ -357,17 +379,28 @@ void TransferFunctions::VisitBlockExpr(BlockExpr *BE) {
 }
 
 void TransferFunctions::VisitDeclRefExpr(DeclRefExpr *DR) {
-  if (const VarDecl *D = dyn_cast<VarDecl>(DR->getDecl()))
-    if (!isAlwaysAlive(D) && LV.inAssignment.find(DR) == LV.inAssignment.end())
-      val.liveDecls = LV.DSetFact.add(val.liveDecls, D);
+  const Decl* D = DR->getDecl();
+  bool InAssignment = LV.inAssignment[DR];
+  if (const auto *BD = dyn_cast<BindingDecl>(D)) {
+    if (!InAssignment)
+      val.liveBindings =
+          LV.BSetFact.add(val.liveBindings, cast<BindingDecl>(D));
+  } else if (const auto *VD = dyn_cast<VarDecl>(D)) {
+    if (!InAssignment && !isAlwaysAlive(VD))
+      val.liveDecls = LV.DSetFact.add(val.liveDecls, VD);
+  }
 }
 
 void TransferFunctions::VisitDeclStmt(DeclStmt *DS) {
-  for (const auto *DI : DS->decls())
-    if (const auto *VD = dyn_cast<VarDecl>(DI)) {
+  for (const auto *DI : DS->decls()) {
+    if (const auto *DD = dyn_cast<DecompositionDecl>(DI)) {
+      for (const auto *BD : DD->bindings())
+        val.liveBindings = LV.BSetFact.remove(val.liveBindings, BD);
+    } else if (const auto *VD = dyn_cast<VarDecl>(DI)) {
       if (!isAlwaysAlive(VD))
         val.liveDecls = LV.DSetFact.remove(val.liveDecls, VD);
     }
+  }
 }
 
 void TransferFunctions::VisitObjCForCollectionStmt(ObjCForCollectionStmt *OS) {
@@ -422,12 +455,14 @@ void TransferFunctions::VisitUnaryOperator(UnaryOperator *UO) {
   case UO_PreDec:
     break;
   }
-  
-  if (DeclRefExpr *DR = dyn_cast<DeclRefExpr>(UO->getSubExpr()->IgnoreParens()))
-    if (isa<VarDecl>(DR->getDecl())) {
+
+  if (auto *DR = dyn_cast<DeclRefExpr>(UO->getSubExpr()->IgnoreParens())) {
+    const Decl *D = DR->getDecl();
+    if (isa<VarDecl>(D) || isa<BindingDecl>(D)) {
       // Treat ++/-- as a kill.
       observer->observerKill(DR);
     }
+  }
 }
 
 LiveVariables::LivenessValues
@@ -508,10 +543,10 @@ LiveVariables::computeLiveness(AnalysisDeclContext &AC,
       for (CFGBlock::const_iterator bi = block->begin(), be = block->end();
            bi != be; ++bi) {
         if (Optional<CFGStmt> cs = bi->getAs<CFGStmt>()) {
-          if (const BinaryOperator *BO =
-                  dyn_cast<BinaryOperator>(cs->getStmt())) {
+          const Stmt* stmt = cs->getStmt();
+          if (const auto *BO = dyn_cast<BinaryOperator>(stmt)) {
             if (BO->getOpcode() == BO_Assign) {
-              if (const DeclRefExpr *DR =
+              if (const auto *DR =
                     dyn_cast<DeclRefExpr>(BO->getLHS()->IgnoreParens())) {
                 LV->inAssignment[DR] = 1;
               }

test/Analysis/live-bindings-test.cpp

@@ -0,0 +1,124 @@
+// RUN: %clang_analyze_cc1 -std=c++17 -analyzer-checker=core,deadcode -verify %s
+
+typedef unsigned long size_t;
+
+// Machinery required for custom structured bindings decomposition.
+namespace std {
+template <class T> class tuple_size;
+template <class T>
+ constexpr size_t tuple_size_v = tuple_size<T>::value;
+template <size_t I, class T> class tuple_element;
+
+template<class T, T v>
+struct integral_constant {
+    static constexpr T value = v;
+    typedef T value_type;
+    typedef integral_constant type;
+    constexpr operator value_type() const noexcept { return value; }
+};
+}
+
+struct S {
+  int a;
+  double b;
+  S(int a, double b) : a(a), b(b) {};
+};
+
+S GetNumbers();
+
+int used_binding() {
+    const auto [a, b] = GetNumbers(); // no-warning
+    return a + b; 
+}
+
+void no_warning_on_copy(S s) {
+  // Copy constructor might have side effects.
+  const auto [a, b] = s; // no-warning
+}
+
+
+int unused_binding_ignored() {
+    const auto [a, b] = GetNumbers(); // expected-warning{{Value stored to '[a, b]' during its initialization is never read}}
+    return 0;
+}
+
+int unused_binding_liveness_required() {
+    auto [a2, b2] = GetNumbers(); // expected-warning{{Value stored to '[a2, b2]' during its initialization is never read}}
+    a2 = 10;
+    b2 = 20;
+    return a2 + b2;
+}
+
+int kill_one_binding() {
+  auto [a, b] = GetNumbers(); // no-warning
+  a = 100;
+  return a + b;
+
+}
+
+int kill_one_binding2() {
+  auto [a, b] = GetNumbers(); // expected-warning{{Value stored to '[a, b]' during its initialization is never read}}
+  a = 100;
+  return a;
+}
+
+void use_const_reference_bindings() {
+  const auto &[a, b] = GetNumbers(); // no-warning
+}
+
+void use_reference_bindings() {
+  S s(0, 0);
+  auto &[a, b] = s; // no-warning
+  a = 200;
+}
+
+int read_through_pointer() {
+  auto [a, b] = GetNumbers(); // no-warning
+  int *z = &a;
+  return *z;
+}
+
+auto [globalA, globalB] = GetNumbers(); // no-warning, globals
+auto [globalC, globalD] = GetNumbers(); // no-warning, globals
+
+void use_globals() {
+  globalA = 300; // no-warning
+  globalB = 200;
+}
+
+struct Mytuple {
+  int a;
+  int b;
+
+  template <size_t N>
+  int get() const {
+    if      constexpr (N == 0) return a;
+    else if constexpr (N == 1) return b;
+  }
+};
+
+namespace std {
+    template<>
+    struct tuple_size<Mytuple>
+        : std::integral_constant<size_t, 2> {};
+
+    template<size_t N>
+    struct tuple_element<N, Mytuple> {
+        using type = int;
+    };
+}
+
+void no_warning_on_tuple_types_copy(Mytuple t) {
+  auto [a, b] = t; // no-warning
+}
+
+Mytuple getMytuple();
+
+void deconstruct_tuple_types_warning() {
+  auto [a, b] = getMytuple(); // expected-warning{{Value stored to '[a, b]' during its initialization is never read}}
+}
+
+int deconstruct_tuple_types_no_warning() {
+  auto [a, b] = getMytuple(); // no-warning
+  return a + b;
+}