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

[ymandel]

Summary

Overrides are constrained by the signature of the overridden method, so a
warning on an override is frequently unactionable.

[ymandel]

Add test case

[njames93]

Lg, but can you address that one nit before landing

clang-tools-extra/docs/ReleaseNotes.rst

169

Can this be moved to below the braces around statements check as we like to keep things in alphabetical order.

[ymandel]

address comments

clang-tools-extra/clang-tidy/readability/ConstReturnTypeCheck.cpp

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()));
   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);
 }
 
 void ConstReturnTypeCheck::check(const MatchFinder::MatchResult &Result) {
   const auto *Def = Result.Nodes.getNodeAs<FunctionDecl>("func");
   CheckResult CR = checkDef(Def, Result);
   {

clang-tools-extra/docs/ReleaseNotes.rst

- Improved :doc:`modernize-use-equals-default <clang-tidy/checks/modernize/use-equals-default>`
   with nonstandard return types, template constructors, private/protected default constructors
   for C++17 or earlier. The automatic fixit has been adjusted to avoid adding superfluous
   semicolon. The check is restricted to C++11 or later.
 
 - Change the default behavior of :doc:`readability-avoid-const-params-in-decls
   <clang-tidy/checks/readability/avoid-const-params-in-decls>` to not
   warn about `const` value parameters of declarations inside macros.
 
 - Fixed crashes in :doc:`readability-braces-around-statements
   <clang-tidy/checks/readability/braces-around-statements>` and
   :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.
 
   The check now skips concept definitions since redundant expressions still make sense
   inside them.
 
 Removed checks
 ^^^^^^^^^^^^^^

clang-tools-extra/test/clang-tidy/checkers/readability/const-return-type.cpp

 
 class PVBase {
 public:
   virtual const int getC() = 0;
   // 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: 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)`.
 decltype(auto) n16() {
   static const int i = 42;
   return i;
 }