Author: Evgeny Shulgin Date: 2022-03-17T14:27:09+03:00 New Revision: 6043520c207269ccdbfe70e63ab48118d5735deb
URL: https://github.com/llvm/llvm-project/commit/6043520c207269ccdbfe70e63ab48118d5735deb DIFF: https://github.com/llvm/llvm-project/commit/6043520c207269ccdbfe70e63ab48118d5735deb.diff LOG: [clang-tidy] Don't check decltype return types in `readability-const-return-type` The checker removes `const`s that are superfluos and badly affect readability. `decltype(auto)`/`decltype(expr)` are often const-qualified, but have no effect on readability and usually can't stop being const-qualified without significant code change. Fixes https://github.com/llvm/llvm-project/issues/52890 Reviewed By: aaron.ballman Differential Revision: https://reviews.llvm.org/D119470 Added: Modified: clang-tools-extra/clang-tidy/readability/ConstReturnTypeCheck.cpp clang-tools-extra/test/clang-tidy/checkers/readability-const-return-type.cpp Removed: ################################################################################ diff --git a/clang-tools-extra/clang-tidy/readability/ConstReturnTypeCheck.cpp b/clang-tools-extra/clang-tidy/readability/ConstReturnTypeCheck.cpp index 814d3f266c5c3..ec92226773781 100644 --- a/clang-tools-extra/clang-tidy/readability/ConstReturnTypeCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/ConstReturnTypeCheck.cpp @@ -53,6 +53,18 @@ findConstToRemove(const FunctionDecl *Def, namespace { +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; @@ -95,10 +107,14 @@ static CheckResult checkDef(const clang::FunctionDecl *Def, void ConstReturnTypeCheck::registerMatchers(MatchFinder *Finder) { // Find all function definitions for which the return types are `const` - // qualified. + // qualified, ignoring decltype types. + auto NonLocalConstType = qualType( + unless(isLocalConstQualified()), + anyOf(decltypeType(), autoType(), isTypeOfType(), isTypeOfExprType())); Finder->addMatcher( - functionDecl(returns(isConstQualified()), - anyOf(isDefinition(), cxxMethodDecl(isPure()))) + functionDecl( + returns(allOf(isConstQualified(), unless(NonLocalConstType))), + anyOf(isDefinition(), cxxMethodDecl(isPure()))) .bind("func"), this); } 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 70da965275650..a8642746145b8 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 @@ -1,4 +1,4 @@ -// RUN: %check_clang_tidy %s readability-const-return-type %t +// RUN: %check_clang_tidy -std=c++14 %s readability-const-return-type %t // p# = positive test // n# = negative test @@ -285,3 +285,40 @@ class PVDerive : public PVBase { // 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 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; +} + +// Don't warn about `decltype(<expr>)` types +const int n17i = 1; +decltype(n17i) n17() { + return 17; +} + +// Do warn when on decltype types with the local const qualifier +// `const decltype(auto)` won't compile, so check only `const decltype(<expr>)` +const decltype(n17i) n18() { + // CHECK-MESSAGES: [[@LINE-1]]:1: warning: return type 'const decltype(n17i) + // CHECK-FIXES: decltype(n17i) n18() { + return 18; +} + +// `volatile` modifier doesn't affect the checker +volatile decltype(n17i) n19() { + return 19; +} + +// Don't warn about `__typeof__(<expr>)` types +__typeof__(n17i) n20() { + return 20; +} + +// Don't warn about `__typeof__(type)` types +__typeof__(const int) n21() { + return 21; +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits