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

This revision was automatically updated to reflect the committed changes.
Closed by commit rGdd1d5488e47d: [analyzer][Liveness][NFC] Get rid of statement 
liveness, because such a thing… (authored by Szelethus).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82598

Files:
  clang/docs/analyzer/developer-docs/DebugChecks.rst
  clang/include/clang/Analysis/Analyses/LiveVariables.h
  clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h
  clang/lib/Analysis/LiveVariables.cpp
  clang/lib/StaticAnalyzer/Checkers/DebugCheckers.cpp
  clang/lib/StaticAnalyzer/Core/Environment.cpp
  clang/lib/StaticAnalyzer/Core/SymbolManager.cpp
  clang/test/Analysis/live-stmts.cpp
  clang/test/Analysis/live-stmts.mm

Index: clang/test/Analysis/live-stmts.mm
===
--- clang/test/Analysis/live-stmts.mm
+++ clang/test/Analysis/live-stmts.mm
@@ -1,5 +1,5 @@
 // RUN: %clang_analyze_cc1 -w -fblocks %s \
-// RUN:   -analyzer-checker=debug.DumpLiveStmts \
+// RUN:   -analyzer-checker=debug.DumpLiveExprs \
 // RUN:   2>&1 | FileCheck %s
 
 @interface Item
@@ -18,25 +18,25 @@
 public:
   RAII(Blk blk): blk(blk) {}
 
-// CHECK: [ B0 (live statements at block exit) ]
+// CHECK: [ B0 (live expressions at block exit) ]
 // CHECK-EMPTY:
 // CHECK-EMPTY:
-// CHECK-NEXT: [ B1 (live statements at block exit) ]
+// CHECK-NEXT: [ B1 (live expressions at block exit) ]
 // CHECK-EMPTY:
 // CHECK-EMPTY:
-// CHECK-NEXT: [ B2 (live statements at block exit) ]
+// CHECK-NEXT: [ B2 (live expressions at block exit) ]
 // CHECK-EMPTY:
 // CHECK-EMPTY:
 
   ~RAII() { blk(); }
 
-// CHECK-NEXT: [ B0 (live statements at block exit) ]
+// CHECK-NEXT: [ B0 (live expressions at block exit) ]
 // CHECK-EMPTY:
 // CHECK-EMPTY:
-// CHECK-NEXT: [ B1 (live statements at block exit) ]
+// CHECK-NEXT: [ B1 (live expressions at block exit) ]
 // CHECK-EMPTY:
 // CHECK-EMPTY:
-// CHECK-NEXT: [ B2 (live statements at block exit) ]
+// CHECK-NEXT: [ B2 (live expressions at block exit) ]
 // CHECK-EMPTY:
 // CHECK-EMPTY:
 };
@@ -45,57 +45,37 @@
   RAII raii(^{});
   for (Item *item in coll) {}
 }
-// CHECK-NEXT: [ B0 (live statements at block exit) ]
+// CHECK-NEXT: [ B0 (live expressions at block exit) ]
 // CHECK-EMPTY:
 // CHECK-EMPTY:
-// CHECK-NEXT: [ B1 (live statements at block exit) ]
+// CHECK-NEXT: [ B1 (live expressions at block exit) ]
 // CHECK-EMPTY:
 // CHECK-EMPTY:
-// CHECK-NEXT: [ B2 (live statements at block exit) ]
-// CHECK-EMPTY:
-// CHECK-NEXT: DeclStmt {{.*}}
-// CHECK-NEXT: `-VarDecl {{.*}}  item 'Item *'
+// CHECK-NEXT: [ B2 (live expressions at block exit) ]
 // CHECK-EMPTY:
 // CHECK-NEXT: ImplicitCastExpr {{.*}} 'Collection *' 
 // CHECK-NEXT: `-DeclRefExpr {{.*}} 'Collection *' lvalue ParmVar {{.*}} 'coll' 'Collection *'
 // CHECK-EMPTY:
