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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
