[clang] Reland "[analyzer] Harden safeguards for Z3 query times" (PR #97298)

Balazs Benics via cfe-commits cfe-commits at lists.llvm.org
Wed Oct 9 23:28:23 PDT 2024


steakhal wrote:

> @steakhal As I was browsing the list of analyzer options, I was surprised to see that the numerical unsigned options added by this commit are placed in the "Boolean analyzer options" section of AnalyzerOptions.def.
I understand that it's natural to group them together with the parent option "should-crosscheck-with-z3"

Intentionally done so, like you inferred.

> [...], but I still think that it would be good to either move them to the "Unsigned analyzer options" section or perhaps reorganize the whole file to follow a more natural scheme, e.g. semantic relatedness between options or perhaps a "dumb, but obvious" alphabetical ordering.

I see little value of moving these around. I think this grouping by kind was arbitrary, and a bad choice of the past.
Alphabetical ordering usually works out because it's a fairly accepted design of having common options sharing a common prefix - which fits well with alphabetical ordering.
However, it only works if the options were named like that. And I'm pretty sure that's not the case we have here.
Renaming would be a really intrusive change, so I'd advise not pursuing. On the technical side, I frequently reach out to this file if I want to learn more about something, look up the relevant option, and hop to the commit where the feature was introduced.
Even though this workflow applies to all other files, I feel I'm doing this much more often here than with any other files; so this argument wouldn't hold for those, but may be something to consider in this specific case.

In any case, I'm weakly against. But I could be convinced otherwise, assuming that there is a "better" ordering (maybe alphabetical), or grouping for these options and we also have a way of enforcing it in CI.

https://github.com/llvm/llvm-project/pull/97298


More information about the cfe-commits mailing list