[PATCH] D66267: [analyzer] TrackConstraintBRVisitor: Do not track unknown values

Thu Aug 15 15:43:39 PDT 2019

Szelethus added a comment.

In D66267#1632164 <https://reviews.llvm.org/D66267#1632164>, @NoQ wrote:

> In D66267#1630728 <https://reviews.llvm.org/D66267#1630728>, @Szelethus wrote:
>
> > Shouldn't we just delete this entire visitor altogether and merge it into ConditionBRVisitor (like, eventually, not right now)? It seems to be a relic of the past.
>
>
> I'm actually curious about one particular mess that we have here. Namely, there's a visitor that says "assuming..." and there's checker notes when checkers themselves assume something; how can we be sure they don't duplicate each other?

BugReporter.cpp does some deduplication of notes originating from TrackConstraintBRVisitor and ConditionBRVisitor, but I'm not sure how many other visitors/checkers do we have doing the same.

================
Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:1898-1900
+      // If the contents are symbolic and null, find out when they became null.
+      if (V.getAsLocSymbol(/*IncludeBaseRegions=*/true))
+        if (LVState->isNull(V).isConstrainedTrue())
----------------
When `trackNulOrUndeflValue` to `trackExpressionValue`, I think it was an oversight that the comments in it weren't changed accordingly. This makes a lot more sense now, cheers!

Repository:
  rC Clang

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

https://reviews.llvm.org/D66267