[clang-tools-extra] 1f1a417 - [clang-tidy] Relax readability-const-return-type (#90560)

via cfe-commits cfe-commits at lists.llvm.org
Wed May 1 19:43:38 PDT 2024


Author: Piotr Zegar
Date: 2024-05-02T04:43:35+02:00
New Revision: 1f1a417925624a67cb6cb2bbbdd901e0e90ee237

URL: https://github.com/llvm/llvm-project/commit/1f1a417925624a67cb6cb2bbbdd901e0e90ee237
DIFF: https://github.com/llvm/llvm-project/commit/1f1a417925624a67cb6cb2bbbdd901e0e90ee237.diff

LOG: [clang-tidy] Relax readability-const-return-type (#90560)

>From now readability-const-return-type won't provide warnings for
returning const types, where const is not on top level. In such case
const there is a performance issue, but not a readability.

Closes #73270

Added: 
    

Modified: 
    clang-tools-extra/clang-tidy/readability/ConstReturnTypeCheck.cpp
    clang-tools-extra/docs/ReleaseNotes.rst
    clang-tools-extra/test/clang-tidy/checkers/readability/const-return-type.cpp

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clang-tidy/readability/ConstReturnTypeCheck.cpp b/clang-tools-extra/clang-tidy/readability/ConstReturnTypeCheck.cpp
index e92350632b556b..c13a8010c22210 100644
--- a/clang-tools-extra/clang-tidy/readability/ConstReturnTypeCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/ConstReturnTypeCheck.cpp
@@ -55,14 +55,6 @@ AST_MATCHER(QualType, isLocalConstQualified) {
   return Node.isLocalConstQualified();
 }
 
-AST_MATCHER(QualType, isTypeOfType) {
-  return isa<TypeOfType>(Node.getTypePtr());
-}
-
-AST_MATCHER(QualType, isTypeOfExprType) {
-  return isa<TypeOfExprType>(Node.getTypePtr());
-}
-
 struct CheckResult {
   // Source range of the relevant `const` token in the definition being checked.
   CharSourceRange ConstRange;
@@ -110,16 +102,11 @@ void ConstReturnTypeCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
 void ConstReturnTypeCheck::registerMatchers(MatchFinder *Finder) {
   // Find all function definitions for which the return types are `const`
   // qualified, ignoring decltype types.
-  auto NonLocalConstType =
-      qualType(unless(isLocalConstQualified()),
-               anyOf(decltypeType(), autoType(), isTypeOfType(),
-                     isTypeOfExprType(), substTemplateTypeParmType()));
   Finder->addMatcher(
-      functionDecl(
-          returns(allOf(isConstQualified(), unless(NonLocalConstType))),
-          anyOf(isDefinition(), cxxMethodDecl(isPure())),
-          // Overridden functions are not actionable.
-          unless(cxxMethodDecl(isOverride())))
+      functionDecl(returns(isLocalConstQualified()),
+                   anyOf(isDefinition(), cxxMethodDecl(isPure())),
+                   // Overridden functions are not actionable.
+                   unless(cxxMethodDecl(isOverride())))
           .bind("func"),
       this);
 }

diff  --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 6c9745d585b1c1..d59da3a61b7b61 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -318,6 +318,10 @@ Changes in existing checks
   <clang-tidy/checks/readability/avoid-return-with-void-value>` check by adding
   fix-its.
 
+- Improved :doc:`readability-const-return-type
+  <clang-tidy/checks/readability/const-return-type>` check to eliminate false
+  positives when returning types with const not at the top level.
+
 - Improved :doc:`readability-duplicate-include
   <clang-tidy/checks/readability/duplicate-include>` check by excluding include
   directives that form the filename using macro.

diff  --git a/clang-tools-extra/test/clang-tidy/checkers/readability/const-return-type.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/const-return-type.cpp
index 10b2858c9caa82..76a3555663b180 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/readability/const-return-type.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/readability/const-return-type.cpp
@@ -215,11 +215,9 @@ CREATE_FUNCTION();
 
 using ty = const int;
 ty p21() {}
-// CHECK-MESSAGES: [[@LINE-1]]:1: warning: return type 'ty' (aka 'const int') is
 
 typedef const int ty2;
 ty2 p22() {}
-// CHECK-MESSAGES: [[@LINE-1]]:1: warning: return type 'ty2' (aka 'const int') i
 
 // Declaration uses a macro, while definition doesn't.  In this case, we won't
 // fix the declaration, and will instead issue a warning.
@@ -249,7 +247,6 @@ auto p27() -> int const { return 3; }
 // CHECK-MESSAGES: [[@LINE-1]]:1: warning: return type 'const int' is 'const'-qu
 
 std::add_const<int>::type p28() { return 3; }
-// CHECK-MESSAGES: [[@LINE-1]]:1: warning: return type 'std::add_const<int>::typ
 
 // p29, p30 are based on
 // llvm/projects/test-suite/SingleSource/Benchmarks/Misc-C++-EH/spirit.cpp:
@@ -355,3 +352,20 @@ struct p41 {
   // CHECK-FIXES: T foo() const { return 2; }
 };
 template struct p41<int>;
+
+namespace PR73270 {
+  template<typename K, typename V>
+  struct Pair {
+    using first_type = const K;
+    using second_type = V;
+  };
+
+  template<typename PairType>
+  typename PairType::first_type getFirst() {
+    return {};
+  }
+
+  void test() {
+    getFirst<Pair<int, int>>();
+  }
+}


        


More information about the cfe-commits mailing list