Skip to content

Conversation

@HerrCai0907
Copy link
Contributor

No description provided.

@llvmbot
Copy link
Member

llvmbot commented Dec 20, 2024

@llvm/pr-subscribers-clang-tidy

Author: Congcong Cai (HerrCai0907)

Changes

Full diff: https://github.com/llvm/llvm-project/pull/120768.diff

3 Files Affected:

  • (modified) clang-tools-extra/clang-tidy/ClangTidy.cpp (+4-4)
  • (modified) clang-tools-extra/clang-tidy/ClangTidy.h (+3-3)
  • (modified) clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp (+21-15)
diff --git a/clang-tools-extra/clang-tidy/ClangTidy.cpp b/clang-tools-extra/clang-tidy/ClangTidy.cpp
index 9c8c93c5d16c72..959b11777e88d4 100644
--- a/clang-tools-extra/clang-tidy/ClangTidy.cpp
+++ b/clang-tools-extra/clang-tidy/ClangTidy.cpp
@@ -646,9 +646,9 @@ void exportReplacements(const llvm::StringRef MainFilePath,
   YAML << TUD;
 }
 
-NamesAndOptions
+ChecksAndOptions
 getAllChecksAndOptions(bool AllowEnablingAnalyzerAlphaCheckers) {
-  NamesAndOptions Result;
+  ChecksAndOptions Result;
   ClangTidyOptions Opts;
   Opts.Checks = "*";
   clang::tidy::ClangTidyContext Context(
@@ -661,7 +661,7 @@ getAllChecksAndOptions(bool AllowEnablingAnalyzerAlphaCheckers) {
   }
 
   for (const auto &Factory : Factories)
-    Result.Names.insert(Factory.getKey());
+    Result.Checks.insert(Factory.getKey());
 
 #if CLANG_TIDY_ENABLE_STATIC_ANALYZER
   SmallString<64> Buffer(AnalyzerCheckNamePrefix);
@@ -670,7 +670,7 @@ getAllChecksAndOptions(bool AllowEnablingAnalyzerAlphaCheckers) {
            AllowEnablingAnalyzerAlphaCheckers)) {
     Buffer.truncate(DefSize);
     Buffer.append(AnalyzerCheck);
-    Result.Names.insert(Buffer);
+    Result.Checks.insert(Buffer);
   }
   for (std::string OptionName : {
 #define GET_CHECKER_OPTIONS
diff --git a/clang-tools-extra/clang-tidy/ClangTidy.h b/clang-tools-extra/clang-tidy/ClangTidy.h
index 51d9e226c79465..4ffd49f6ebf50f 100644
--- a/clang-tools-extra/clang-tidy/ClangTidy.h
+++ b/clang-tools-extra/clang-tidy/ClangTidy.h
@@ -58,12 +58,12 @@ class ClangTidyASTConsumerFactory {
 std::vector<std::string> getCheckNames(const ClangTidyOptions &Options,
                                        bool AllowEnablingAnalyzerAlphaCheckers);
 
-struct NamesAndOptions {
-  llvm::StringSet<> Names;
+struct ChecksAndOptions {
+  llvm::StringSet<> Checks;
   llvm::StringSet<> Options;
 };
 
-NamesAndOptions
+ChecksAndOptions
 getAllChecksAndOptions(bool AllowEnablingAnalyzerAlphaCheckers = true);
 
 /// Returns the effective check-specific options.
diff --git a/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp b/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
index d42dafa8ffc362..b8d843cba71330 100644
--- a/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
+++ b/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
@@ -526,6 +526,24 @@ static bool verifyFileExtensions(
   return AnyInvalid;
 }
 
+static bool verifyOptions(const llvm::StringSet<> &ValidOptions,
+                          const ClangTidyOptions::OptionMap &OptionMap,
+                          StringRef Source) {
+  bool AnyInvalid = false;
+  for (auto Key : OptionMap.keys()) {
+    if (ValidOptions.contains(Key))
+      continue;
+    AnyInvalid = true;
+    auto &Output = llvm::WithColor::warning(llvm::errs(), Source)
+                   << "unknown check option '" << Key << '\'';
+    llvm::StringRef Closest = closest(Key, ValidOptions);
+    if (!Closest.empty())
+      Output << "; did you mean '" << Closest << '\'';
+    Output << VerifyConfigWarningEnd;
+  }
+  return AnyInvalid;
+}
+
 static SmallString<256> makeAbsolute(llvm::StringRef Input) {
   if (Input.empty())
     return {};
@@ -629,29 +647,17 @@ int clangTidyMain(int argc, const char **argv) {
   if (VerifyConfig) {
     std::vector<ClangTidyOptionsProvider::OptionsSource> RawOptions =
         OptionsProvider->getRawOptions(FileName);
-    NamesAndOptions Valid =
+    ChecksAndOptions Valid =
         getAllChecksAndOptions(AllowEnablingAnalyzerAlphaCheckers);
     bool AnyInvalid = false;
     for (const auto &[Opts, Source] : RawOptions) {
       if (Opts.Checks)
-        AnyInvalid |= verifyChecks(Valid.Names, *Opts.Checks, Source);
-
+        AnyInvalid |= verifyChecks(Valid.Checks, *Opts.Checks, Source);
       if (Opts.HeaderFileExtensions && Opts.ImplementationFileExtensions)
         AnyInvalid |=
             verifyFileExtensions(*Opts.HeaderFileExtensions,
                                  *Opts.ImplementationFileExtensions, Source);
-
-      for (auto Key : Opts.CheckOptions.keys()) {
-        if (Valid.Options.contains(Key))
-          continue;
-        AnyInvalid = true;
-        auto &Output = llvm::WithColor::warning(llvm::errs(), Source)
-                       << "unknown check option '" << Key << '\'';
-        llvm::StringRef Closest = closest(Key, Valid.Options);
-        if (!Closest.empty())
-          Output << "; did you mean '" << Closest << '\'';
-        Output << VerifyConfigWarningEnd;
-      }
+      AnyInvalid |= verifyOptions(Valid.Options, Opts.CheckOptions, Source);
     }
     if (AnyInvalid)
       return 1;

@llvmbot
Copy link
Member

llvmbot commented Dec 20, 2024

@llvm/pr-subscribers-clang-tools-extra

Author: Congcong Cai (HerrCai0907)

Changes

Full diff: https://github.com/llvm/llvm-project/pull/120768.diff

3 Files Affected:

  • (modified) clang-tools-extra/clang-tidy/ClangTidy.cpp (+4-4)
  • (modified) clang-tools-extra/clang-tidy/ClangTidy.h (+3-3)
  • (modified) clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp (+21-15)
diff --git a/clang-tools-extra/clang-tidy/ClangTidy.cpp b/clang-tools-extra/clang-tidy/ClangTidy.cpp
index 9c8c93c5d16c72..959b11777e88d4 100644
--- a/clang-tools-extra/clang-tidy/ClangTidy.cpp
+++ b/clang-tools-extra/clang-tidy/ClangTidy.cpp
@@ -646,9 +646,9 @@ void exportReplacements(const llvm::StringRef MainFilePath,
   YAML << TUD;
 }
 
-NamesAndOptions
+ChecksAndOptions
 getAllChecksAndOptions(bool AllowEnablingAnalyzerAlphaCheckers) {
-  NamesAndOptions Result;
+  ChecksAndOptions Result;
   ClangTidyOptions Opts;
   Opts.Checks = "*";
   clang::tidy::ClangTidyContext Context(
@@ -661,7 +661,7 @@ getAllChecksAndOptions(bool AllowEnablingAnalyzerAlphaCheckers) {
   }
 
   for (const auto &Factory : Factories)
-    Result.Names.insert(Factory.getKey());
+    Result.Checks.insert(Factory.getKey());
 
 #if CLANG_TIDY_ENABLE_STATIC_ANALYZER
   SmallString<64> Buffer(AnalyzerCheckNamePrefix);
@@ -670,7 +670,7 @@ getAllChecksAndOptions(bool AllowEnablingAnalyzerAlphaCheckers) {
            AllowEnablingAnalyzerAlphaCheckers)) {
     Buffer.truncate(DefSize);
     Buffer.append(AnalyzerCheck);
-    Result.Names.insert(Buffer);
+    Result.Checks.insert(Buffer);
   }
   for (std::string OptionName : {
 #define GET_CHECKER_OPTIONS
diff --git a/clang-tools-extra/clang-tidy/ClangTidy.h b/clang-tools-extra/clang-tidy/ClangTidy.h
index 51d9e226c79465..4ffd49f6ebf50f 100644
--- a/clang-tools-extra/clang-tidy/ClangTidy.h
+++ b/clang-tools-extra/clang-tidy/ClangTidy.h
@@ -58,12 +58,12 @@ class ClangTidyASTConsumerFactory {
 std::vector<std::string> getCheckNames(const ClangTidyOptions &Options,
                                        bool AllowEnablingAnalyzerAlphaCheckers);
 
-struct NamesAndOptions {
-  llvm::StringSet<> Names;
+struct ChecksAndOptions {
+  llvm::StringSet<> Checks;
   llvm::StringSet<> Options;
 };
 
-NamesAndOptions
+ChecksAndOptions
 getAllChecksAndOptions(bool AllowEnablingAnalyzerAlphaCheckers = true);
 
 /// Returns the effective check-specific options.
diff --git a/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp b/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
index d42dafa8ffc362..b8d843cba71330 100644
--- a/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
+++ b/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
@@ -526,6 +526,24 @@ static bool verifyFileExtensions(
   return AnyInvalid;
 }
 
+static bool verifyOptions(const llvm::StringSet<> &ValidOptions,
+                          const ClangTidyOptions::OptionMap &OptionMap,
+                          StringRef Source) {
+  bool AnyInvalid = false;
+  for (auto Key : OptionMap.keys()) {
+    if (ValidOptions.contains(Key))
+      continue;
+    AnyInvalid = true;
+    auto &Output = llvm::WithColor::warning(llvm::errs(), Source)
+                   << "unknown check option '" << Key << '\'';
+    llvm::StringRef Closest = closest(Key, ValidOptions);
+    if (!Closest.empty())
+      Output << "; did you mean '" << Closest << '\'';
+    Output << VerifyConfigWarningEnd;
+  }
+  return AnyInvalid;
+}
+
 static SmallString<256> makeAbsolute(llvm::StringRef Input) {
   if (Input.empty())
     return {};
@@ -629,29 +647,17 @@ int clangTidyMain(int argc, const char **argv) {
   if (VerifyConfig) {
     std::vector<ClangTidyOptionsProvider::OptionsSource> RawOptions =
         OptionsProvider->getRawOptions(FileName);
-    NamesAndOptions Valid =
+    ChecksAndOptions Valid =
         getAllChecksAndOptions(AllowEnablingAnalyzerAlphaCheckers);
     bool AnyInvalid = false;
     for (const auto &[Opts, Source] : RawOptions) {
       if (Opts.Checks)
-        AnyInvalid |= verifyChecks(Valid.Names, *Opts.Checks, Source);
-
+        AnyInvalid |= verifyChecks(Valid.Checks, *Opts.Checks, Source);
       if (Opts.HeaderFileExtensions && Opts.ImplementationFileExtensions)
         AnyInvalid |=
             verifyFileExtensions(*Opts.HeaderFileExtensions,
                                  *Opts.ImplementationFileExtensions, Source);
-
-      for (auto Key : Opts.CheckOptions.keys()) {
-        if (Valid.Options.contains(Key))
-          continue;
-        AnyInvalid = true;
-        auto &Output = llvm::WithColor::warning(llvm::errs(), Source)
-                       << "unknown check option '" << Key << '\'';
-        llvm::StringRef Closest = closest(Key, Valid.Options);
-        if (!Closest.empty())
-          Output << "; did you mean '" << Closest << '\'';
-        Output << VerifyConfigWarningEnd;
-      }
+      AnyInvalid |= verifyOptions(Valid.Options, Opts.CheckOptions, Source);
     }
     if (AnyInvalid)
       return 1;

@HerrCai0907 HerrCai0907 changed the title [clang-tidy][NFC] extract options verify to sperately function [clang-tidy][NFC] extract options verify to separately function Dec 20, 2024
Copy link
Contributor

@5chmidti 5chmidti left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@HerrCai0907 HerrCai0907 merged commit e5de2a2 into llvm:main Dec 20, 2024
11 checks passed
@HerrCai0907 HerrCai0907 deleted the fix/refactor-option-verify branch December 20, 2024 23:07
qiaojbao pushed a commit to GPUOpen-Drivers/llvm-project that referenced this pull request Feb 7, 2025
Local branch amd-gfx 179d2f9 Merged main:d2b8acc10464 into amd-gfx:f8992209d7db
Remote branch main e5de2a2 [clang-tidy][NFC] extract options verify to separately function (llvm#120768)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants