Skip to content

Conversation

@localspook
Copy link
Contributor

82289aa refactored this function to not need this parameter.

@llvmbot
Copy link
Member

llvmbot commented Oct 20, 2025

@llvm/pr-subscribers-clang-tidy

Author: Victor Chernyakin (localspook)

Changes

82289aa refactored this function to not need this parameter.


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

1 Files Affected:

  • (modified) clang-tools-extra/clang-tidy/ClangTidyCheck.cpp (+4-6)
diff --git a/clang-tools-extra/clang-tidy/ClangTidyCheck.cpp b/clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
index d36cc3e6e23db..6e0c252a80bf8 100644
--- a/clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
+++ b/clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
@@ -90,12 +90,10 @@ ClangTidyCheck::OptionsView::getLocalOrGlobal(StringRef LocalName) const {
   return std::nullopt;
 }
 
-static std::optional<bool> getAsBool(StringRef Value,
-                                     const llvm::Twine &LookupName) {
-
+static std::optional<bool> getAsBool(StringRef Value) {
   if (std::optional<bool> Parsed = llvm::yaml::parseBool(Value))
     return Parsed;
-  // To maintain backwards compatability, we support parsing numbers as
+  // To maintain backwards compatibility, we support parsing numbers as
   // booleans, even though its not supported in YAML.
   long long Number = 0;
   if (!Value.getAsInteger(10, Number))
@@ -107,7 +105,7 @@ template <>
 std::optional<bool>
 ClangTidyCheck::OptionsView::get<bool>(StringRef LocalName) const {
   if (std::optional<StringRef> ValueOr = get(LocalName)) {
-    if (auto Result = getAsBool(*ValueOr, NamePrefix + LocalName))
+    if (auto Result = getAsBool(*ValueOr))
       return Result;
     diagnoseBadBooleanOption(NamePrefix + LocalName, *ValueOr);
   }
@@ -119,7 +117,7 @@ std::optional<bool>
 ClangTidyCheck::OptionsView::getLocalOrGlobal<bool>(StringRef LocalName) const {
   auto Iter = findPriorityOption(CheckOptions, NamePrefix, LocalName, Context);
   if (Iter != CheckOptions.end()) {
-    if (auto Result = getAsBool(Iter->getValue().Value, Iter->getKey()))
+    if (auto Result = getAsBool(Iter->getValue().Value))
       return Result;
     diagnoseBadBooleanOption(Iter->getKey(), Iter->getValue().Value);
   }

@llvmbot
Copy link
Member

llvmbot commented Oct 20, 2025

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

Author: Victor Chernyakin (localspook)

Changes

82289aa refactored this function to not need this parameter.


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

1 Files Affected:

  • (modified) clang-tools-extra/clang-tidy/ClangTidyCheck.cpp (+4-6)
diff --git a/clang-tools-extra/clang-tidy/ClangTidyCheck.cpp b/clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
index d36cc3e6e23db..6e0c252a80bf8 100644
--- a/clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
+++ b/clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
@@ -90,12 +90,10 @@ ClangTidyCheck::OptionsView::getLocalOrGlobal(StringRef LocalName) const {
   return std::nullopt;
 }
 
-static std::optional<bool> getAsBool(StringRef Value,
-                                     const llvm::Twine &LookupName) {
-
+static std::optional<bool> getAsBool(StringRef Value) {
   if (std::optional<bool> Parsed = llvm::yaml::parseBool(Value))
     return Parsed;
-  // To maintain backwards compatability, we support parsing numbers as
+  // To maintain backwards compatibility, we support parsing numbers as
   // booleans, even though its not supported in YAML.
   long long Number = 0;
   if (!Value.getAsInteger(10, Number))
@@ -107,7 +105,7 @@ template <>
 std::optional<bool>
 ClangTidyCheck::OptionsView::get<bool>(StringRef LocalName) const {
   if (std::optional<StringRef> ValueOr = get(LocalName)) {
-    if (auto Result = getAsBool(*ValueOr, NamePrefix + LocalName))
+    if (auto Result = getAsBool(*ValueOr))
       return Result;
     diagnoseBadBooleanOption(NamePrefix + LocalName, *ValueOr);
   }
@@ -119,7 +117,7 @@ std::optional<bool>
 ClangTidyCheck::OptionsView::getLocalOrGlobal<bool>(StringRef LocalName) const {
   auto Iter = findPriorityOption(CheckOptions, NamePrefix, LocalName, Context);
   if (Iter != CheckOptions.end()) {
-    if (auto Result = getAsBool(Iter->getValue().Value, Iter->getKey()))
+    if (auto Result = getAsBool(Iter->getValue().Value))
       return Result;
     diagnoseBadBooleanOption(Iter->getKey(), Iter->getValue().Value);
   }

Copy link
Contributor

@vbvictor vbvictor left a comment

Choose a reason for hiding this comment

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

Could you elaborate why it isn't needed anymore?

@localspook
Copy link
Contributor Author

localspook commented Oct 22, 2025

getAsBool used to return an llvm::Expected, and it needed this parameter to build a nice error message when it failed. It was refactored to return a std::optional, which is just std::nullopt on error. The error message is now built in the functions that call getAsBool.

Copy link
Contributor

@vbvictor vbvictor left a comment

Choose a reason for hiding this comment

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

LGTM

@localspook localspook merged commit 3b8f63d into llvm:main Oct 27, 2025
14 checks passed
@localspook localspook deleted the unused-parameter branch October 27, 2025 04:36
varun-r-mallya pushed a commit to varun-r-mallya/llvm-project that referenced this pull request Oct 27, 2025
82289aa refactored this function to not need this parameter.
dvbuka pushed a commit to dvbuka/llvm-project that referenced this pull request Oct 27, 2025
82289aa refactored this function to not need this parameter.
Lukacma pushed a commit to Lukacma/llvm-project that referenced this pull request Oct 29, 2025
82289aa refactored this function to not need this parameter.
aokblast pushed a commit to aokblast/llvm-project that referenced this pull request Oct 30, 2025
82289aa refactored this function to not need this parameter.
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