Szelethus 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. ---------------- NoQ wrote: > Szelethus wrote: > > NoQ wrote: > > > 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. > > Hmm, maybe I could add that to my TODO list :) > > > > I think the comment should still mention cc1 flags, but with an addition: > > > Add a -cc1 only if your flag is related to the Static Analyzer, but > > > doesn't actually change the actual analysis. > Sounds good! Or something like that: "TODO: Some of these options are > controlled by raw frontend flags. These should be eventually converted into > -analyzer-config flags. New analyzer options should not be implemented as > frontend flags. Frontend flags still make sense for things that do not affect > the actual analysis." Will do that! Aaaafter I'm confident enough that the previous patches cause no more trouble. https://reviews.llvm.org/D53483 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
