[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 inline comments.


================
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);
   }
----------------
xazax.hun wrote:
> Szelethus wrote:
> > ..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?
> > 
> >  Shouldn't we have a distinct element for the assignment?
> 
> Strictly speaking, we have CFGs for a function. The assignment is **not** in this function, it is in the `operator()` of the class representing this lambda expression. 
> 
> So basically, we do have a `LambdaExpr` to represent the expression, but the body of the lambda is in a separate entity.
Well, `debug.DumpCFG` definitely doesn't indulge me with a separate lambda CFG, so I figured this is a (rightful) optimization or compression.

My point is, this entire code snippet is a seriously error prone, best-effort heuristic. Switch casing every small little corner case might be tedious, troublesome in terms of scaling, and I for sure don't want to maintain it 'til eternity, but we have to acknowledge that this isn't a perfect solution either.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82598