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

[via cfe-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang-tidy

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

Author: Piotr Zegar (PiotrZSL)

<details>
<summary>Changes</summary>

>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

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


3 Files Affected:

- (modified) clang-tools-extra/clang-tidy/readability/ConstReturnTypeCheck.cpp (+4-17) 
- (modified) clang-tools-extra/docs/ReleaseNotes.rst (+4) 
- (modified) clang-tools-extra/test/clang-tidy/checkers/readability/const-return-type.cpp (+17-3) 


``````````diff
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 3038d2b125f20d..d8bda2d8aea142 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -312,6 +312,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>>();
+  }
+}

``````````

</details>


https://github.com/llvm/llvm-project/pull/90560