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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
