[clang] [analyzer] Enforce not making overly complicated symbols (PR #144327)
Balázs Benics via cfe-commits
cfe-commits at lists.llvm.org
Mon Jun 23 07:02:04 PDT 2025
balazs-benics-sonarsource wrote:
> > I was thinking about possible ways to unblock this change.
> > If the additional code complexity needs justification, by measuring the average impact (to ensure no regression happens in the common cases), and by repeating the measurement of the handful of edge-cases where it should bring noticeable (10%+) improvement. My estimate of these efforts would be a couple of days of work. I probably can't afford to spend this time.
>
> I don't think that these additional measurements would be worth the effort. The measurement quality was my chronologically first concern, but it's not the crux of the matter.
>
> * If I understand the situation correctly you don't claim that this patch would provide any benefit that can be detected by users who analyze a complete real-world software project (and not just individual outlier TUs).
> * I find it plausible that this PR indeed improves the runtime of a handful of edge-cases and I don't suspect that it would increase the overall runtime, but I don't think that the benefits which are only visible on isolated outlier entry points (or artificial test cases) justify the additional code complexity.
I'd say yes to both, while pointing out again that the handful of cases where the improvement can be observed are also real-world code, but rare code.
> My personal opinion is that the best decision would be abandoning this PR (costs outweigh the benefits), but this is an inherently subjective judgement, so if you are convinced that merging this PR would be better, then asking other contributors is the right way forward (and I won't block the decision if others agree with it).
Thank you for your flexibility. I still believe that obeying max symbol complexity on its own brings value even without knowing that sometimes it also eliminates long running edge cases. I believe it justifies for the added complexity, but I'm very open about refining the patch in other directions that would also ensure max symbol complexity.
/cc @Xazax-hun @haoNoQ for second opinion.
https://github.com/llvm/llvm-project/pull/144327
More information about the cfe-commits
mailing list