[clang-tools-extra] Fix #68492: point to the correct const location (PR #69103)

via cfe-commits cfe-commits at lists.llvm.org
Mon Oct 16 15:20:05 PDT 2023


https://github.com/Da-Viper updated https://github.com/llvm/llvm-project/pull/69103

>From 354a8e4034afd82e6ea854848a86b9011e26269b Mon Sep 17 00:00:00 2001
From: Ezike Ebuka <yerimyah1 at gmail.com>
Date: Fri, 13 Oct 2023 19:27:15 +0100
Subject: [PATCH 1/3] Fix #68492: point to the correct const location

---
 .../readability/AvoidConstParamsInDecls.cpp   | 43 ++++++++++++-------
 .../avoid-const-params-in-decls.cpp           | 14 +++---
 2 files changed, 34 insertions(+), 23 deletions(-)

diff --git a/clang-tools-extra/clang-tidy/readability/AvoidConstParamsInDecls.cpp b/clang-tools-extra/clang-tidy/readability/AvoidConstParamsInDecls.cpp
index 6476f1d7fdf2b81..24cbbd8bc60a2b5 100644
--- a/clang-tools-extra/clang-tidy/readability/AvoidConstParamsInDecls.cpp
+++ b/clang-tools-extra/clang-tidy/readability/AvoidConstParamsInDecls.cpp
@@ -21,6 +21,24 @@ SourceRange getTypeRange(const ParmVarDecl &Param) {
   return {Param.getBeginLoc(), Param.getLocation().getLocWithOffset(-1)};
 }
 
