xazax.hun added a comment.

I looked at bug path with the Optional. I did not debug into the analyzer but 
my intuition is the following: `DefChainEnd` is interesting. `TerminatedPaths` 
is the control dependency of `DefChainEnd`. And there are probably other things 
that are the control and/or value dependency of `TerminatedPaths` . One trick 
we could try is to limit the number of dependencies we track. For example, we 
could try what happens if we only track the immediate control dependencies of 
the original interesting region/symbol and does not add more control dependency 
recursively.

In case we find limiting the number of dependencies useful, I could imagine a 
user-facing flag like `report-verbosity`. The default one could include only 
the notes we find useful after limiting the tracking. A detailed option could 
add all the notes from the transitive tracking of dependencies. A verbose 
option could remove path pruning as well. This is just random brainstorming :) 
First, we need to so whether non-transitive tracking it is actually improving 
the situation.

I am not sure about assuming `operator bool` being correct. I think we first 
could think about other tricks to limit the tracking (see my idea above) and if 
we fail I would only add such rules as a last resort.



================
Comment at: 
clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h:157
+  /// Conditions we're already tracking.
+  llvm::SmallPtrSet<const Expr *, 4> TrackedConditions;
+
----------------
Do we need this? I wonder if marking the result of the condition as interesting 
is sufficient.


================
Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:1850
+
+    return const_cast<CFGBlock *>(Node->getLocationContext()
+        ->getAnalysisDeclContext()->getCFGStmtMap()->getBlock(S));
----------------
Why do we need a ptr to non-const CFGBlock?


================
Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:1895
+
+  if (ControlDepTree.isControlDependency(OriginB, NB))
+    if (const Expr *Condition = getTerminatorCondition(NB))
----------------
Maybe this is the worng place to comment, but `isControlDependency` sounds a 
bit weird. How about `isControlDependenent`?


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

Reply via email to