[PATCH] D137968: [clang-tidy] Ignore overriden methods in `readability-const-return-type`.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel updated this revision to Diff 475440.
ymandel added a comment.

address comments


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137968/new/

https://reviews.llvm.org/D137968

Files:
  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


Index: clang-tools-extra/test/clang-tidy/checkers/readability/const-return-type.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/readability/const-return-type.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability/const-return-type.cpp
@@ -279,13 +279,22 @@
   // CHECK-NOT-FIXES: virtual int getC() = 0;
 };
 
-class PVDerive : public PVBase {
+class NVDerive : public PVBase {
 public:
-  const int getC() { return 1; }
-  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: return type 'const int' is 'const'-qualified at the top level, which may reduce code readability without improving const correctness
-  // CHECK-NOT-FIXES: int getC() { return 1; }
+  // Don't warn about overridden methods, because it may be impossible to make
+  // them non-const as the user may not be able to change the base class.
+  const int getC() override { return 1; }
 };
 
+class NVDeriveOutOfLine : public PVBase {
+public:
+  // Don't warn about overridden methods, because it may be impossible to make
+  // them non-const as one may not be able to change the base class
+  const int getC();
+};
+
+const int NVDeriveOutOfLine::getC() { return 1; }
+
 // Don't warn about const auto types, because it may be impossible to make them non-const
 // without a significant semantics change. Since `auto` drops cv-qualifiers,
 // tests check `decltype(auto)`.
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -171,6 +171,11 @@
   :doc:`readability-simplify-boolean-expr <clang-tidy/checks/readability/simplify-boolean-expr>`
   when using a C++23 ``if consteval`` statement.
 
+- Change the behavior of :doc:`readability-const-return-type
+  <clang-tidy/checks/readability/const-return-type>` to not
+  warn about `const` return types in overridden functions since the derived
+  class cannot always choose to change the function signature.
+
 - Improved :doc:`misc-redundant-expression <clang-tidy/checks/misc/redundant-expression>`
   check.
 
Index: clang-tools-extra/clang-tidy/readability/ConstReturnTypeCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/readability/ConstReturnTypeCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/ConstReturnTypeCheck.cpp
@@ -114,7 +114,9 @@
   Finder->addMatcher(
       functionDecl(
           returns(allOf(isConstQualified(), unless(NonLocalConstType))),
-          anyOf(isDefinition(), cxxMethodDecl(isPure())))
+          anyOf(isDefinition(), cxxMethodDecl(isPure())),
+          // Overridden functions are not actionable.
+          unless(cxxMethodDecl(isOverride())))
           .bind("func"),
       this);
 }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D137968.475440.patch
Type: text/x-patch
Size: 2841 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20221115/da665190/attachment.bin>