NagyDonat wrote:

Thank you for the very through statistical analysis and visualization -- this 
paints a clear picture about the effects of the change.

> In conclusion, in contrast to the originally proposed variant of this patch 
> this evolved version is more controversial and the benefits are not clear 
> cut. I don't think it's simpler than the original variant, so I think we 
> reached a stale mate.
> 
> Unfortunately, I can't invest more time into this PR right now, so I'll close 
> this.

I'm sad that this "check complexity at the beginning of `evalBinOp`" idea 
didn't provide convincing results :slightly_frowning_face:

I don't see a theoretical reason why are these benefits less clear cut than 
your originally proposed change. (By the way, I'm not sure whether this variant 
causes more regressions than the originally proposed variant, because the 
statistics that you published originally don't provide information about entry 
points that became slower.)

> I had migrate the uses of the internal `evalBinOpXX` APIs to use the 
> top-level `evalBinOp` from checkers and other recursive places, to ensure 
> that the check withing `evalBinOp` is honored.

This is probably irrelevant now, but as far as I see it would've been enough to 
put the complexity checking to the beginning of `evalBinOpNN` (instead of 
`evalBinOp`) because it's the only "branch" of `evalBinOp` that can create 
complex symbols. (Pointer arithmetic is represented in a different way...)

https://github.com/llvm/llvm-project/pull/144327
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to