[PATCH] D62883: [analyzer] Track terminator conditions on which a tracked expressions depends

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

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

Artem had some comments that are not marked as done, but LGTM!



================
Comment at: clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h:157
+  /// Conditions we're already tracking.
+  llvm::SmallPtrSet<const Expr *, 4> TrackedConditions;
+
----------------
Szelethus wrote:
> xazax.hun wrote:
> > Do we need this? I wonder if marking the result of the condition as interesting is sufficient.
> Later on, when I'm covering the "should-not-have-happened" case, it's possible that a condition would be added that wasn't even explored by the analyzer, so this is kinda necessary.
When the analyzer did not explore a given code snippet we will not have an exploded node, but we can change this in a followup patch.


================
Comment at: clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h:355-357
+  bool addTrackedCondition(const ExplodedNode *Cond) {
+    return TrackedConditions.insert(Cond).second;
+  }
----------------
NoQ wrote:
> Pls add a comment that explains when does this function return true or false. I always forget what does insert().second do :)
This is not done yet :)


================
Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:1628
+    if (const Expr *Condition = NB->getTerminatorConditionExpr())
+      if (BR.addTrackedCondition(N))
+        bugreporter::trackExpressionValue(
----------------
NoQ wrote:
> Maybe let's add a comment that this is for inter-visitor communication only. Because otherwise we won't visit the same node twice in the same visitor.
Also not done.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62883