vsavchenko added inline comments.

================
Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:2263-2264
 Tracker::Tracker(PathSensitiveBugReport &Report) : Report(Report) {
   addHighPriorityHandler<DefaultExpressionHandler>();
+  addLowPriorityHandler<PRValueHandler>();
   // TODO: split trackExpressionValue and FindLastStoreBRVisitor into handlers
----------------
Szelethus wrote:
> I checked this commit out, and failed to see how these low/high priority 
> handlers work out in practice. I tried to
> * Make the default handler low prio as well
> * Make the PRValue high prio
> * All other combinations of the those values
> * Change the order of adding these handlers
> 
> No tests failed. Is there any one patch so far that demonstrates why we need 
> this? A unit test?
add with high priority == add to the beginning of the queue
add with low priority == add to the end of the queue

This means that it doesn't matter with what priority we add the very first one.
I guess with these two it doesn't matter which order they have one against 
another, that's why tests don't fail.  This code here keeps the same order it 
was originally in `trackExpressionValue` to maintain parity.

In the future commits, `InterestingLValueHandler` HAVE TO BE before before 
`DefaultExpressionHandler` and there are tests that validate it.  Actually this 
is the only place that I know that really cares about the order and that's why 
I needed to add priorities and the way for one handler to cancel all further 
handlers.

Does this answer your question?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103630

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to