fix review

Author: HerrCai0907

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

@@ -54,7 +54,7 @@ LLVM_INSTANTIATE_REGISTRY(clang::tidy::ClangTidyModuleRegistry)
 namespace clang::tidy {
 
 namespace custom {
-extern void registerCustomChecks(ClangTidyOptions const &O,
+extern void registerCustomChecks(const ClangTidyOptions &O,
                                  ClangTidyCheckFactories &Factories);
 } // namespace custom
 

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

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

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

@@ -18,7 +18,6 @@
 #include "llvm/Support/MemoryBufferRef.h"
 #include "llvm/Support/VirtualFileSystem.h"
 #include <functional>
-#include <map>
 #include <optional>
 #include <string>
 #include <system_error>

clang-tools-extra/clang-tidy/custom/CustomTidyModule.cpp

@@ -18,13 +18,13 @@ class CustomModule : public ClangTidyModule {
 
 // We need to register the checks more flexibly than builtin modules. The checks
 // will changed dynamically when switching to different source file.
-extern void registerCustomChecks(ClangTidyOptions const &Options,
+extern void registerCustomChecks(const ClangTidyOptions &Options,
                                  ClangTidyCheckFactories &Factories) {
   static llvm::SmallSet<llvm::SmallString<32>, 8> CustomCheckNames{};
   if (!Options.CustomChecks.has_value() || Options.CustomChecks->empty())
     return;
-  for (llvm::SmallString<32> const &Name : CustomCheckNames)
-    Factories.erase(Name);
+  for (const llvm::SmallString<32> &Name : CustomCheckNames)
+    Factories.eraseCheck(Name);
   for (const ClangTidyOptions::CustomCheckValue &V :
        Options.CustomChecks.value()) {
     llvm::SmallString<32> Name = llvm::StringRef{"custom-" + V.Name};
@@ -45,6 +45,6 @@ static ClangTidyModuleRegistry::Add<custom::CustomModule>
 
 // This anchor is used to force the linker to link in the generated object file
 // and thus register the AlteraModule.
-volatile int CustomModuleAnchorSource = 0;
+volatile int CustomModuleAnchorSource = 0; // NOLINT (misc-use-internal-linkage)
 
 } // namespace clang::tidy

clang-tools-extra/clang-tidy/custom/QueryCheck.cpp

@@ -20,6 +20,12 @@ using namespace clang::ast_matchers;
 
 namespace clang::tidy::custom {
 
+static void emitConfigurationDiag(ClangTidyContext *Context, StringRef Message,
+                                  StringRef CheckName) {
+  Context->configurationDiag("%0 in '%1'", DiagnosticIDs::Warning)
+      << Message << CheckName;
+}
+
 static SmallVector<ast_matchers::dynamic::DynTypedMatcher>
 parseQuery(const ClangTidyOptions::CustomCheckValue &V,
            ClangTidyContext *Context) {
@@ -39,23 +45,52 @@ parseQuery(const ClangTidyOptions::CustomCheckValue &V,
       LetQuery.run(llvm::errs(), QS);
       break;
     }
+    case query::QK_NoOp: {
+      const auto &NoOpQuery = llvm::cast<query::NoOpQuery>(*Q);
+      NoOpQuery.run(llvm::errs(), QS);
+      break;
+    }
     case query::QK_Invalid: {
       const auto &InvalidQuery = llvm::cast<query::InvalidQuery>(*Q);
-      Context->configurationDiag(InvalidQuery.ErrStr, DiagnosticIDs::Error);
+      emitConfigurationDiag(Context, InvalidQuery.ErrStr, V.Name);
       return {};
     }
     // FIXME: TODO
-    case query::QK_File:
-    case query::QK_DisableOutputKind:
-    case query::QK_EnableOutputKind:
-    case query::QK_SetOutputKind:
-    case query::QK_SetTraversalKind:
-    case query::QK_Help:
-    case query::QK_NoOp:
-    case query::QK_Quit:
+    case query::QK_File: {
+      emitConfigurationDiag(Context, "unsupported query kind 'File'", V.Name);
+      return {};
+    }
+    case query::QK_DisableOutputKind: {
+      emitConfigurationDiag(
+          Context, "unsupported query kind 'DisableOutputKind'", V.Name);
+      return {};
+    }
+    case query::QK_EnableOutputKind: {
+      emitConfigurationDiag(
+          Context, "unsupported query kind 'EnableOutputKind'", V.Name);
+      return {};
+    }
+    case query::QK_SetOutputKind: {
+      emitConfigurationDiag(Context, "unsupported query kind 'SetOutputKind'",
+                            V.Name);
+      return {};
+    }
+    case query::QK_SetTraversalKind: {
+      emitConfigurationDiag(
+          Context, "unsupported query kind 'SetTraversalKind'", V.Name);
+      return {};
+    }
     case query::QK_SetBool: {
-      Context->configurationDiag("unsupported query kind",
-                                 DiagnosticIDs::Error);
+      emitConfigurationDiag(Context, "unsupported query kind 'SetBool'",
+                            V.Name);
+      return {};
+    }
+    case query::QK_Help: {
+      emitConfigurationDiag(Context, "unsupported query kind 'Help'", V.Name);
+      return {};
+    }
+    case query::QK_Quit: {
+      emitConfigurationDiag(Context, "unsupported query kind 'Quit'", V.Name);
       return {};
     }
     }
@@ -89,19 +124,19 @@ void QueryCheck::registerMatchers(MatchFinder *Finder) {
 }
 
 void QueryCheck::check(const MatchFinder::MatchResult &Result) {
-  auto Emit = [this](DiagMaps const &DiagMaps, std::string const &BindName,
-                     DynTypedNode const &Node, DiagnosticIDs::Level Level) {
-    if (!DiagMaps.contains(Level))
+  auto Emit = [this](const DiagMaps &DiagMaps, const std::string &BindName,
+                     const DynTypedNode &Node, DiagnosticIDs::Level Level) {
+    DiagMaps::const_iterator DiagMapIt = DiagMaps.find(Level);
+    if (DiagMapIt == DiagMaps.end())
       return;
-    auto &DiagMap = DiagMaps.at(Level);
-    if (!DiagMap.contains(BindName))
+    const BindNameMapToDiagMessage &BindNameMap = DiagMapIt->second;
+    BindNameMapToDiagMessage::const_iterator BindNameMapIt =
+        BindNameMap.find(BindName);
+    if (BindNameMapIt == BindNameMap.end())
       return;
-    for (const std::string &Message : DiagMap.at(BindName)) {
+    for (const std::string &Message : BindNameMapIt->second)
       diag(Node.getSourceRange().getBegin(), Message, Level);
-    }
   };
-  for (const auto &[Name, Node] : Result.Nodes.getMap())
-    Emit(Diags, Name, Node, DiagnosticIDs::Error);
   for (const auto &[Name, Node] : Result.Nodes.getMap())
     Emit(Diags, Name, Node, DiagnosticIDs::Warning);
   // place Note last, otherwise it will not be emitted

clang-tools-extra/clang-tidy/custom/QueryCheck.h

@@ -29,9 +29,10 @@ class QueryCheck : public ClangTidyCheck {
 
 private:
   llvm::SmallVector<ast_matchers::dynamic::DynTypedMatcher> Matchers{};
+  using BindNameMapToDiagMessage =
+      llvm::StringMap<llvm::SmallVector<std::string>>;
   using DiagMaps =
-      llvm::DenseMap<DiagnosticIDs::Level,
-                     llvm::StringMap<llvm::SmallVector<std::string>>>;
+      llvm::DenseMap<DiagnosticIDs::Level, BindNameMapToDiagMessage>;
   DiagMaps Diags;
 };
 

clang-tools-extra/docs/ReleaseNotes.rst

@@ -104,11 +104,6 @@ Improvements to clang-tidy
   clauses. Added a ``--match-partial-fixes`` option to keep previous behavior on
   specific tests. This may break tests for users with custom out-of-tree checks
   who use :program:`check_clang_tidy.py` as-is.
-- :program:`clang-tidy` no longer processes declarations from system headers
-  by default, greatly improving performance. This behavior is disabled if the
-  `SystemHeaders` option is enabled.
-  Note: this may lead to false negatives; downstream users may need to adjust
-  their checks to preserve existing behavior.
 - :program:`clang-tidy` now supports query based custom checks by `CustomChecks`
   configuration option.
   :doc:`Query Based Custom Check Document <clang-tidy/QueryBasedCustomChecks>`

clang-tools-extra/test/clang-tidy/checkers/custom/Inputs/incorrect-clang-tidy.yml

@@ -1,5 +1,5 @@
 CustomChecks:
-  - Name: test-let-bind-invalid
+  - Name: test-let-bind-invalid-1
     Query: |
       let expr varDecl(isStaticStorageClass()).bind("vd")
       match expr
@@ -8,6 +8,13 @@ CustomChecks:
       - BindName: vd
         Message: find static variable
         Level: Warning
+  - Name: test-let-bind-invalid-2
+    Query: |
+      match varDeclInvalid(isStaticStorageClass()).bind("vd")
+    Diagnostic:
+      - BindName: vd
+        Message: find static variable
+        Level: Warning
   - Name: test-let-bind-valid
     Query: |
       let expr varDecl(isStaticStorageClass()).bind("vd")

clang-tools-extra/test/clang-tidy/checkers/custom/query-incorrect-query.cpp

@@ -1,6 +1,7 @@
-// RUN: not %check_clang_tidy %s custom-* %t --config-file=%S/Inputs/incorrect-clang-tidy.yml
+// RUN: %check_clang_tidy %s custom-* %t --config-file=%S/Inputs/incorrect-clang-tidy.yml
 
-// CHECK-MESSAGES: error: unsupported query kind [clang-tidy-config]
+// CHECK-MESSAGES: warning: 1:1: Matcher not found: varDeclInvalid in 'test-let-bind-invalid-2' [clang-tidy-config]
+// CHECK-MESSAGES: warning: unsupported query kind 'SetOutputKind' in 'test-let-bind-invalid-1' [clang-tidy-config]
 
 static int S;
 // CHECK-MESSAGES: [[@LINE-1]]:1: warning: find static variable [custom-test-let-bind-valid]