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

Sun Nov 25 13:22:27 PST 2018

Charusso added a comment.

@Szelethus thanks you for the comments!

In D53076#1307336 <https://reviews.llvm.org/D53076#1307336>, @Szelethus wrote:

> I think the reason why the printed message was either along the lines of "this is 0" and "this is non-0", is that we don't necessarily know what constraint solver we're using, and adding more non-general code `BugReporter` is most probably a bad approach.

It was an unimplemented feature to write out known stuff. There is no constraint solving, it is rather constant value dumping.

> How about we tinker with `ContraintManager` a little more, maybe add an `explainValueOfVarDecl`, that would return the value of a variable in a given `ProgramState`? We could implement it for `RangeConstraintSolver` (essentially move the code you already wrote there), and leave a `TODO` with a super generic implementation for Z3.

I agree with @george.karpenkov, we want to have the smallest scope.

================
Comment at: lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:1892-1920
+  const auto CurrentCR = N->getState()->get<ConstraintRange>();
+  const auto PredCR = PredState->get<ConstraintRange>();
+  const auto PredPredCR = PredPredState->get<ConstraintRange>();
+
+  // We only want to check BlockEdges once so we have to determine if the change
+  // of the range information is not happened because of dead symbols.
+  //
----------------
Szelethus wrote:
> `ConstraintRange`, as far as I know, is the data  `RangedConstraintManager` stores in the GDM. What if we're using a different constraint solver? I think it'd be better to take the more abstract approach, extend `ConstraintManager`'s interface with an `isEqual` or `operator==` pure virtual method, and implement it within it's descendants.
Because we only operate in the `ConditionBRVisitor` it would be useless for now.

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

https://reviews.llvm.org/D53076