https://github.com/PiotrZSL created https://github.com/llvm/llvm-project/pull/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 >From 72306af4660bdbc5bd475ef3512150c00ec9f24d Mon Sep 17 00:00:00 2001 From: Piotr Zegar <m...@piotrzegar.pl> Date: Tue, 30 Apr 2024 05:16:55 +0000 Subject: [PATCH] [clang-tidy] Relax readability-const-return-type >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 --- .../readability/ConstReturnTypeCheck.cpp | 21 ++++--------------- clang-tools-extra/docs/ReleaseNotes.rst | 4 ++++ .../readability/const-return-type.cpp | 20 +++++++++++++++--- 3 files changed, 25 insertions(+), 20 deletions(-) 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>>(); + } +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits