[PATCH] D82598: [analyzer][Liveness][NFC] Get rid of statement liveness, because such a thing doesn't exist

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus marked an inline comment as done.
Szelethus added a comment.

In D82598#2162496 <https://reviews.llvm.org/D82598#2162496>, @xazax.hun wrote:

> In D82598#2162239 <https://reviews.llvm.org/D82598#2162239>, @Szelethus wrote:
>
> > I chased my own tail for weeks before realizing that there is indeed another instance when a live **statement** is stored, other then `ObjCForCollectionStmt`...
> >
> >   void clang_analyzer_eval(bool);
> >  
> >   void test_lambda_refcapture() {
> >     int a = 6;
> >     [&](int &a) { a = 42; }(a);
> >     clang_analyzer_eval(a == 42); // expected-warning{{TRUE}}
> >   }
> >
>
>
> Hmm, interesting. I don't really understand why do we need to keep that block live, as we definitely won't use any of the value it provides (since it does not provide a value at all).


Actually, what I said initially is true:

> [...] the only non-expression statements it **queried** are ObjCForCollectionStmts [...]

so I think it'd be okay to simply drop this. Also, its easy to see why this popped up, because... (cont. in inline)



================
Comment at: clang/lib/Analysis/LiveVariables.cpp:317-320
   for (Stmt *Child : S->children()) {
-    if (Child)
-      AddLiveStmt(val.liveStmts, LV.SSetFact, Child);
+    if (const auto *E = dyn_cast_or_null<Expr>(Child))
+      AddLiveExpr(val.liveExprs, LV.SSetFact, E);
   }
----------------
..this part of the code caused the issue. Looking at the related CFG,
```lang=c++
void test_lambda_refcapture()
 [B2 (ENTRY)]
   Succs (1): B1

 [B1]
   1: 6
   2: int a = 6;
   3: operator()
   4: [B1.3] (ImplicitCastExpr, FunctionToPointerDecay, void (*)(int &) const)
   5: [&](int &a) {
    a = 42;
}
   6: [B1.5]
   7: [B1.6] (ImplicitCastExpr, NoOp, const class (lambda at /home/szelethus/Documents/llvm-project/clang/test/Analysis/live-stmts.cpp:183:3))
   8: a
   9: [B1.7]([B1.8]) (OperatorCall)
  10: clang_analyzer_eval
  11: [B1.10] (ImplicitCastExpr, FunctionToPointerDecay, void (*)(_Bool))
  12: a
  13: [B1.12] (ImplicitCastExpr, LValueToRValue, int)
  14: 42
  15: [B1.13] == [B1.14]
  16: [B1.11]([B1.15])
   Preds (1): B2
   Succs (1): B0

 [B0 (EXIT)]
   Preds (1): B1
```
its clear that element 5 added the live statement, and I think that that this entire CFG just simply isn't right. Shouldn't we have a distinct element for the assignment?



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D82598/new/

https://reviews.llvm.org/D82598