-// CHECK-NEXT: CompoundStmt {{.*}}
-// CHECK-EMPTY:
 // CHECK-EMPTY:
-// CHECK-NEXT: [ B3 (live statements at block exit) ]
-// CHECK-EMPTY:
-// CHECK-NEXT: DeclStmt {{.*}}
-// CHECK-NEXT: `-VarDecl {{.*}}  item 'Item *'
+// CHECK-NEXT: [ B3 (live expressions at block exit) ]
 // CHECK-EMPTY:
 // CHECK-NEXT: ImplicitCastExpr {{.*}} 'Collection *' 
 // CHECK-NEXT: `-DeclRefExpr {{.*}} 'Collection *' lvalue ParmVar {{.*}} 'coll' 'Collection *'
 // CHECK-EMPTY:
-// CHECK-NEXT: CompoundStmt {{.*}}
-// CHECK-EMPTY:
 // CHECK-EMPTY:
-// CHECK-NEXT: [ B4 (live statements at block exit) ]
-// CHECK-EMPTY:
-// CHECK-NEXT: DeclStmt {{.*}}
-// CHECK-NEXT: `-VarDecl {{.*}}  item 'Item *'
+// CHECK-NEXT: [ B4 (live expressions at block exit) ]
 // CHECK-EMPTY:
 // CHECK-NEXT: ImplicitCastExpr {{.*}} 'Collection *' 
 // CHECK-NEXT: `-DeclRefExpr {{.*}} 'Collection *' lvalue ParmVar {{.*}} 'coll' 'Collection *'
 // CHECK-EMPTY:
-// CHECK-NEXT: CompoundStmt {{.*}}
-// CHECK-EMPTY:
-// CHECK-EMPTY:
-// CHECK-NEXT: [ B5 (live statements at block exit) ]
-// CHECK-EMPTY:
-// CHECK-NEXT: DeclStmt {{.*}}
-// CHECK-NEXT: `-VarDecl {{.*}}  item 'Item *'
 // CHECK-EMPTY:
-// CHECK-NEXT: CompoundStmt {{.*}}
+// CHECK-NEXT: [ B5 (live expressions at block exit) ]
 // CHECK-EMPTY:
 // CHECK-EMPTY:
-// CHECK-NEXT: [ B0 (live statements at block exit) ]
+// CHECK-NEXT: [ B0 (live expressions at block exit) ]
 // CHECK-EMPTY:
 // CHECK-EMPTY:
-// CHECK-NEXT: [ B1 (live statements at block exit) ]
+// CHECK-NEXT: [ B1 (live expressions at block exit) ]
 // CHECK-EMPTY:
 // CHECK-EMPTY:
 
Index: clang/test/Analysis/live-stmts.cpp
===
--- clang/test/Analysis/live-stmts.cpp
+++ clang/test/Analysis/live-stmts.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_analyze_cc1 -w -analyzer-checker=debug.DumpLiveStmts %s 2>&1\
+// RUN: %clang_analyze_cc1 -w -analyzer-checker=debug.DumpLiveExprs %s 2>&1\
 // RUN:   | FileCheck %s
 
 int coin();
@@ -7,13 +7,24 @@
 int 

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

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun accepted this revision.
xazax.hun added a comment.

So now all of the dependencies are landed and this should be ready for its 
prime time, right?


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

