[PATCH] D53483: [analyzer] Restrict AnalyzerOptions' interface so that non-checker objects have to be registered

Artem Dergachev via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Nov 2 12:10:11 PDT 2018


NoQ added inline comments.


================
Comment at: include/clang/StaticAnalyzer/Core/AnalyzerOptions.h:146-149
+/// If you'd like to add a new -cc1 flag, add it to
+/// include/clang/Driver/CC1Options.td, add a new field to store the value of
+/// that flag in this class, and initialize it in
+/// lib/Frontend/CompilerInvocation.cpp.
----------------
Szelethus wrote:
> NoQ wrote:
> > Szelethus wrote:
> > > NoQ wrote:
> > > > Nono, don't add more -cc1 flags :)
> > > Code review is there to stop adding unnecessary -cc1 flags. Are we sure we wouldn't even like to document it? I myself will add at least 2 more -cc1 flags in the future (-analyzer-config-help, -analyzer-checker-option-help), that undoubtedly belong there.
> > But these flags wouldn't define new analyzer options(?)
> That is correct, but we do store similar cc1 flags here, because they belong to the the analyzer.
> 
> Although, hm, some of those would be more fitting as -analyzer-config flags, but I don't see how I could pull that off in a backward compatible way.
Those were there before `AnalyzerOptions` were invented. That said, @george.karpenkov just made an experiment with converting `-analyzer-eagerly-assume` into `-analyzer-config eagerly-assume` (D51251) and it seems to have went well. For feature flags that were not really ever supposed to have been appended by GUIs, it is enough to just make sure that the correct behavior is enabled by default; there's no need to maintain backwards compatibility.


https://reviews.llvm.org/D53483





More information about the cfe-commits mailing list