[analyzer] Deprecate `-analyzer-store region` flag

[steakhal]

Summary

I'm trying to remove unused options from the Analyses.def file, then
merge the rest of the useful options into the AnalyzerOptions.def.
Then make sure one can set these by an -analyzer-config XXX=YYY style
flag.
Then surface the -analyzer-config to the clang frontend;

After all of this, we can pursue the tablegen approach described
https://discourse.llvm.org/t/rfc-tablegen-clang-static-analyzer-engine-options-for-better-documentation/61488

In this patch, I'm proposing flag deprecations.
We should support deprecated analyzer flags for exactly one release. In
this case I'm planning to drop this flag in clang-16.

In the clang frontend, now we won't pass this option to the cc1
frontend, rather emit a warning diagnostic reminding the users about
this deprecated flag, which will be turned into error in clang-16.

Unfortunately, I had to remove all the tests referring to this flag,
causing a mass change. I've also added a test for checking this warning.

I've seen that scan-build also uses this flag, but I think we should
remove that part only after we turn this into a hard error.

[martong]

We should support deprecated analyzer flags for at least one release. In this case I'm planning to drop this flag in clang-17

Should we emit a warning for the user about the deprecation?

[steakhal]

In D126215#3531780, @martong wrote:

We should support deprecated analyzer flags for at least one release. In this case I'm planning to drop this flag in clang-17

Should we emit a warning for the user about the deprecation?

Definitely!

[steakhal]

• Emit a warning when passing this option.
• State explicitly in the release notes that passing this option to clang-17 and above will cause a hard error.
• Remove all spellings of this option in tests.
• Add a test for testing the deprecation warning.

[steakhal]

@NoQ, please have a look at this.

clang/test/Analysis/deprecated-flags-and-options.cpp

1–13 ↗

