[PATCH] D23300: [analyzer] Add "Assuming..." diagnostic pieces for unsupported condition expressions.

[Artem Dergachev via cfe-commits]

NoQ created this revision.
NoQ added reviewers: zaks.anna, dcoughlin, xazax.hun, a.sidorin.
NoQ added a subscriber: cfe-commits.

In path diagnostics, the "Assuming ..." diagnostic piece represents the event (i.e. yellow in html diagnostics) of adding constraint to a symbol, and there are also "Taking ..." diagnostic pieces, which represent the decision on program path that was made based on existing constraints.

For example:
```
void foo(int x) {
  if (x == 2) // Assuming and Taking.
  {
    if (x == 3) { // Only Taking.
    } else {
      1 / 0; // Bug.
    }
  }
}
```
1. This code would produce the "Assuming `x` is equal to `2`" piece, which represents that on the previous execution path it was not known that `x` is equal to `2`, but now we consider this path as separate.
2. Then this code would produce the "Taking true branch" piece, which would tell the user that we inevitably follow the true branch because of the assumption we made earlier.
3. Then in the inner `if`, this code would produce the "Taking false branch" piece. However, it would not produce the new "Assuming `x` is not equal to `3`" piece, because it doesn't need to assume this - it is already inevitable.

Now, the problem with "Assuming ..." pieces is that they are "pretty". If the analyzer fails to pretty-print the assumption - i.e. transform "(x == 2)" to "`x` is equal to `2`" - then it fails to add the piece, and the user gets a false impression that taking the branch was inevitable.

Expressions unsupported by the pretty-printer are fairly common and include function call expressions, field or instance variable expressions, and logical operators.

This patch adds the generic "Assuming the condition is true/false" pieces whenever the pretty-printing fails. This way we keep understanding why the analyzer thinks that a certain branch is feasible.

This patch helps a lot with understanding bug reports, which originally seem like false-positives.

I've eye-inspected all the changes on tests, and i like all the newly added pieces.

This mechanism can still be improved by supporting more expression pretty-printing.

A couple of examples: {F2256294} {F2256295}

https://reviews.llvm.org/D23300

Files:
  include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitor.h
  lib/StaticAnalyzer/Core/BugReporter.cpp
  lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
  test/Analysis/DeallocUseAfterFreeErrors.m
  test/Analysis/conditional-path-notes.c
  test/Analysis/edges-new.mm
  test/Analysis/generics.m
  test/Analysis/inlining/path-notes.m
  test/Analysis/localization.m
  test/Analysis/malloc-plist.c
  test/Analysis/model-file.cpp
  test/Analysis/plist-output.m

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D23300.67298.patch
Type: text/x-patch
Size: 155741 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160809/6b1083bb/attachment-0001.bin>