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
