summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorThomas Etter <tetter@google.com>2022-11-14 18:56:07 +0000
committerYitzhak Mandelbaum <yitzhakm@google.com>2022-11-15 14:24:05 +0000
commitf3afd16b65ebda19e417120acf3c3793c171cf5e (patch)
tree71e05b958e0f49e7b918b77965813b60e8a76a43
parente05ce03cfa0b36e9b99149e21afcb1fc039df813 (diff)
[clang-tidy] Ignore overriden methods in `readability-const-return-type`.
Overrides are constrained by the signature of the overridden method, so a warning on an override is frequently unactionable. Differential Revision: https://reviews.llvm.org/D137968
-rw-r--r--clang-tools-extra/clang-tidy/readability/ConstReturnTypeCheck.cpp4
-rw-r--r--clang-tools-extra/docs/ReleaseNotes.rst5
-rw-r--r--clang-tools-extra/test/clang-tidy/checkers/readability/const-return-type.cpp17
3 files changed, 21 insertions, 5 deletions
diff --git a/clang-tools-extra/clang-tidy/readability/ConstReturnTypeCheck.cpp b/clang-tools-extra/clang-tidy/readability/ConstReturnTypeCheck.cpp
index ec9222677378..5a451cca800d 100644
--- a/clang-tools-extra/clang-tidy/readability/ConstReturnTypeCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/ConstReturnTypeCheck.cpp
@@ -114,7 +114,9 @@ void ConstReturnTypeCheck::registerMatchers(MatchFinder *Finder) {
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);
}
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index c5cc967259af..7852c6f3d994 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -171,6 +171,11 @@ Changes in existing checks
: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.
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 4b59cafb9f66..dd3d7aa9fa22 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
@@ -279,13 +279,22 @@ public:
// 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)`.