(On Diff #431618)

This is the new test file. All the rest of the changed test files are unimportant.

[martong]

LGTM.

[steakhal]

• Specify exactly that we propose one release deprecation, then removal.
• Test the help message.

[martong]

Still looks good.

[steakhal]

I believe, the silence from the code owner means that he agrees with this change.
I'll land it tomorrow.

[martong]

In D126215#3565572, @steakhal wrote:

I believe, the silence from the code owner means that he agrees with this change.
I'll land it tomorrow.

I believe there is a misconception about the role of a code owner, we cannot expect Artem to review all patches. According to the LLVM Developer Poicy

The sole responsibility of a code owner is to ensure that a commit to their area of the code is appropriately reviewed, either by themself or by someone else.

Many other analyzer pathces in the past had been reviewed by me and you @steakhal and we didn't receive any complaints after they landed. So, I think it is okay to land this one as well.

[thakis]

Looks like this breaks building clang-tidy: http://45.33.8.238/linux/78232/step_4.txt

Please take a look, and revert for now if it takes a while to fix.

[steakhal]

In D126215#3572984, @thakis wrote:

Looks like this breaks building clang-tidy: http://45.33.8.238/linux/78232/step_4.txt

Please take a look, and revert for now if it takes a while to fix.

Thanks. It should be fixed by now.

[steakhal]

• Add the new diag::warn_analyzer_deprecated_option warning to the "deprecated-static-analyzer-flag" DiagGroup to prevent breaking the clang/test/Misc/warning-flags.c test file.

I'm also adding @thakis to review this change, prior to proceeding.

[thakis]

(As long as this builds and passes tests, I don't have an opinion on this change.)

clang/docs/ReleaseNotes.rst

- `New CTU implementation
   to single-TU analysis, the lost reports are highly likely to be false
   positives.
 
 - Added a new checker ``alpha.unix.cstring.UninitializedRead`` this will check for uninitialized reads
   from common memory copy/manipulation functions such as ``memcpy``, ``mempcpy``, ``memmove``, ``memcmp``, `
   `strcmp``, ``strncmp``, ``strcpy``, ``strlen``, ``strsep`` and many more. Although 
   this checker currently is in list of alpha checkers due to a false positive.
 
+- Deprecate ``-analyzer-store region`` analyzer flag. This flag is still
+  accepted, but ``clang-17`` will reject this flag.
+
 .. _release-notes-ubsan:
 
 Undefined Behavior Sanitizer (UBSan)
 ------------------------------------
 
 Core Analysis Improvements
 ==========================
 

clang/include/clang/Driver/Options.td

 let Flags = [CC1Option, NoDriverOption] in {
 
 def analysis_UnoptimizedCFG : Flag<["-"], "unoptimized-cfg">,
   HelpText<"Generate unoptimized CFGs for all analyses">,
   MarshallingInfoFlag<AnalyzerOpts<"UnoptimizedCFG">>;
 def analysis_CFGAddImplicitDtors : Flag<["-"], "cfg-add-implicit-dtors">,
   HelpText<"Add C++ implicit destructors to CFGs for all analyses">;
 
+// We should remove this option in clang-17 release.
 def analyzer_store : Separate<["-"], "analyzer-store">,
-  HelpText<"Source Code Analysis - Abstract Memory Store Models">;
+  HelpText<"Source Code Analysis - Abstract Memory Store Models [DEPRECATED]">;
 def analyzer_store_EQ : Joined<["-"], "analyzer-store=">, Alias<analyzer_store>;
 
 def analyzer_constraints : Separate<["-"], "analyzer-constraints">,
   HelpText<"Source Code Analysis - Symbolic Constraint Engines">;
 def analyzer_constraints_EQ : Joined<["-"], "analyzer-constraints=">,
   Alias<analyzer_constraints>;
 
 def analyzer_output : Separate<["-"], "analyzer-output">,

clang/include/clang/StaticAnalyzer/Core/Analyses.def

 //===-- Analyses.def - Metadata about Static Analyses -----------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 //  This file defines the set of static analyses used by AnalysisConsumer.
 //
 //===----------------------------------------------------------------------===//
 
-#ifndef ANALYSIS_STORE
-#define ANALYSIS_STORE(NAME, CMDFLAG, DESC, CREATFN)
-#endif
-
-ANALYSIS_STORE(RegionStore, "region", "Use region-based analyzer store",
-               CreateRegionStoreManager)
-
 #ifndef ANALYSIS_CONSTRAINTS
 #define ANALYSIS_CONSTRAINTS(NAME, CMDFLAG, DESC, CREATFN)
 #endif
 
 ANALYSIS_CONSTRAINTS(RangeConstraints, "range",
                      "Use constraint tracking of concrete value ranges",
                      CreateRangeConstraintManager)
 

 #endif
 
 ANALYSIS_INLINING_MODE(All, "all", "Analyze all functions as top level")
 
 ANALYSIS_INLINING_MODE(
     NoRedundancy, "noredundancy",
     "Do not analyze a function which has been previously inlined")
 
-#undef ANALYSIS_STORE
 #undef ANALYSIS_CONSTRAINTS
 #undef ANALYSIS_DIAGNOSTICS
 #undef ANALYSIS_PURGE
 #undef ANALYSIS_INLINING_MODE
 #undef ANALYSIS_IPA

clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h

 
 /// Analysis - Set of available source code analyses.
 enum Analyses {
 #define ANALYSIS(NAME, CMDFLAG, DESC, SCOPE) NAME,
 #include "clang/StaticAnalyzer/Core/Analyses.def"
 NumAnalyses
 };
 
-/// AnalysisStores - Set of available analysis store models.
-enum AnalysisStores {
-#define ANALYSIS_STORE(NAME, CMDFLAG, DESC, CREATFN) NAME##Model,
-#include "clang/StaticAnalyzer/Core/Analyses.def"
-NumStores
-};
-
 /// AnalysisConstraints - Set of available constraint models.
 enum AnalysisConstraints {
 #define ANALYSIS_CONSTRAINTS(NAME, CMDFLAG, DESC, CREATFN) NAME##Model,
 #include "clang/StaticAnalyzer/Core/Analyses.def"
 NumConstraints
 };
 
 /// AnalysisDiagClients - Set of available diagnostic clients for rendering
public:
   std::vector<std::pair<std::string, bool>> CheckersAndPackages;
 
   /// Vector of checker/package names which will not emit warnings.
   std::vector<std::string> SilencedCheckersAndPackages;
 
   /// A key-value table of use-specified configuration values.
   // TODO: This shouldn't be public.
   ConfigTable Config;
-  AnalysisStores AnalysisStoreOpt = RegionStoreModel;
   AnalysisConstraints AnalysisConstraintsOpt = RangeConstraintsModel;
   AnalysisDiagClients AnalysisDiagOpt = PD_HTML;
   AnalysisPurgeMode AnalysisPurgeOpt = PurgeStmt;
 
   std::string AnalyzeSpecificFunction;
 
   /// File path to which the exploded graph should be dumped.
   std::string DumpExplodedGraphTo;

clang/lib/Frontend/CompilerInvocation.cpp

#define ANALYZER_OPTION_WITH_MARSHALLING(                                      \
     DEFAULT_VALUE, IMPLIED_CHECK, IMPLIED_VALUE, NORMALIZER, DENORMALIZER,     \
     MERGER, EXTRACTOR, TABLE_INDEX)                                            \
   GENERATE_OPTION_WITH_MARSHALLING(                                            \
       Args, SA, KIND, FLAGS, SPELLING, ALWAYS_EMIT, KEYPATH, DEFAULT_VALUE,    \
       IMPLIED_CHECK, IMPLIED_VALUE, DENORMALIZER, EXTRACTOR, TABLE_INDEX)
 #include "clang/Driver/Options.inc"
 #undef ANALYZER_OPTION_WITH_MARSHALLING
 
-  if (Opts.AnalysisStoreOpt != RegionStoreModel) {
-    switch (Opts.AnalysisStoreOpt) {
-#define ANALYSIS_STORE(NAME, CMDFLAG, DESC, CREATFN)                           \
-  case NAME##Model:                                                            \
-    GenerateArg(Args, OPT_analyzer_store, CMDFLAG, SA);                        \
-    break;
-#include "clang/StaticAnalyzer/Core/Analyses.def"
-    default:
-      llvm_unreachable("Tried to generate unknown analysis store.");
-    }
-  }
-
   if (Opts.AnalysisConstraintsOpt != RangeConstraintsModel) {
     switch (Opts.AnalysisConstraintsOpt) {
 #define ANALYSIS_CONSTRAINTS(NAME, CMDFLAG, DESC, CREATFN)                     \
   case NAME##Model:                                                            \
     GenerateArg(Args, OPT_analyzer_constraints, CMDFLAG, SA);                  \
     break;
 #include "clang/StaticAnalyzer/Core/Analyses.def"
     default:
#define ANALYZER_OPTION_WITH_MARSHALLING(                                      \
     DEFAULT_VALUE, IMPLIED_CHECK, IMPLIED_VALUE, NORMALIZER, DENORMALIZER,     \
     MERGER, EXTRACTOR, TABLE_INDEX)                                            \
   PARSE_OPTION_WITH_MARSHALLING(                                               \
       Args, Diags, ID, FLAGS, PARAM, SHOULD_PARSE, KEYPATH, DEFAULT_VALUE,     \
       IMPLIED_CHECK, IMPLIED_VALUE, NORMALIZER, MERGER, TABLE_INDEX)
 #include "clang/Driver/Options.inc"
 #undef ANALYZER_OPTION_WITH_MARSHALLING
 
-  if (Arg *A = Args.getLastArg(OPT_analyzer_store)) {
-    StringRef Name = A->getValue();
-    AnalysisStores Value = llvm::StringSwitch<AnalysisStores>(Name)
-#define ANALYSIS_STORE(NAME, CMDFLAG, DESC, CREATFN) \
-      .Case(CMDFLAG, NAME##Model)
-#include "clang/StaticAnalyzer/Core/Analyses.def"
-      .Default(NumStores);
-    if (Value == NumStores) {
-      Diags.Report(diag::err_drv_invalid_value)
-        << A->getAsString(Args) << Name;
-    } else {
-      Opts.AnalysisStoreOpt = Value;
-    }
-  }
-
   if (Arg *A = Args.getLastArg(OPT_analyzer_constraints)) {
     StringRef Name = A->getValue();
     AnalysisConstraints Value = llvm::StringSwitch<AnalysisConstraints>(Name)
 #define ANALYSIS_CONSTRAINTS(NAME, CMDFLAG, DESC, CREATFN) \
       .Case(CMDFLAG, NAME##Model)
 #include "clang/StaticAnalyzer/Core/Analyses.def"
       .Default(NumConstraints);
     if (Value == NumConstraints) {

clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp

CREATEFN(Opts->getDiagOpts(), PathConsumers, OutDir, PP, CTU,              \
              MacroExpansions);                                                 \
     break;
 #include "clang/StaticAnalyzer/Core/Analyses.def"
     default:
       llvm_unreachable("Unknown analyzer output type!");
     }
 
     // Create the analyzer component creators.
-    switch (Opts->AnalysisStoreOpt) {
-    default:
-      llvm_unreachable("Unknown store manager.");
-#define ANALYSIS_STORE(NAME, CMDFLAG, DESC, CREATEFN)           \
-      case NAME##Model: CreateStoreMgr = CREATEFN; break;
-#include "clang/StaticAnalyzer/Core/Analyses.def"
-    }
+    CreateStoreMgr = &CreateRegionStoreManager;
 
     switch (Opts->AnalysisConstraintsOpt) {
     default:
       llvm_unreachable("Unknown constraint manager.");
 #define ANALYSIS_CONSTRAINTS(NAME, CMDFLAG, DESC, CREATEFN)     \
       case NAME##Model: CreateConstraintMgr = CREATEFN; break;
 #include "clang/StaticAnalyzer/Core/Analyses.def"
     }