NoQ added a comment. Ok. Oof. Whoa. That's amazing.
Let me re-iterate to make sure that my understanding of this patchset is correct. **Chapter 1.** //"A is correlated with B (ρ = 0.56), given C, assuming D and under E conditions"// - Two new entities are introduced: trackers and handlers. - Trackers are attached to bug reports. A single bug report may have multiple trackers. - Why not have only one tracker per report? - Handlers are attached to trackers. Each tracker can have multiple handlers with a priority system. - Do you envision it growing into a dependency system later? - A subclass of visitors known as "tracking visitors" are connected to trackers. One tracker may have multiple visitors attached to it. - Trackers are a tiny, almost uncustomizable class. It contains is a method called `track()` that is invoked when attention of handlers is required. - Each tracker comes with a default set of handlers. - The only way to customize a tracker is to add extra handlers to it. - Handlers implement a `handle()` method invoked by the tracker every time their attention is required in `track()`. - Handlers may return a note from their `handle()` method. If a handler returns a note, this note is attached to the bug report and lower-priority handlers are not invoked at all. - There are two major classes of handlers: expression handlers and store handlers. Expression handlers pay attention to Environment entries, store handlers pay attention to Region Store bindings. **Chapter 2.** //"For immediate release: Scientists find potential link between A and B"// - Bug visitors are still responsible for discovering connections across exploded nodes. On the contrary, `track()`/`handle()` workflow is instantaneous. - It sounds like this whole patchset is orthogonal to reimplementing value tracking visitors with note tags, as long as we can pass a reference to the appropriate tracker into the tag(?) - Tracking starts when a checker creates a tracker and invokes `track()` on it, which instantly causes the respective `handle()` to be invoked. - `handle()` may attach visitors. These visitors may in turn call `track()` as they reach a different exploded node. - The termination condition is as usual: no further visitors attached. - It sounds like this patchset is orthogonal to the problem of detecting the tracking termination point(?) It does make it easier to implement customized reactions to such termination though. **Chapter 3.** //"A causes B all the time. What will this means for Obama?"// - `trackExpressionValue()` now boils down to creation of a single tracker and an initial invocation of `track()` on the expression which in turn causes expression handlers to immediately react. - Most of the old body of `trackExpressionValue()` is transformed into a default expression handler. So it's still part of an immediate reaction, just a bit more generalized. - Most of the visitors that previously composed `trackExpressionValue()` are still part of the process; they're attached by the default expression handler. - However, instead of adding notes directly, they simply invoke `track()`. All note-attaching work is moved into additional default handlers. **Chapter 4.** //"I'm wearing this to ward off A!"// - Basic usage in checkers is basically unchanged. They still simply call `trackExpressionValue()` and it just works. - Advanced usage - which is the whole point of this patchset - probably implies adding custom handlers to the tracker created by `trackExpressionValue()`. - Let's consider an example. Suppose you're implementing `evalCall` for `llvm::dyn_cast`. You could have your expression handler detect a modeled `dyn_cast` call and implement its `handle()` method to emit a checker-specific note and start tracking the argument. - With this implementation such note will only be emitted if the value is tracked back to `dyn_cast`. Not every modeled `dyn_cast` will receive a note but only the one that, say, caused a null dereference on its return value when it failed. - Now, how does this interact with note tags? The idea behind note tags is so that visitors didn't need to reverse-engineer the analysis. - It sounds like the handler is a step back to the visitor world: it doesn't know how exactly was `dyn_cast` modeled during analysis, so it has to reverse-engineer what happened. - For example, in order to find out whether `dyn_cast` succeeded or failed, it has to explore the program state to find out which values are passed into the cast and returned from the cast. - Whereas a note tag would have this information accessible directly. - Hence a suggestion. Maybe instead of having a system of handlers, allow visitors to //ask// the tracker whether `track()` was invoked at this node on a certain expression or region? - I.e., polling instead of callbacks. - This would naturally integrate with note tags. - This does not solve the priority problem. We could try to solve it separately by, say, enforcing at most one note per exploded node, and then implementing a similar priority system over visitors. - Or am I missing something about your solution that makes connection to note tags a non-issue? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103605/new/ https://reviews.llvm.org/D103605 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits