ClockMan created this revision.
Herald added subscribers: carlosgalvezp, arphaman, xazax.hun.
Herald added a reviewer: njames93.
Herald added a project: All.
ClockMan updated this revision to Diff 493024.
ClockMan added a comment.
ClockMan published this revision for review.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

clang-format


ClockMan added a comment.

Read for review.


Currently in clang-tidy 48 checks are registred more
than once under diffrent names. This causes issues
for end user, who by mistake could enable same check
multiple times. This commit provides command that
can be used by user to troubleshot this issue.

Read more: https://github.com/llvm/llvm-project/issues/49960


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D142816

Files:
  clang-tools-extra/clang-tidy/ClangTidy.cpp
  clang-tools-extra/clang-tidy/ClangTidy.h
  clang-tools-extra/clang-tidy/ClangTidyModule.cpp
  clang-tools-extra/clang-tidy/ClangTidyModule.h
  clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
  clang-tools-extra/docs/clang-tidy/index.rst
  clang-tools-extra/test/clang-tidy/infrastructure/list-unique-checks.cpp

Index: clang-tools-extra/test/clang-tidy/infrastructure/list-unique-checks.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/infrastructure/list-unique-checks.cpp
@@ -0,0 +1,4 @@
+// RUN: mkdir -p %T/clang-tidy/list-unique-checks/
+// RUN: echo '{Checks: "-*,google-readability-braces-around-statements,hicpp-braces-around-statements,readability-braces-around-statements"}' > %T/clang-tidy/.clang-tidy
+// RUN: cd %T/clang-tidy/list-unique-checks
+// RUN: clang-tidy -list-unique-checks | grep "^ *google-readability-braces-around-statements, hicpp-braces-around-statements, readability-braces-around-statements"
Index: clang-tools-extra/docs/clang-tidy/index.rst
===================================================================
--- clang-tools-extra/docs/clang-tidy/index.rst
+++ clang-tools-extra/docs/clang-tidy/index.rst
@@ -139,7 +139,7 @@
                                      When the value is empty, clang-tidy will
                                      attempt to find a file named .clang-tidy for
                                      each source file in its parent directories.
-    --config-file=<string>         - 
+    --config-file=<string>         -
                                     Specify the path of .clang-tidy or custom config file:
                                       e.g. --config-file=/some/path/myTidyConfigFile
                                     This option internally works exactly the same way as
@@ -217,6 +217,10 @@
     --list-checks                  -
                                      List all enabled checks and exit. Use with
                                      -checks=* to list all available checks.
+    --list-unique-checks           -
+                                     List all enabled checks and exit. Check
+                                     aliases are listed in the same line. Use
+                                     with -checks=* to list all available checks.
     -load=<plugin>                 -
                                      Load the dynamic object ``plugin``. This
                                      object should register new static analyzer
@@ -237,7 +241,7 @@
                                      format to stderr. When this option is passed,
                                      these per-TU profiles are instead stored as JSON.
     --system-headers               - Display the errors from system headers.
-    --use-color                    - 
+    --use-color                    -
                                     Use colors in diagnostics. If not set, colors
                                     will be used if the terminal connected to
                                     standard output supports colors.
Index: clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
===================================================================
--- clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
+++ clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
@@ -130,10 +130,10 @@
                                cl::init(false), cl::cat(ClangTidyCategory));
 
 static cl::opt<bool> FixNotes("fix-notes", cl::desc(R"(
-If a warning has no fix, but a single fix can 
-be found through an associated diagnostic note, 
-apply the fix. 
-Specifying this flag will implicitly enable the 
+If a warning has no fix, but a single fix can
+be found through an associated diagnostic note,
+apply the fix.
+Specifying this flag will implicitly enable the
 '--fix' flag.
 )"),
                               cl::init(false), cl::cat(ClangTidyCategory));
@@ -152,8 +152,8 @@
 This option overrides the 'FormatStyle` option in
 .clang-tidy file, if any.
 )"),
-                                   cl::init("none"),
-                                   cl::cat(ClangTidyCategory));
+                                        cl::init("none"),
+                                        cl::cat(ClangTidyCategory));
 
 static cl::opt<bool> ListChecks("list-checks", cl::desc(R"(
 List all enabled checks and exit. Use with
@@ -161,6 +161,14 @@
 )"),
                                 cl::init(false), cl::cat(ClangTidyCategory));
 
+static cl::opt<bool> ListUniqueChecks("list-unique-checks", cl::desc(R"(
+List all enabled checks and exit. Check
+aliases are listed in the same line. Use
+with -checks=* to list all available checks.
+)"),
+                                      cl::init(false),
+                                      cl::cat(ClangTidyCategory));
+
 static cl::opt<bool> ExplainConfig("explain-config", cl::desc(R"(
 For each enabled check explains, where it is
 enabled, i.e. in clang-tidy binary, command
@@ -238,8 +246,7 @@
 warnings treated as errors if the respective
 options are specified.
 )"),
-                           cl::init(false),
-                           cl::cat(ClangTidyCategory));
+                           cl::init(false), cl::cat(ClangTidyCategory));
 
 static cl::opt<std::string> VfsOverlay("vfsoverlay", cl::desc(R"(
 Overlay the virtual filesystem described by file
@@ -293,8 +300,8 @@
   }
 }
 
-static std::unique_ptr<ClangTidyOptionsProvider> createOptionsProvider(
-   llvm::IntrusiveRefCntPtr<vfs::FileSystem> FS) {
+static std::unique_ptr<ClangTidyOptionsProvider>
+createOptionsProvider(llvm::IntrusiveRefCntPtr<vfs::FileSystem> FS) {
   ClangTidyGlobalOptions GlobalOptions;
   if (std::error_code Err = parseLineFilter(LineFilter, GlobalOptions)) {
     llvm::errs() << "Invalid LineFilter: " << Err.message() << "\n\nUsage:\n";
@@ -542,6 +549,22 @@
     return 0;
   }
 
+  if (ListUniqueChecks) {
+    std::vector<std::string> EnabledUniqueChecks = getCheckNames(
+        EffectiveOptions, AllowEnablingAnalyzerAlphaCheckers, true);
+
+    if (EnabledUniqueChecks.empty()) {
+      llvm::errs() << "No checks enabled.\n";
+      return 1;
+    }
+
+    llvm::outs() << "Enabled checks:";
+    for (const auto &CheckName : EnabledUniqueChecks)
+      llvm::outs() << "\n    " << CheckName;
+    llvm::outs() << "\n\n";
+    return 0;
+  }
+
   if (DumpConfig) {
     EffectiveOptions.CheckOptions =
         getCheckOptions(EffectiveOptions, AllowEnablingAnalyzerAlphaCheckers);
Index: clang-tools-extra/clang-tidy/ClangTidyModule.h
===================================================================
--- clang-tools-extra/clang-tidy/ClangTidyModule.h
+++ clang-tools-extra/clang-tidy/ClangTidyModule.h
@@ -12,8 +12,11 @@
 #include "ClangTidyOptions.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Compiler.h"
+#include <cstdint>
 #include <functional>
 #include <memory>
+#include <string_view>
 
 namespace clang::tidy {
 
@@ -29,10 +32,10 @@
   using CheckFactory = std::function<std::unique_ptr<ClangTidyCheck>(
       llvm::StringRef Name, ClangTidyContext *Context)>;
 
-  /// Registers check \p Factory with name \p Name.
-  ///
-  /// For all checks that have default constructors, use \c registerCheck.
-  void registerCheckFactory(llvm::StringRef Name, CheckFactory Factory);
+  struct CheckInfo {
+    std::size_t TypeId;
+    CheckFactory Factory;
+  };
 
   /// Registers the \c CheckType with the name \p Name.
   ///
@@ -56,10 +59,12 @@
   /// };
   /// \endcode
   template <typename CheckType> void registerCheck(llvm::StringRef CheckName) {
-    registerCheckFactory(CheckName,
-                         [](llvm::StringRef Name, ClangTidyContext *Context) {
-                           return std::make_unique<CheckType>(Name, Context);
-                         });
+    registerCheckFactory(
+        CheckName,
+        [](llvm::StringRef Name, ClangTidyContext *Context) {
+          return std::make_unique<CheckType>(Name, Context);
+        },
+        getCheckTypeId<CheckType>());
   }
 
   /// Create instances of checks that are enabled.
@@ -70,12 +75,34 @@
   std::vector<std::unique_ptr<ClangTidyCheck>>
   createChecksForLanguage(ClangTidyContext *Context) const;
 
-  typedef llvm::StringMap<CheckFactory> FactoryMap;
+  typedef llvm::StringMap<CheckInfo> FactoryMap;
   FactoryMap::const_iterator begin() const { return Factories.begin(); }
   FactoryMap::const_iterator end() const { return Factories.end(); }
   bool empty() const { return Factories.empty(); }
 
 private:
+  struct UnusedTypeToValidateIfPrettyFunctionContainTemplateTypes {};
+
+  template <typename T,
+            typename Validate =
+                UnusedTypeToValidateIfPrettyFunctionContainTemplateTypes>
+  static std::size_t getCheckTypeId() {
+    std::string_view Name(LLVM_PRETTY_FUNCTION);
+    if (Name.find("UnusedTypeToValidateIfPrettyFunctionContainTemplateTypes") !=
+        std::string_view::npos) {
+      return std::hash<std::string_view>()(Name);
+    }
+
+    static unsigned IdValue = 0U;
+    return std::hash<void *>()(&IdValue);
+  }
+
+  /// Registers check \p Factory with name \p Name.
+  ///
+  /// For all checks that have default constructors, use \c registerCheck.
+  void registerCheckFactory(llvm::StringRef Name, CheckFactory Factory,
+                            unsigned TypeId);
+
   FactoryMap Factories;
 };
 
Index: clang-tools-extra/clang-tidy/ClangTidyModule.cpp
===================================================================
--- clang-tools-extra/clang-tidy/ClangTidyModule.cpp
+++ clang-tools-extra/clang-tidy/ClangTidyModule.cpp
@@ -16,8 +16,9 @@
 namespace clang::tidy {
 
 void ClangTidyCheckFactories::registerCheckFactory(StringRef Name,
-                                                   CheckFactory Factory) {
-  Factories.insert_or_assign(Name, std::move(Factory));
+                                                   CheckFactory Factory,
+                                                   unsigned TypeId) {
+  Factories.insert_or_assign(Name, CheckInfo{TypeId, std::move(Factory)});
 }
 
 std::vector<std::unique_ptr<ClangTidyCheck>>
@@ -25,7 +26,8 @@
   std::vector<std::unique_ptr<ClangTidyCheck>> Checks;
   for (const auto &Factory : Factories) {
     if (Context->isCheckEnabled(Factory.getKey()))
-      Checks.emplace_back(Factory.getValue()(Factory.getKey(), Context));
+      Checks.emplace_back(
+          Factory.getValue().Factory(Factory.getKey(), Context));
   }
   return Checks;
 }
@@ -39,7 +41,7 @@
     if (!Context->isCheckEnabled(Factory.getKey()))
       continue;
     std::unique_ptr<ClangTidyCheck> Check =
-        Factory.getValue()(Factory.getKey(), Context);
+        Factory.getValue().Factory(Factory.getKey(), Context);
     if (Check->isLanguageVersionSupported(LO))
       Checks.push_back(std::move(Check));
   }
Index: clang-tools-extra/clang-tidy/ClangTidy.h
===================================================================
--- clang-tools-extra/clang-tidy/ClangTidy.h
+++ clang-tools-extra/clang-tidy/ClangTidy.h
@@ -42,7 +42,7 @@
   createASTConsumer(clang::CompilerInstance &Compiler, StringRef File);
 
   /// Get the list of enabled checks.
-  std::vector<std::string> getCheckNames();
+  std::vector<std::string> getCheckNames(bool GroupByCheck = false);
 
   /// Get the union of options from all checks.
   ClangTidyOptions::OptionMap getCheckOptions();
@@ -56,7 +56,8 @@
 /// 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 AllowEnablingAnalyzerAlphaCheckers,
+                                       bool GroupByCheck = false);
 
 struct NamesAndOptions {
   llvm::StringSet<> Names;
Index: clang-tools-extra/clang-tidy/ClangTidy.cpp
===================================================================
--- clang-tools-extra/clang-tidy/ClangTidy.cpp
+++ clang-tools-extra/clang-tidy/ClangTidy.cpp
@@ -41,6 +41,8 @@
 #include "clang/Tooling/Tooling.h"
 #include "llvm/Support/Process.h"
 #include <algorithm>
+#include <map>
+#include <set>
 #include <utility>
 
 #if CLANG_TIDY_ENABLE_STATIC_ANALYZER
@@ -407,8 +409,8 @@
 
   std::unique_ptr<ClangTidyProfiling> Profiling;
   if (Context.getEnableProfiling()) {
-    Profiling = std::make_unique<ClangTidyProfiling>(
-        Context.getProfileStorageParams());
+    Profiling =
+        std::make_unique<ClangTidyProfiling>(Context.getProfileStorageParams());
     FinderOptions.CheckProfiling.emplace(Profiling->Records);
   }
 
@@ -419,8 +421,8 @@
   Preprocessor *ModuleExpanderPP = PP;
 
   if (Context.getLangOpts().Modules && OverlayFS != nullptr) {
-    auto ModuleExpander = std::make_unique<ExpandModularHeadersPPCallbacks>(
-        &Compiler, OverlayFS);
+    auto ModuleExpander =
+        std::make_unique<ExpandModularHeadersPPCallbacks>(&Compiler, OverlayFS);
     ModuleExpanderPP = ModuleExpander->getPreprocessor();
     PP->addPPCallbacks(std::move(ModuleExpander));
   }
@@ -454,11 +456,28 @@
       std::move(Checks));
 }
 
