Automerge: Revert "[clang-tidy] support query based custom check" (#159380)

Reverts llvm/llvm-project#131804.

This breaks a build bot; see discussion in the original PR. I could
reproduce this problem locally. The problem is that the original PR
makes use of `registerCustomChecks` in `ClangTidy.cpp` but does not add
the new custom module to the dependencies of the target that builds that
file. Adding the dependency would create a cyclic dependency, so the fix
doesn't seem obvious. See details in the PR description.

Author: ingomueller-net

clang-tools-extra/CMakeLists.txt

@@ -5,8 +5,6 @@ include(GNUInstallDirs)
 
 option(CLANG_TIDY_ENABLE_STATIC_ANALYZER
   "Include static analyzer checks in clang-tidy" ON)
-option(CLANG_TIDY_ENABLE_QUERY_BASED_CUSTOM_CHECKS
-  "Enable query-based custom checks in clang-tidy" ON)
 
 if(CLANG_INCLUDE_TESTS)
   umbrella_lit_testsuite_begin(check-clang-tools)

clang-tools-extra/clang-tidy/CMakeLists.txt

@@ -58,7 +58,6 @@ add_subdirectory(bugprone)
 add_subdirectory(cert)
 add_subdirectory(concurrency)
 add_subdirectory(cppcoreguidelines)
-add_subdirectory(custom)
 add_subdirectory(darwin)
 add_subdirectory(fuchsia)
 add_subdirectory(google)
@@ -102,10 +101,6 @@ set(ALL_CLANG_TIDY_CHECKS
   clangTidyReadabilityModule
   clangTidyZirconModule
   )
-
-if(CLANG_TIDY_ENABLE_QUERY_BASED_CUSTOM_CHECKS)
-  list(APPEND ALL_CLANG_TIDY_CHECKS clangTidyCustomModule)
-endif()
 if(CLANG_TIDY_ENABLE_STATIC_ANALYZER)
   list(APPEND ALL_CLANG_TIDY_CHECKS clangTidyMPIModule)
 endif()

clang-tools-extra/clang-tidy/ClangTidy.cpp

@@ -53,11 +53,6 @@ LLVM_INSTANTIATE_REGISTRY(clang::tidy::ClangTidyModuleRegistry)
 
 namespace clang::tidy {
 
-namespace custom {
-extern void registerCustomChecks(const ClangTidyOptions &O,
-                                 ClangTidyCheckFactories &Factories);
-} // namespace custom
-
 namespace {
 #if CLANG_TIDY_ENABLE_STATIC_ANALYZER
 #define ANALYZER_CHECK_NAME_PREFIX "clang-analyzer-"
@@ -347,10 +342,6 @@ ClangTidyASTConsumerFactory::ClangTidyASTConsumerFactory(
     IntrusiveRefCntPtr<llvm::vfs::OverlayFileSystem> OverlayFS)
     : Context(Context), OverlayFS(std::move(OverlayFS)),
       CheckFactories(new ClangTidyCheckFactories) {
-#if CLANG_TIDY_ENABLE_QUERY_BASED_CUSTOM_CHECKS
-  if (Context.canExperimentalCustomChecks())
-    custom::registerCustomChecks(Context.getOptions(), *CheckFactories);
-#endif
   for (ClangTidyModuleRegistry::entry E : ClangTidyModuleRegistry::entries()) {
     std::unique_ptr<ClangTidyModule> Module = E.instantiate();
     Module->addCheckFactories(*CheckFactories);
@@ -420,10 +411,7 @@ ClangTidyASTConsumerFactory::createASTConsumer(
                         .getCurrentWorkingDirectory();
   if (WorkingDir)
     Context.setCurrentBuildDirectory(WorkingDir.get());
-#if CLANG_TIDY_ENABLE_QUERY_BASED_CUSTOM_CHECKS
-  if (Context.canExperimentalCustomChecks())
-    custom::registerCustomChecks(Context.getOptions(), *CheckFactories);
-#endif
+
   std::vector<std::unique_ptr<ClangTidyCheck>> Checks =
       CheckFactories->createChecksForLanguage(&Context);
 
@@ -509,13 +497,13 @@ ClangTidyOptions::OptionMap ClangTidyASTConsumerFactory::getCheckOptions() {
   return Options;
 }
 
-std::vector<std::string> getCheckNames(const ClangTidyOptions &Options,
-                                       bool AllowEnablingAnalyzerAlphaCheckers,
-                                       bool ExperimentalCustomChecks) {
+std::vector<std::string>
+getCheckNames(const ClangTidyOptions &Options,
+              bool AllowEnablingAnalyzerAlphaCheckers) {
   clang::tidy::ClangTidyContext Context(
       std::make_unique<DefaultOptionsProvider>(ClangTidyGlobalOptions(),
                                                Options),
-      AllowEnablingAnalyzerAlphaCheckers, false, ExperimentalCustomChecks);
+      AllowEnablingAnalyzerAlphaCheckers);
   ClangTidyASTConsumerFactory Factory(Context);
   return Factory.getCheckNames();
 }
@@ -536,12 +524,11 @@ void filterCheckOptions(ClangTidyOptions &Options,
 
 ClangTidyOptions::OptionMap
 getCheckOptions(const ClangTidyOptions &Options,
-                bool AllowEnablingAnalyzerAlphaCheckers,
-                bool ExperimentalCustomChecks) {
+                bool AllowEnablingAnalyzerAlphaCheckers) {
   clang::tidy::ClangTidyContext Context(
       std::make_unique<DefaultOptionsProvider>(ClangTidyGlobalOptions(),
                                                Options),
-      AllowEnablingAnalyzerAlphaCheckers, false, ExperimentalCustomChecks);
+      AllowEnablingAnalyzerAlphaCheckers);
   ClangTidyDiagnosticConsumer DiagConsumer(Context);
   auto DiagOpts = std::make_unique<DiagnosticOptions>();
   DiagnosticsEngine DE(llvm::makeIntrusiveRefCnt<DiagnosticIDs>(), *DiagOpts,
@@ -678,19 +665,15 @@ void exportReplacements(const llvm::StringRef MainFilePath,
   YAML << TUD;
 }
 
-ChecksAndOptions getAllChecksAndOptions(bool AllowEnablingAnalyzerAlphaCheckers,
-                                        bool ExperimentalCustomChecks) {
+ChecksAndOptions
+getAllChecksAndOptions(bool AllowEnablingAnalyzerAlphaCheckers) {
   ChecksAndOptions Result;
   ClangTidyOptions Opts;
   Opts.Checks = "*";
   clang::tidy::ClangTidyContext Context(
       std::make_unique<DefaultOptionsProvider>(ClangTidyGlobalOptions(), Opts),
-      AllowEnablingAnalyzerAlphaCheckers, false, ExperimentalCustomChecks);
+      AllowEnablingAnalyzerAlphaCheckers);
   ClangTidyCheckFactories Factories;
-#if CLANG_TIDY_ENABLE_QUERY_BASED_CUSTOM_CHECKS
-  if (ExperimentalCustomChecks)
-    custom::registerCustomChecks(Context.getOptions(), Factories);
-#endif
   for (const ClangTidyModuleRegistry::entry &Module :
        ClangTidyModuleRegistry::entries()) {
     Module.instantiate()->addCheckFactories(Factories);

clang-tools-extra/clang-tidy/ClangTidy.h

@@ -56,16 +56,15 @@ class ClangTidyASTConsumerFactory {
 /// Fills the list of check names that are enabled when the provided
 /// filters are applied.
 std::vector<std::string> getCheckNames(const ClangTidyOptions &Options,
-                                       bool AllowEnablingAnalyzerAlphaCheckers,
-                                       bool ExperimentalCustomChecks);
+                                       bool AllowEnablingAnalyzerAlphaCheckers);
 
 struct ChecksAndOptions {
   llvm::StringSet<> Checks;
   llvm::StringSet<> Options;
 };
 
-ChecksAndOptions getAllChecksAndOptions(bool AllowEnablingAnalyzerAlphaCheckers,
-                                        bool ExperimentalCustomChecks);
+ChecksAndOptions
+getAllChecksAndOptions(bool AllowEnablingAnalyzerAlphaCheckers = true);
 
 /// Returns the effective check-specific options.
 ///
@@ -75,8 +74,7 @@ ChecksAndOptions getAllChecksAndOptions(bool AllowEnablingAnalyzerAlphaCheckers,
 /// Options.
 ClangTidyOptions::OptionMap
 getCheckOptions(const ClangTidyOptions &Options,
-                bool AllowEnablingAnalyzerAlphaCheckers,
-                bool ExperimentalCustomChecks);
+                bool AllowEnablingAnalyzerAlphaCheckers);
 
 /// Filters CheckOptions in \p Options to only include options specified in
 /// the \p EnabledChecks which is a sorted vector.

clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp

@@ -160,12 +160,11 @@ ClangTidyError::ClangTidyError(StringRef CheckName,
 
 ClangTidyContext::ClangTidyContext(
     std::unique_ptr<ClangTidyOptionsProvider> OptionsProvider,
-    bool AllowEnablingAnalyzerAlphaCheckers, bool EnableModuleHeadersParsing,
-    bool ExperimentalCustomChecks)
+    bool AllowEnablingAnalyzerAlphaCheckers, bool EnableModuleHeadersParsing)
     : OptionsProvider(std::move(OptionsProvider)),
+
       AllowEnablingAnalyzerAlphaCheckers(AllowEnablingAnalyzerAlphaCheckers),
-      EnableModuleHeadersParsing(EnableModuleHeadersParsing),
-      ExperimentalCustomChecks(ExperimentalCustomChecks) {
+      EnableModuleHeadersParsing(EnableModuleHeadersParsing) {
   // Before the first translation unit we can get errors related to command-line
   // parsing, use dummy string for the file name in this case.
   setCurrentFile("dummy");

clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h

@@ -19,7 +19,6 @@
 #include "llvm/ADT/StringSet.h"
 #include "llvm/Support/Regex.h"
 #include <optional>
-#include <utility>
 
 namespace clang {
 
@@ -69,13 +68,10 @@ struct ClangTidyStats {
 /// \endcode
 class ClangTidyContext {
 public:
-  ClangTidyContext(std::unique_ptr<ClangTidyOptionsProvider> OptionsProvider)
-      : ClangTidyContext(std::move(OptionsProvider), false, false, false) {}
   /// Initializes \c ClangTidyContext instance.
   ClangTidyContext(std::unique_ptr<ClangTidyOptionsProvider> OptionsProvider,
-                   bool AllowEnablingAnalyzerAlphaCheckers,
-                   bool EnableModuleHeadersParsing,
-                   bool ExperimentalCustomChecks);
+                   bool AllowEnablingAnalyzerAlphaCheckers = false,
+                   bool EnableModuleHeadersParsing = false);
   /// Sets the DiagnosticsEngine that diag() will emit diagnostics to.
   // FIXME: this is required initialization, and should be a constructor param.
   // Fix the context -> diag engine -> consumer -> context initialization cycle.
@@ -214,10 +210,6 @@ class ClangTidyContext {
     return EnableModuleHeadersParsing;
   }
 
-  // whether experimental custom checks can be enabled.
-  // enabled with `--experimental-custom-checks`
-  bool canExperimentalCustomChecks() const { return ExperimentalCustomChecks; }
-
   void setSelfContainedDiags(bool Value) { SelfContainedDiags = Value; }
 
   bool areDiagsSelfContained() const { return SelfContainedDiags; }
@@ -266,7 +258,6 @@ class ClangTidyContext {
 
   bool AllowEnablingAnalyzerAlphaCheckers;
   bool EnableModuleHeadersParsing;
-  bool ExperimentalCustomChecks;
 
   bool SelfContainedDiags = false;
 

clang-tools-extra/clang-tidy/ClangTidyForceLinker.h

@@ -54,13 +54,6 @@ extern volatile int CppCoreGuidelinesModuleAnchorSource;
 static int LLVM_ATTRIBUTE_UNUSED CppCoreGuidelinesModuleAnchorDestination =
     CppCoreGuidelinesModuleAnchorSource;
 
-#if CLANG_TIDY_ENABLE_QUERY_BASED_CUSTOM_CHECKS
-// This anchor is used to force the linker to link the CustomModule.
-extern volatile int CustomModuleAnchorSource;
-static int LLVM_ATTRIBUTE_UNUSED CustomModuleAnchorDestination =
-    CustomModuleAnchorSource;
-#endif
-
 // This anchor is used to force the linker to link the DarwinModule.
 extern volatile int DarwinModuleAnchorSource;
 static int LLVM_ATTRIBUTE_UNUSED DarwinModuleAnchorDestination =

clang-tools-extra/clang-tidy/ClangTidyModule.h

@@ -62,8 +62,6 @@ class ClangTidyCheckFactories {
                          });
   }
 
-  void eraseCheck(llvm::StringRef CheckName) { Factories.erase(CheckName); }
-
   /// Create instances of checks that are enabled.
   std::vector<std::unique_ptr<ClangTidyCheck>>
   createChecks(ClangTidyContext *Context) const;

clang-tools-extra/clang-tidy/ClangTidyOptions.cpp

@@ -8,10 +8,8 @@
 
 #include "ClangTidyOptions.h"
 #include "ClangTidyModuleRegistry.h"
-#include "clang/Basic/DiagnosticIDs.h"
 #include "clang/Basic/LLVM.h"
 #include "llvm/ADT/SmallString.h"
-#include "llvm/ADT/StringExtras.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/ErrorOr.h"
 #include "llvm/Support/MemoryBufferRef.h"
@@ -131,51 +129,6 @@ void yamlize(IO &IO, ClangTidyOptions::OptionMap &Val, bool,
   }
 }
 
-namespace {
-struct MultiLineString {
-  std::string &S;
-};
-} // namespace
-
-template <> struct BlockScalarTraits<MultiLineString> {
-  static void output(const MultiLineString &S, void *Ctxt, raw_ostream &OS) {
-    OS << S.S;
-  }
-  static StringRef input(StringRef Str, void *Ctxt, MultiLineString &S) {
-    S.S = Str;
-    return "";
-  }
-};
-
-template <> struct ScalarEnumerationTraits<clang::DiagnosticIDs::Level> {
-  static void enumeration(IO &IO, clang::DiagnosticIDs::Level &Level) {
-    IO.enumCase(Level, "Warning", clang::DiagnosticIDs::Level::Warning);
-    IO.enumCase(Level, "Note", clang::DiagnosticIDs::Level::Note);
-  }
-};
-template <> struct SequenceElementTraits<ClangTidyOptions::CustomCheckDiag> {
-  static const bool flow = false;
-};
-template <> struct MappingTraits<ClangTidyOptions::CustomCheckDiag> {
-  static void mapping(IO &IO, ClangTidyOptions::CustomCheckDiag &D) {
-    IO.mapRequired("BindName", D.BindName);
-    MultiLineString MLS{D.Message};
-    IO.mapRequired("Message", MLS);
-    IO.mapOptional("Level", D.Level);
-  }
-};
-template <> struct SequenceElementTraits<ClangTidyOptions::CustomCheckValue> {
-  static const bool flow = false;
-};
-template <> struct MappingTraits<ClangTidyOptions::CustomCheckValue> {
-  static void mapping(IO &IO, ClangTidyOptions::CustomCheckValue &V) {
-    IO.mapRequired("Name", V.Name);
-    MultiLineString MLS{V.Query};
-    IO.mapRequired("Query", MLS);
-    IO.mapRequired("Diagnostic", V.Diags);
-  }
-};
-
 struct ChecksVariant {
   std::optional<std::string> AsString;
   std::optional<std::vector<std::string>> AsVector;
@@ -231,7 +184,6 @@ template <> struct MappingTraits<ClangTidyOptions> {
     IO.mapOptional("InheritParentConfig", Options.InheritParentConfig);
     IO.mapOptional("UseColor", Options.UseColor);
     IO.mapOptional("SystemHeaders", Options.SystemHeaders);
-    IO.mapOptional("CustomChecks", Options.CustomChecks);
   }
 };
 
@@ -293,8 +245,7 @@ ClangTidyOptions &ClangTidyOptions::mergeWith(const ClangTidyOptions &Other,
   overrideValue(UseColor, Other.UseColor);
   mergeVectors(ExtraArgs, Other.ExtraArgs);
   mergeVectors(ExtraArgsBefore, Other.ExtraArgsBefore);
-  // FIXME: how to handle duplicate names check?
-  mergeVectors(CustomChecks, Other.CustomChecks);
+
   for (const auto &KeyValue : Other.CheckOptions) {
     CheckOptions.insert_or_assign(
         KeyValue.getKey(),

clang-tools-extra/clang-tidy/ClangTidyOptions.h

@@ -9,7 +9,6 @@
 #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CLANGTIDYOPTIONS_H
 #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CLANGTIDYOPTIONS_H
 
-#include "clang/Basic/DiagnosticIDs.h"
 #include "llvm/ADT/IntrusiveRefCntPtr.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringMap.h"
@@ -130,19 +129,6 @@ struct ClangTidyOptions {
   /// Key-value mapping used to store check-specific options.
   OptionMap CheckOptions;
 
-  struct CustomCheckDiag {
-    std::string BindName;
-    std::string Message;
-    std::optional<DiagnosticIDs::Level> Level;
-  };
-  struct CustomCheckValue {
-    std::string Name;
-    std::string Query;
-    llvm::SmallVector<CustomCheckDiag> Diags;
-  };
-  using CustomCheckValueList = llvm::SmallVector<CustomCheckValue>;
-  std::optional<CustomCheckValueList> CustomChecks;
-
   using ArgList = std::vector<std::string>;
 
   /// Add extra compilation arguments to the end of the list.