+// Finds the location of the qualifying `const` token in the `ParmValDecl`'s
+// return type. Returns `std::nullopt` when the parm type is not
+// `const`-qualified like when the type is an alias or a macro.
+static std::optional<Token>
+findConstToRemove(const ParmVarDecl &Param,
+                  const MatchFinder::MatchResult &Result) {
+
+  CharSourceRange FileRange = Lexer::makeFileCharRange(
+      CharSourceRange::getTokenRange(getTypeRange(Param)),
+      *Result.SourceManager, Result.Context->getLangOpts());
+
+  if (FileRange.isInvalid())
+    return std::nullopt;
+
+  return tidy::utils::lexer::getQualifyingToken(
+      tok::kw_const, FileRange, *Result.Context, *Result.SourceManager);
+}
+
 } // namespace
 
 void AvoidConstParamsInDecls::storeOptions(ClangTidyOptions::OptionMap &Opts) {
@@ -30,11 +48,10 @@ void AvoidConstParamsInDecls::storeOptions(ClangTidyOptions::OptionMap &Opts) {
 void AvoidConstParamsInDecls::registerMatchers(MatchFinder *Finder) {
   const auto ConstParamDecl =
       parmVarDecl(hasType(qualType(isConstQualified()))).bind("param");
-  Finder->addMatcher(
-      functionDecl(unless(isDefinition()),
-                   has(typeLoc(forEach(ConstParamDecl))))
-          .bind("func"),
-      this);
+  Finder->addMatcher(functionDecl(unless(isDefinition()),
+                                  has(typeLoc(forEach(ConstParamDecl))))
+                         .bind("func"),
+                     this);
 }
 
 void AvoidConstParamsInDecls::check(const MatchFinder::MatchResult &Result) {
@@ -50,7 +67,10 @@ void AvoidConstParamsInDecls::check(const MatchFinder::MatchResult &Result) {
     return;
   }
 
-  auto Diag = diag(Param->getBeginLoc(),
+  const auto Tok = findConstToRemove(*Param, Result);
+  const auto ConstLocation = Tok ? Tok->getLocation() : Param->getBeginLoc();
+
+  auto Diag = diag(ConstLocation,
                    "parameter %0 is const-qualified in the function "
                    "declaration; const-qualification of parameters only has an "
                    "effect in function definitions");
@@ -70,18 +90,9 @@ void AvoidConstParamsInDecls::check(const MatchFinder::MatchResult &Result) {
     // from a macro.
     return;
   }
-
-  CharSourceRange FileRange = Lexer::makeFileCharRange(
-      CharSourceRange::getTokenRange(getTypeRange(*Param)),
-      *Result.SourceManager, getLangOpts());
-
-  if (!FileRange.isValid())
-    return;
-
-  auto Tok = tidy::utils::lexer::getQualifyingToken(
-      tok::kw_const, FileRange, *Result.Context, *Result.SourceManager);
   if (!Tok)
     return;
+
   Diag << FixItHint::CreateRemoval(
       CharSourceRange::getTokenRange(Tok->getLocation(), Tok->getLocation()));
 }
diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/avoid-const-params-in-decls.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/avoid-const-params-in-decls.cpp
index d521fd238b7d521..bc098efe3044a5d 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/readability/avoid-const-params-in-decls.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/readability/avoid-const-params-in-decls.cpp
@@ -9,15 +9,15 @@ void F1(const int i);
 // CHECK-FIXES: void F1(int i);
 
 void F2(const int *const i);
-// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: parameter 'i' is const-qualified
+// CHECK-MESSAGES: :[[@LINE-1]]:20: warning: parameter 'i' is const-qualified
 // CHECK-FIXES: void F2(const int *i);
 
 void F3(int const i);
-// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: parameter 'i' is const-qualified
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: parameter 'i' is const-qualified
 // CHECK-FIXES: void F3(int i);
 
 void F4(alias_type const i);
-// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: parameter 'i' is const-qualified
+// CHECK-MESSAGES: :[[@LINE-1]]:20: warning: parameter 'i' is const-qualified
 // CHECK-FIXES: void F4(alias_type i);
 
 void F5(const int);
@@ -25,7 +25,7 @@ void F5(const int);
 // CHECK-FIXES: void F5(int);
 
 void F6(const int *const);
-// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: parameter 1 is const-qualified
+// CHECK-MESSAGES: :[[@LINE-1]]:20: warning: parameter 1 is const-qualified
 // CHECK-FIXES: void F6(const int *);
 
 void F7(int, const int);
@@ -42,7 +42,7 @@ void F9(const int long_name);
 // CHECK-FIXES: void F9(int long_name);
 
 void F10(const int *const *const long_name);
-// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: parameter 'long_name'
+// CHECK-MESSAGES: :[[@LINE-1]]:28: warning: parameter 'long_name'
 // CHECK-FIXES: void F10(const int *const *long_name);
 
 void F11(const unsigned int /*v*/);
@@ -71,11 +71,11 @@ void F15(const A<const int> Named);
 // CHECK-FIXES: void F15(A<const int> Named);
 
 void F16(const A<const int> *const);
-// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: parameter 1 is const-qualified
+// CHECK-MESSAGES: :[[@LINE-1]]:30: warning: parameter 1 is const-qualified
 // CHECK-FIXES: void F16(const A<const int> *);
 
 void F17(const A<const int> *const Named);
-// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: parameter 'Named' is const-qualified
+// CHECK-MESSAGES: :[[@LINE-1]]:30: warning: parameter 'Named' is const-qualified
 // CHECK-FIXES: void F17(const A<const int> *Named);
 
 struct Foo {

>From 3dffafe6455c4376f1043f5e72d3a5b8ce820f42 Mon Sep 17 00:00:00 2001
From: Ezike Ebuka <yerimyah1 at gmail.com>
Date: Sun, 15 Oct 2023 16:25:17 +0100
Subject: [PATCH 2/3] Add: Update ReleaseNotes.rst with the changes made

---
 clang-tools-extra/docs/ReleaseNotes.rst | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index c1b926b296b055a..c64df96068241a8 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -293,6 +293,10 @@ Changes in existing checks
   <clang-tidy/checks/performance/noexcept-swap>` check to enforce a stricter
   match with the swap function signature, eliminating false-positives.
 
+- Improved :doc:`readability-const-params-in-decls
+  <clang-tidy/checks/readability/const-params-in-decls>` place the hint check under
+  the const token instead of the beginning of parameter declaration.
+
 - Improved :doc:`readability-braces-around-statements
   <clang-tidy/checks/readability/braces-around-statements>` check to
   ignore false-positive for ``if constexpr`` in lambda expression.

>From aa4537d6d78ee0d4b3694d8aa904cab4b56113c0 Mon Sep 17 00:00:00 2001
From: Ezike Ebuka <yerimyah1 at gmail.com>
Date: Mon, 16 Oct 2023 23:19:29 +0100
Subject: [PATCH 3/3] Update: change the location in the release note

---
 clang-tools-extra/docs/ReleaseNotes.rst | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index c64df96068241a8..74a9679e8843717 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -293,14 +293,14 @@ Changes in existing checks
   <clang-tidy/checks/performance/noexcept-swap>` check to enforce a stricter
   match with the swap function signature, eliminating false-positives.
 
-- Improved :doc:`readability-const-params-in-decls
-  <clang-tidy/checks/readability/const-params-in-decls>` place the hint check under
-  the const token instead of the beginning of parameter declaration.
-
 - Improved :doc:`readability-braces-around-statements
   <clang-tidy/checks/readability/braces-around-statements>` check to
   ignore false-positive for ``if constexpr`` in lambda expression.
 
+- Improved :doc:`readability-const-params-in-decls
+  <clang-tidy/checks/readability/const-params-in-decls>` place the hint check under
+  the const token instead of the beginning of parameter declaration.
+
 - Improved :doc:`readability-container-size-empty
   <clang-tidy/checks/readability/container-size-empty>` check to
   detect comparison between string and empty string literals and support



More information about the cfe-commits mailing list