-std::vector<std::string> ClangTidyASTConsumerFactory::getCheckNames() {
+std::vector<std::string>
+ClangTidyASTConsumerFactory::getCheckNames(bool GroupByCheck) {
   std::vector<std::string> CheckNames;
+  std::map<std::size_t, std::set<std::string>> Grouped;
+
   for (const auto &CheckFactory : *CheckFactories) {
-    if (Context.isCheckEnabled(CheckFactory.getKey()))
-      CheckNames.emplace_back(CheckFactory.getKey());
+    if (Context.isCheckEnabled(CheckFactory.getKey())) {
+      if (GroupByCheck)
+        Grouped[CheckFactory.getValue().TypeId].emplace(CheckFactory.getKey());
+      else
+        CheckNames.emplace_back(CheckFactory.getKey());
+    }
+  }
+
+  if (GroupByCheck) {
+    for (const auto &Group : Grouped) {
+      std::string Checks;
+      for (const auto &Check : Group.second) {
+        Checks.append(", ").append(Check);
+      }
+      CheckNames.emplace_back(Checks.substr(2U));
+    }
   }
 
 #if CLANG_TIDY_ENABLE_STATIC_ANALYZER
@@ -480,15 +499,15 @@
   return Options;
 }
 
-std::vector<std::string>
-getCheckNames(const ClangTidyOptions &Options,
-              bool AllowEnablingAnalyzerAlphaCheckers) {
+std::vector<std::string> getCheckNames(const ClangTidyOptions &Options,
+                                       bool AllowEnablingAnalyzerAlphaCheckers,
+                                       bool GroupByCheck) {
   clang::tidy::ClangTidyContext Context(
       std::make_unique<DefaultOptionsProvider>(ClangTidyGlobalOptions(),
-                                                Options),
+                                               Options),
       AllowEnablingAnalyzerAlphaCheckers);
   ClangTidyASTConsumerFactory Factory(Context);
-  return Factory.getCheckNames();
+  return Factory.getCheckNames(GroupByCheck);
 }
 
 ClangTidyOptions::OptionMap
@@ -496,7 +515,7 @@
                 bool AllowEnablingAnalyzerAlphaCheckers) {
   clang::tidy::ClangTidyContext Context(
       std::make_unique<DefaultOptionsProvider>(ClangTidyGlobalOptions(),
-                                                Options),
+                                               Options),
       AllowEnablingAnalyzerAlphaCheckers);
   ClangTidyASTConsumerFactory Factory(Context);
   return Factory.getCheckOptions();
@@ -652,7 +671,7 @@
 
   Context.setOptionsCollector(&Result.Options);
   for (const auto &Factory : Factories) {
-    Factory.getValue()(Factory.getKey(), &Context);
+    Factory.getValue().Factory(Factory.getKey(), &Context);
   }
 
   return Result;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to