https://reviews.llvm.org/D82598

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[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 updated this revision to Diff 291231.
Szelethus added a comment.

Rename the live statements checker to live expressions checker. The test file 
added in a revert commit changed rather heavily, but it makes sense that these 
entries are removed IMO. Unless anyone objects, I'll intend to land this as-is.


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

https://reviews.llvm.org/D82598

Files:
  clang/docs/analyzer/developer-docs/DebugChecks.rst
  clang/include/clang/Analysis/Analyses/LiveVariables.h
  clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h
  clang/lib/Analysis/LiveVariables.cpp
  clang/lib/StaticAnalyzer/Checkers/DebugCheckers.cpp
  clang/lib/StaticAnalyzer/Core/Environment.cpp
  clang/lib/StaticAnalyzer/Core/SymbolManager.cpp
  clang/test/Analysis/live-stmts.cpp
  clang/test/Analysis/live-stmts.mm

Index: clang/test/Analysis/live-stmts.mm
===
--- clang/test/Analysis/live-stmts.mm
+++ clang/test/Analysis/live-stmts.mm
@@ -1,5 +1,5 @@
 // RUN: %clang_analyze_cc1 -w -fblocks %s \
-// RUN:   -analyzer-checker=debug.DumpLiveStmts \
+// RUN:   -analyzer-checker=debug.DumpLiveExprs \
 // RUN:   2>&1 | FileCheck %s
 
 @interface Item
@@ -18,25 +18,25 @@
 public:
   RAII(Blk blk): blk(blk) {}
 
-// CHECK: [ B0 (live statements at block exit) ]
+// CHECK: [ B0 (live expressions at block exit) ]
 // CHECK-EMPTY:
 // CHECK-EMPTY:
-// CHECK-NEXT: [ B1 (live statements at block exit) ]
+// CHECK-NEXT: [ B1 (live expressions at block exit) ]
 // CHECK-EMPTY:
 // CHECK-EMPTY:
-// CHECK-NEXT: [ B2 (live statements at block exit) ]
+// CHECK-NEXT: [ B2 (live expressions at block exit) ]
 // CHECK-EMPTY:
 // CHECK-EMPTY:
 
   ~RAII() { blk(); }
 
-// CHECK-NEXT: [ B0 (live statements at block exit) ]
+// CHECK-NEXT: [ B0 (live expressions at block exit) ]
 // CHECK-EMPTY:
 // CHECK-EMPTY:
-// CHECK-NEXT: [ B1 (live statements at block exit) ]
+// CHECK-NEXT: [ B1 (live expressions at block exit) ]
 // CHECK-EMPTY:
 // CHECK-EMPTY:
-// CHECK-NEXT: [ B2 (live statements at block exit) ]
+// CHECK-NEXT: [ B2 (live expressions at block exit) ]
 // CHECK-EMPTY:
 // CHECK-EMPTY:
 };
@@ -45,57 +45,37 @@
   RAII raii(^{});
   for (Item *item in coll) {}
 }
-// CHECK-NEXT: [ B0 (live statements at block exit) ]
+// CHECK-NEXT: [ B0 (live expressions at block exit) ]
 // CHECK-EMPTY:
 // CHECK-EMPTY:
-// CHECK-NEXT: [ B1 (live statements at block exit) ]
+// CHECK-NEXT: [ B1 (live expressions at block exit) ]
 // CHECK-EMPTY:
 // CHECK-EMPTY:
-// CHECK-NEXT: [ B2 (live statements at block exit) ]
-// CHECK-EMPTY:
-// CHECK-NEXT: DeclStmt {{.*}}
-// CHECK-NEXT: `-VarDecl {{.*}}  item 'Item *'
+// CHECK-NEXT: [ B2 (live expressions at block exit) ]
 // CHECK-EMPTY:
 // CHECK-NEXT: ImplicitCastExpr {{.*}} 'Collection *' 
 // CHECK-NEXT: `-DeclRefExpr {{.*}} 'Collection *' lvalue ParmVar {{.*}} 'coll' 'Collection *'
 // CHECK-EMPTY:
-// CHECK-NEXT: CompoundStmt {{.*}}
-// CHECK-EMPTY:
 // CHECK-EMPTY:
-// CHECK-NEXT: [ B3 (live statements at block exit) ]
-// CHECK-EMPTY:
-// CHECK-NEXT: DeclStmt {{.*}}
-// CHECK-NEXT: `-VarDecl {{.*}}  item 'Item *'
+// CHECK-NEXT: [ B3 (live expressions at block exit) ]
 // CHECK-EMPTY:
 // CHECK-NEXT: ImplicitCastExpr {{.*}} 'Collection *' 
 // CHECK-NEXT: `-DeclRefExpr {{.*}} 'Collection *' lvalue ParmVar {{.*}} 'coll' 'Collection *'
 // CHECK-EMPTY:
-// CHECK-NEXT: CompoundStmt {{.*}}
-// CHECK-EMPTY:
 // CHECK-EMPTY:
-// CHECK-NEXT: [ B4 (live statements at block exit) ]
-// CHECK-EMPTY:
-// CHECK-NEXT: DeclStmt {{.*}}
-// CHECK-NEXT: `-VarDecl {{.*}}  item 'Item *'
+// CHECK-NEXT: [ B4 (live expressions at block exit) ]
 // CHECK-EMPTY:
 // CHECK-NEXT: ImplicitCastExpr {{.*}} 'Collection *' 
 // CHECK-NEXT: `-DeclRefExpr {{.*}} 'Collection *' lvalue ParmVar {{.*}} 'coll' 'Collection *'
 // CHECK-EMPTY:
-// CHECK-NEXT: CompoundStmt {{.*}}
-// CHECK-EMPTY:
-// CHECK-EMPTY:
-// CHECK-NEXT: [ B5 (live statements at block exit) ]
-// CHECK-EMPTY:
-// CHECK-NEXT: DeclStmt {{.*}}
-// CHECK-NEXT: `-VarDecl {{.*}}  item 'Item *'
 // CHECK-EMPTY:
-// CHECK-NEXT: CompoundStmt {{.*}}
+// CHECK-NEXT: [ B5 (live expressions at block exit) ]
 // CHECK-EMPTY:
 // CHECK-EMPTY:
-// CHECK-NEXT: [ B0 (live statements at block exit) ]
+// CHECK-NEXT: [ B0 (live expressions at block exit) ]
 // CHECK-EMPTY:
 // CHECK-EMPTY:
-// CHECK-NEXT: [ B1 (live statements at block exit) ]
+// CHECK-NEXT: [ B1 (live expressions at block exit) ]
 // CHECK-EMPTY:
 // CHECK-EMPTY:
 
Index: clang/test/Analysis/live-stmts.cpp
===
--- clang/test/Analysis/live-stmts.cpp
+++ clang/test/Analysis/live-stmts.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_analyze_cc1 -w -analyzer-checker=debug.DumpLiveStmts %s 2>&1\
+// RUN: %clang_analyze_cc1 -w -analyzer-checker=debug.DumpLiveExprs %s 2>&1\
 // RUN:   | 

[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 added a comment.

In D82598#2172416 , @NoQ wrote:

> I still wonder what made this statement live in my example. There must have 
> been some non-trivial liveness analysis going on that caused a statement to 
> be live; probably something to do with the C++ destructor elements.


Pushed rG032b78a0762bee129f33e4255ada6d374aa70c71 
. Mind 
that no `ObjCForCollectionStmt` was found in the `DumpLiveStms` run.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82598



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

I still wonder what made this statement live in my example. There must have 
been some non-trivial liveness analysis going on that caused a statement to be 
live; probably something to do with the C++ destructor elements.

In D82598#2172371 , @Szelethus wrote:

> Wow, I never realized I accidentally landed that assert (D82122#2172360 
> ), but I guess its great to have 
> that covered. Would you prefer to have that reverted as I'm looking to fix 
> this for good?


It doesn't add any actual functionality so i think it makes sense to revert 
unless you have a quick fix.

Also if the assert is in fact true then i'd rather make a much stronger 
statement by banning statements from the Environment entirely, as in, like, at 
compile time by making it accept `Expr *` instead of `Stmt *`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82598



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[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 added a comment.

Wow, I never realized I accidentally landed that assert (D82122#2172360 
), but I guess its great to have that 
covered. Would you prefer to have that reverted as I'm looking to fix this for 
good?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82598



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[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 added a comment.

Thank you so much, you really went out of your way to dig that out! I'll try to 
patch this somehow.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82598



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Ok, here's the crashing example with `ObjCForCollectionStmt`. It should be 
saved as an `.mm` file and it crashes under pure `--analyze`.

  @interface Item
  // ...
  @end
  
  @interface Collection
  // ...
  @end
  
  typedef void (^Blk)();
  
  struct RAII {
Blk blk;
  
  public:
RAII(Blk blk): blk(blk) {}
~RAII() { blk(); }
  };
  
  void foo(Collection *coll) {
RAII raii(^{});
for (Item *item in coll) {}
  }

The CFG ("allocate a variable, pick the item and put it into that variable, 
execute the body, repeat"):
F12397775: Screen Shot 2020-07-23 at 10.08.02 PM.png 


The interesting part of the ExplodedGraph:
F12397783: Screen Shot 2020-07-23 at 10.11.42 PM.png 


And here's the FIXME that you're looking for:

  ...
  44 /// Generate a node in \p Bldr for an iteration statement using ObjC
  45 /// for-loop iterator.
  46 static void populateObjCForDestinationSet(
  47 ExplodedNodeSet , SValBuilder ,
  48 const ObjCForCollectionStmt *S, const Stmt *elem, SVal elementV,
  49 SymbolManager , const NodeBuilderContext *currBldrCtx,
  50 StmtNodeBuilder , bool hasElements) {
  ...
  56 SVal hasElementsV = svalBuilder.makeTruthVal(hasElements);
  57
  58 // FIXME: S is not an expression. We should not be binding values to 
it.
  59 ProgramStateRef nextState = state->BindExpr(S, LCtx, hasElementsV);
  ...

So, like, the engine is conveniently assigning 0 or 1 to the 
collection-statement in the Environment when the collection is assumed to be 
empty or not.

It's obviously a hack. This shouldn't be in the Environment. This should have 
been a GDM trait attached to the collection. Ideally it should also be modeled, 
i.e. sometimes we do know whether the collection is empty, and it might even be 
modeled occasionally. But in any case this shouldn't be in the Environment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82598



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added a comment.

In D82598#2164363 , @Szelethus wrote:

> Actually, what I said initially is true:
>
> > [...] the only non-expression statements it **queried** are 
> > ObjCForCollectionStmts [...]


Btw, sorry. I somehow misunderstood the snippet and tought that the test fails 
after the change.  I am also ok with dropping this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82598



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[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 added a comment.

Ouch. Let me know how severe this is, because this is a big milestone in my 
project.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82598



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

In D82598#2164363 , @Szelethus wrote:

> > 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.


Yeah, that sounds about right; you observed the current behavior to be a 
counterexample but found no evidence that the current behavior makes any sense.

P.S. We've found an example where `ObjCForCollectionStmt`s break (i.e., your 
recently added assertion gets hit), i'll reduce and think of a patch. Still 
shocked this wasn't covered with tests.




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(Child))
+  AddLiveExpr(val.liveExprs, LV.SSetFact, E);
   }

Szelethus wrote:
> 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 = 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.
> Well, debug.DumpCFG definitely doesn't indulge me with a separate lambda CFG

Does that mean that `checkASTCodeBody` doesn't get run on lambda bodies, and 
neither do any of our path-insensitive checks?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82598



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[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(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 = 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



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun 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(Child))
+  AddLiveExpr(val.liveExprs, LV.SSetFact, E);
   }

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 = 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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82598



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[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 , @xazax.hun wrote:

> In 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 = 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(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 = 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



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added a comment.

In 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 = 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).

I am only doing guessing here, but maybe the side effect of the statement is 
pruned from the store as soon as it is no longer live?

I might be wrong here, but maybe solving this problem would not be the job of 
the liveness analysis but we have a bug elsewhere, as this binding should be 
pruned when `a` is a dead variable.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82598



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[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 added a comment.

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 = 42; }(a);
clang_analyzer_eval(a == 42); // expected-warning{{TRUE}}
  }
  
  // CHECK: [ B0 (live statements at block exit) ]
  // CHECK-EMPTY:
  // CHECK-EMPTY:
  // CHECK-NEXT: [ B1 (live statements at block exit) ]
  // CHECK-EMPTY:
  // CHECK-EMPTY:
  // CHECK-NEXT: [ B2 (live statements at block exit) ]
  // CHECK-EMPTY:
  // CHECK-NEXT: CompoundStmt {{.*}}
  // CHECK-NEXT: `-BinaryOperator {{.*}} 'int' lvalue '='
  // CHECK-NEXT:   |-DeclRefExpr {{.*}} 'int' lvalue ParmVar {{.*}} 'a' 'int &'
  // CHECK-NEXT:   `-IntegerLiteral {{.*}} 'int' 42


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82598



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

In D82598#2119545 , @xazax.hun wrote:

> > Given that we'd also barely ever notice that we forgot one of those, i'm 
> > very much in favor of having liveness analysis instead, that would 
> > declaratively describe which expressions are live when, so that to 
> > automatically guarantee that we always only track what's necessary and 
> > never snowball our state with dead expressions.
>
> Fair point. I do not remember any problem with the current liveness analysis 
> so I guess if something is not broken do not fix it. We also don't have any 
> evidence of this being a bottleneck.


D55566 !


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82598



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[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 added a comment.

Note to self: rename `debug.DumpLiveStmts` to `debug.DumpLiveExprs`. Thanks for 
the review btw! I don't immediately have something to add to your discussion 
though :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82598



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun accepted this revision.
xazax.hun added a comment.

@szelethus
The patch looks good to me and I find it a welcome change that should make 
things easier to understand and maybe even a bit more efficient, I hope this 
won't break ObjC :) My discussion with Artem is orthogonal to this change.

In D82598#2117432 , @NoQ wrote:

> Well, formally it isn't.


Right. Sorry for being unclear there.

> I guess your point is that live expressions analysis could be replaced with 
> imperative cleanup requests from, say, `ExprEngine` to the `Environment`. 
> I.e., "We've just finished evaluating the if-statement, now we should 
> actively tell the Environment to remove the condition expression".

Yes. This is what I am proposing and this is what we did with the lifetime 
analysis. Unfortunately, I did not have time yet to identify all the cleanup 
points and I do agree that this is not a trivial task but I felt that this is 
much more lightweight in terms of memory and computation.

> I guess it could totally work that way, but it'd be pretty hard to not forget 
> such cleanups.

I agree.

> Given that we'd also barely ever notice that we forgot one of those, i'm very 
> much in favor of having liveness analysis instead, that would declaratively 
> describe which expressions are live when, so that to automatically guarantee 
> that we always only track what's necessary and never snowball our state with 
> dead expressions.

Fair point. I do not remember any problem with the current liveness analysis so 
I guess if something is not broken do not fix it. We also don't have any 
evidence of this being a bottleneck.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82598



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

In D82598#2116373 , @xazax.hun wrote:

> > I suspect that it would be pretty bad if you, say, kill the condition of 
> > the `if`-statement before picking the branch. Or kill the initializer in 
> > the `DeclStmt` before putting it into the variable (same for 
> > `CXXCtorInitializer` which isn't even a `Stmt`!).
>
> I would argue that the end of the full expression is AFTER the `if` was 
> evaluated in this case. But I do see what you mean, thanks :)


Well, formally it isn't. The notion of full-expression is defined pretty 
strictly because that's the moment of time when temporary destructors are 
invoked. So the language pays a lot of attention to what exactly constitutes an 
expression, and as of now if-statements don't. In particular, temporary 
destructors will run before the branch is chosen.

I guess your point is that live expressions analysis could be replaced with 
imperative cleanup requests from, say, `ExprEngine` to the `Environment`. I.e., 
"We've just finished evaluating the if-statement, now we should actively tell 
the Environment to remove the condition expression". I guess it could totally 
work that way, but it'd be pretty hard to not forget such cleanups. Given that 
we'd also barely ever notice that we forgot one of those, i'm very much in 
favor of having liveness analysis instead, that would declaratively describe 
which expressions are live when, so that to automatically guarantee that we 
always only track what's necessary and never snowball our state with dead 
expressions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82598



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added a comment.

In D82598#2115656 , @NoQ wrote:

> > We could just kill all subexpr at the end of the full expression
>
> I suspect that it would be pretty bad if you, say, kill the condition of the 
> `if`-statement before picking the branch. Or kill the initializer in the 
> `DeclStmt` before putting it into the variable (same for `CXXCtorInitializer` 
> which isn't even a `Stmt`!).


I would argue that the end of the full expression is AFTER the `if` was 
evaluated in this case. But I do see what you mean, thanks :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82598



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Artem Dergachev via Phabricator via cfe-commits]

NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.

> We could just kill all subexpr at the end of the full expression

I suspect that it would be pretty bad if you, say, kill the condition of the 
`if`-statement before picking the branch. Or kill the initializer in the 
`DeclStmt` before putting it into the variable (same for `CXXCtorInitializer` 
which isn't even a `Stmt`!).

> I might need a tip on how to test on ObjC code :)

Unfortunately i don't have any helpful tips for you :( Such code is not 
cross-compiled very often. I vaguely remember seeing some Objective-C code in 
linux software but it's most likely not very modern/representative. So i guess 
it's on us to inform you of the potential problems.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82598



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added a comment.

I always wondered if we actually need to track the liveness of exprs at all. We 
could just kill all subexpr at the end of the full expression without 
precomputing anything. But I might miss something.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82598



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[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 created this revision.
Szelethus added reviewers: NoQ, xazax.hun, vsavchenko, martong, dcoughlin.
Szelethus added a project: clang.
Herald added subscribers: cfe-commits, ASDenysPetrov, steakhal, Charusso, 
gamesh411, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, rnkovacs, szepet, 
baloghadamsoftware, whisperity.
Szelethus added a comment.

I didn't run this on big projects just yet, and judging from the fact that 
`Environment` only contained ObjetiveC examples of non-expression statements, I 
might need a tip on how to test on ObjC code :)


The summary and very short discussion in D82122 
 summarizes whats happening here.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D82598

Files:
  clang/include/clang/Analysis/Analyses/LiveVariables.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h
  clang/lib/Analysis/LiveVariables.cpp
  clang/lib/StaticAnalyzer/Checkers/DebugCheckers.cpp
  clang/lib/StaticAnalyzer/Core/Environment.cpp
  clang/lib/StaticAnalyzer/Core/SymbolManager.cpp

Index: clang/lib/StaticAnalyzer/Core/SymbolManager.cpp
===
--- clang/lib/StaticAnalyzer/Core/SymbolManager.cpp
+++ clang/lib/StaticAnalyzer/Core/SymbolManager.cpp
@@ -482,7 +482,7 @@
 }
 
 bool
-SymbolReaper::isLive(const Stmt *ExprVal, const LocationContext *ELCtx) const {
+SymbolReaper::isLive(const Expr *ExprVal, const LocationContext *ELCtx) const {
   if (LCtx == nullptr)
 return false;
 
Index: clang/lib/StaticAnalyzer/Core/Environment.cpp
===
--- clang/lib/StaticAnalyzer/Core/Environment.cpp
+++ clang/lib/StaticAnalyzer/Core/Environment.cpp
@@ -183,12 +183,15 @@
  F.getTreeFactory());
 
   // Iterate over the block-expr bindings.
-  for (Environment::iterator I = Env.begin(), E = Env.end();
-   I != E; ++I) {
+  for (Environment::iterator I = Env.begin(), End = Env.end(); I != End; ++I) {
 const EnvironmentEntry  = I.getKey();
 const SVal  = I.getData();
 
-if (SymReaper.isLive(BlkExpr.getStmt(), BlkExpr.getLocationContext())) {
+const Expr *E = dyn_cast(BlkExpr.getStmt());
+if (!E)
+  continue;
+
+if (SymReaper.isLive(E, BlkExpr.getLocationContext())) {
   // Copy the binding to the new map.
   EBMapRef = EBMapRef.add(BlkExpr, X);
 
Index: clang/lib/StaticAnalyzer/Checkers/DebugCheckers.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/DebugCheckers.cpp
+++ clang/lib/StaticAnalyzer/Checkers/DebugCheckers.cpp
@@ -136,7 +136,7 @@
   void checkASTCodeBody(const Decl *D, AnalysisManager& Mgr,
 BugReporter ) const {
 if (LiveVariables *L = Mgr.getAnalysis(D))
-  L->dumpStmtLiveness(Mgr.getSourceManager());
+  L->dumpExprLiveness(Mgr.getSourceManager());
   }
 };
 }
Index: clang/lib/Analysis/LiveVariables.cpp
===
--- clang/lib/Analysis/LiveVariables.cpp
+++ clang/lib/Analysis/LiveVariables.cpp
@@ -27,7 +27,7 @@
 class LiveVariablesImpl {
 public:
   AnalysisDeclContext 
-  llvm::ImmutableSet::Factory SSetFact;
+  llvm::ImmutableSet::Factory SSetFact;
   llvm::ImmutableSet::Factory DSetFact;
   llvm::ImmutableSet::Factory BSetFact;
   llvm::DenseMap blocksEndToLiveness;
@@ -45,7 +45,7 @@
  LiveVariables::Observer *obs = nullptr);
 
   void dumpBlockLiveness(const SourceManager& M);
-  void dumpStmtLiveness(const SourceManager& M);
+  void dumpExprLiveness(const SourceManager& M);
 
   LiveVariablesImpl(AnalysisDeclContext , bool KillAtAssign)
 : analysisContext(ac),
@@ -54,7 +54,7 @@
   BSetFact(false),
   killAtAssign(KillAtAssign) {}
 };
-}
+} // namespace
 
 static LiveVariablesImpl (void *x) {
   return *((LiveVariablesImpl *) x);
@@ -64,8 +64,8 @@
 // Operations and queries on LivenessValues.
 //===--===//
 
-bool LiveVariables::LivenessValues::isLive(const Stmt *S) const {
-  return liveStmts.contains(S);
+bool LiveVariables::LivenessValues::isLive(const Expr *E) const {
+  return liveExprs.contains(E);
 }
 
 bool LiveVariables::LivenessValues::isLive(const VarDecl *D) const {
@@ -97,9 +97,9 @@
 LiveVariablesImpl::merge(LiveVariables::LivenessValues valsA,
  LiveVariables::LivenessValues valsB) {
 
-  llvm::ImmutableSetRef
-SSetRefA(valsA.liveStmts.getRootWithoutRetain(), SSetFact.getTreeFactory()),
-SSetRefB(valsB.liveStmts.getRootWithoutRetain(), SSetFact.getTreeFactory());
+  llvm::ImmutableSetRef
+SSetRefA(valsA.liveExprs.getRootWithoutRetain(), SSetFact.getTreeFactory()),
+SSetRefB(valsB.liveExprs.getRootWithoutRetain(), SSetFact.getTreeFactory());
 
 
   llvm::ImmutableSetRef
@@ -122,7 +122,7 @@
 }
 
 bool LiveVariables::LivenessValues::equals(const LivenessValues ) 

[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 added a comment.

I didn't run this on big projects just yet, and judging from the fact that 
`Environment` only contained ObjetiveC examples of non-expression statements, I 
might need a tip on how to test on ObjC code :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82598



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits