Izaron created this revision. Izaron added reviewers: aaron.ballman, alexfh. Herald added subscribers: carlosgalvezp, xazax.hun. Izaron requested review of this revision. Herald added a project: clang-tools-extra. Herald added a subscriber: cfe-commits.
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 Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D119470 Files: clang-tools-extra/clang-tidy/readability/ConstReturnTypeCheck.cpp 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 @@ -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 @@ -271,3 +271,25 @@ int **const * n_multiple_ptr(); int *const & n_pointer_ref(); + +// 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 as well +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; +} 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 @@ -53,6 +53,10 @@ namespace { +AST_MATCHER(QualType, isLocalConstQualified) { + return Node.isLocalConstQualified(); +} + struct CheckResult { // Source range of the relevant `const` token in the definition being checked. CharSourceRange ConstRange; @@ -95,9 +99,14 @@ void ConstReturnTypeCheck::registerMatchers(MatchFinder *Finder) { // Find all function definitions for which the return types are `const` - // qualified. + // qualified, ignoring decltype types. + const auto NonLocalConstDecltypeType = qualType( + unless(isLocalConstQualified()), anyOf(decltypeType(), autoType())); Finder->addMatcher( - functionDecl(returns(isConstQualified()), isDefinition()).bind("func"), + functionDecl( + returns(allOf(isConstQualified(), unless(NonLocalConstDecltypeType))), + isDefinition()) + .bind("func"), this); }
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 @@ -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 @@ -271,3 +271,25 @@ int **const * n_multiple_ptr(); int *const & n_pointer_ref(); + +// 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 as well +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; +} 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 @@ -53,6 +53,10 @@ namespace { +AST_MATCHER(QualType, isLocalConstQualified) { + return Node.isLocalConstQualified(); +} + struct CheckResult { // Source range of the relevant `const` token in the definition being checked. CharSourceRange ConstRange; @@ -95,9 +99,14 @@ void ConstReturnTypeCheck::registerMatchers(MatchFinder *Finder) { // Find all function definitions for which the return types are `const` - // qualified. + // qualified, ignoring decltype types. + const auto NonLocalConstDecltypeType = qualType( + unless(isLocalConstQualified()), anyOf(decltypeType(), autoType())); Finder->addMatcher( - functionDecl(returns(isConstQualified()), isDefinition()).bind("func"), + functionDecl( + returns(allOf(isConstQualified(), unless(NonLocalConstDecltypeType))), + isDefinition()) + .bind("func"), this); }
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits