[PATCH] D53076: [analyzer] Enhance ConditionBRVisitor to write out more information

[Csaba Dabis via Phabricator via cfe-commits]

Charusso added inline comments.


================
Comment at: test/Analysis/MisusedMovedObject.cpp:187
     A a;
-    if (i == 1) { // expected-note {{Taking false branch}} expected-note {{Taking false branch}}
+    if (i == 1) { // expected-note {{Assuming 'i' is not equal to 1}} expected-note {{Taking false branch}}
+      // expected-note at -1 {{Assuming 'i' is not equal to 1}} expected-note at -1 {{Taking false branch}}
----------------
NoQ wrote:
> These assumptions were already made on the previous branches. There should be no extra assumptions here.
Agree but only if there is no extra constraint EventPiece between them.


================
Comment at: test/Analysis/MisusedMovedObject.cpp:221
     }
-    if (i > 5) { // expected-note {{Taking true branch}}
+    if (i > 5) { // expected-note {{Assuming 'i' is > 5}} expected-note {{Taking true branch}}
       a.foo();   // expected-warning {{Method call on a 'moved-from' object 'a'}} expected-note {{Method call on a 'moved-from' object 'a'}}
----------------
NoQ wrote:
> We have assumed that `i` is `>= 10` on the previous branch. It imples that `i` is greater than `5`, so no additional assumption is being made here.
Agree but only if there is no extra constraint EventPiece between them.


================
Comment at: test/Analysis/NewDelete-path-notes.cpp:10
   if (p)
-    // expected-note at -1 {{Taking true branch}}
+    // expected-note at -1 {{Assuming 'p' is non-null}}
+    // expected-note at -2 {{Taking true branch}}
----------------
NoQ wrote:
> Static Analyzer knows that the standard operator new never returns null. Therefore no assumption is being made here.
As I see SA knows nothing. Where to teach it?


================
Comment at: test/Analysis/inline-plist.c:46
   if (p == 0) {
-    // expected-note at -1 {{Taking true branch}}
+    // expected-note at -1 {{Assuming 'p' is equal to null}}
+    // expected-note at -2 {{Taking true branch}}
----------------
NoQ wrote:
> The condition `!!p` above being assumed to false ensures that `p` is equal to `null` here. We are not assuming it again here.
Agree but only if there is no extra constraint EventPiece between them.


https://reviews.llvm.org/D53076