spaits wrote:
My rationale behind this change is that, as I mentioned this class is for a
very specific purpose. It is a module of sub-system, not a generic "library"
class that is intended to be used from anywhere, so this class would not really
make sense to be used anywhere else.
If we take a look at the definition of the `ExpressionHandler` base class in
`BugReporterVisitor.h`, then we can see that the third argument should be the
node where the evaluation of the expression happens. We should not look for
that node in the function. It is not the function's responsibility but the
caller's responsibility.
```C++
/// Handle the given expression from the given node.
///
/// \param E The expression value which we are tracking
/// \param Original A node "downstream" where the tracking started.
/// \param ExprNode A node where the evaluation of \c E actually happens.
/// \param Opts Tracking options specifying how we are tracking the value.
virtual Tracker::Result handle(const Expr *E, const ExplodedNode *Original,
const ExplodedNode *ExprNode,
TrackingOptions Opts) = 0;
```
So there is already a written precondition for that parameter. I think the best
way to express this precondition in C++17 is an assertion. If the user of the
function call it incorrectly they should be warned as strongly as possible.
With the least amount of runtime cost. Now we could save the cost for building
a call stack, a comparison, a conditional jump based on the result of the
previous comparison and also the deconstruction of the stack.
I also respect and accept your opinions and understand why you prefer that
extra function call.
I will wait a bit with this PR. If no one else will review it in the next few
weeks I will just abort it.
Thank you for your time and effort.
https://github.com/llvm/llvm-project/pull/75076
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
