Index: clang-tidy/readability/CMakeLists.txt =================================================================== --- clang-tidy/readability/CMakeLists.txt +++ clang-tidy/readability/CMakeLists.txt @@ -3,6 +3,7 @@ add_clang_library(clangTidyReadabilityModule AvoidConstParamsInDecls.cpp BracesAroundStatementsCheck.cpp + ConstValueReturnCheck.cpp ContainerSizeEmptyCheck.cpp DeleteNullPointerCheck.cpp DeletedDefaultCheck.cpp Index: clang-tidy/readability/ConstValueReturnCheck.h =================================================================== --- /dev/null +++ clang-tidy/readability/ConstValueReturnCheck.h @@ -0,0 +1,35 @@ +//===--- ConstValueReturnCheck.h - clang-tidy -------------------*- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_CONSTVALUERETURNCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_CONSTVALUERETURNCHECK_H + +#include "../ClangTidy.h" + +namespace clang { +namespace tidy { +namespace readability { + +/// Suggests removal of the `const` qualifier from any function that returns a +/// const type. +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/readability-const-value-return.html +class ConstValueReturnCheck : public ClangTidyCheck { + public: + using ClangTidyCheck::ClangTidyCheck; + void registerMatchers(ast_matchers::MatchFinder* finder) override; + void check(const ast_matchers::MatchFinder::MatchResult& result) override; +}; + +} // namespace readability +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_CONSTVALUERETURNCHECK_H Index: clang-tidy/readability/ConstValueReturnCheck.cpp =================================================================== --- /dev/null +++ clang-tidy/readability/ConstValueReturnCheck.cpp @@ -0,0 +1,89 @@ +//===--- ConstValueReturnCheck.cpp - clang-tidy ---------------------------===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#include "ConstValueReturnCheck.h" +#include "../utils/LexerUtils.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/Lex/Lexer.h" +#include "llvm/ADT/Optional.h" + +using namespace clang::ast_matchers; + +namespace clang { +namespace tidy { +namespace readability { + +namespace { + +// Finds the location of the relevant "const" token in `Def`. +llvm::Optional findConstToRemove( + const FunctionDecl *Def, const MatchFinder::MatchResult &Result) { + // Since either of the locs can be in a macro, use makeFileCharRange to be + // sure that we have a consistent char range, located entirely in the source + // file. + CharSourceRange FileRange = Lexer::makeFileCharRange( + CharSourceRange::getCharRange(Def->getBeginLoc(), + Def->getNameInfo().getLoc()), + *Result.SourceManager, Result.Context->getLangOpts()); + + if (FileRange.isInvalid()) + return {}; + + std::vector Locs = utils::lexer::getConstTokLocations( + FileRange, *Result.Context, *Result.SourceManager); + if (Locs.empty()) + return {}; + + // If the return type is a pointer, then remove `const` after asterisk, which + // will be the last const before function name; else, get first. + return Def->getReturnType()->isPointerType() ? Locs.back() : Locs.front(); +} + +} // namespace + +void ConstValueReturnCheck::registerMatchers(MatchFinder *Finder) { + // Find all the functions for which the return types are const qualified. + Finder->addMatcher( + functionDecl(returns(isConstQualified()), isDefinition()).bind("func"), + this); +} + +void ConstValueReturnCheck::check(const MatchFinder::MatchResult &Result) { + const auto *Def = Result.Nodes.getNodeAs("func"); + if (!Def->getReturnType().isLocalConstQualified()) { + return; + } + + // Fix the definition. + llvm::Optional Loc = findConstToRemove(Def, Result); + if (!Loc) + return; + + DiagnosticBuilder Diagnostics = + diag(*Loc, + "avoid marking return types as ``const`` for values, as it often " + "disables optimizations and causes unnecessary copies") + << FixItHint::CreateRemoval(CharSourceRange::getTokenRange(*Loc, *Loc)); + + // Fix any visible declarations, but don't warn with a separate message -- + // associate all fixes with the definition. + for (const FunctionDecl *Decl = Def->getPreviousDecl(); Decl != nullptr; + Decl = Decl->getPreviousDecl()) { + if (const llvm::Optional PrevLoc = + findConstToRemove(Decl, Result)) { + Diagnostics << FixItHint::CreateRemoval( + CharSourceRange::getTokenRange(*PrevLoc, *PrevLoc)); + } + } +} + +} // namespace readability +} // namespace tidy +} // namespace clang Index: clang-tidy/readability/ReadabilityTidyModule.cpp =================================================================== --- clang-tidy/readability/ReadabilityTidyModule.cpp +++ clang-tidy/readability/ReadabilityTidyModule.cpp @@ -12,6 +12,7 @@ #include "../ClangTidyModuleRegistry.h" #include "AvoidConstParamsInDecls.h" #include "BracesAroundStatementsCheck.h" +#include "ConstValueReturnCheck.h" #include "ContainerSizeEmptyCheck.h" #include "DeleteNullPointerCheck.h" #include "DeletedDefaultCheck.h" @@ -50,6 +51,8 @@ "readability-avoid-const-params-in-decls"); CheckFactories.registerCheck( "readability-braces-around-statements"); + CheckFactories.registerCheck( + "readability-const-value-return"); CheckFactories.registerCheck( "readability-container-size-empty"); CheckFactories.registerCheck( Index: clang-tidy/utils/LexerUtils.h =================================================================== --- clang-tidy/utils/LexerUtils.h +++ clang-tidy/utils/LexerUtils.h @@ -22,6 +22,11 @@ Token getPreviousToken(SourceLocation Location, const SourceManager &SM, const LangOptions &LangOpts, bool SkipComments = true); +// Gets the start locations of all ``const`` tokens in the Range. +std::vector getConstTokLocations( + CharSourceRange Range, const clang::ASTContext &Context, + const SourceManager &SM); + } // namespace lexer } // namespace utils } // namespace tidy Index: clang-tidy/utils/LexerUtils.cpp =================================================================== --- clang-tidy/utils/LexerUtils.cpp +++ clang-tidy/utils/LexerUtils.cpp @@ -31,6 +31,31 @@ return Token; } +std::vector getConstTokLocations( + CharSourceRange Range, const clang::ASTContext &Context, + const SourceManager &SM) { + std::pair LocInfo = + SM.getDecomposedLoc(Range.getBegin()); + StringRef File = SM.getBufferData(LocInfo.first); + const char *TokenBegin = File.data() + LocInfo.second; + Lexer RawLexer(SM.getLocForStartOfFile(LocInfo.first), Context.getLangOpts(), + File.begin(), TokenBegin, File.end()); + Token Tok; + std::vector Locations; + while (!RawLexer.LexFromRawLexer(Tok) && + !SM.isBeforeInTranslationUnit(Range.getEnd(), Tok.getLocation())) { + if (Tok.is(tok::raw_identifier)) { + IdentifierInfo &Info = Context.Idents.get( + StringRef(SM.getCharacterData(Tok.getLocation()), Tok.getLength())); + Tok.setIdentifierInfo(&Info); + Tok.setKind(Info.getTokenID()); + } + if (Tok.is(tok::kw_const)) + Locations.push_back(Tok.getLocation()); + } + return Locations; +} + } // namespace lexer } // namespace utils } // namespace tidy Index: docs/ReleaseNotes.rst =================================================================== --- docs/ReleaseNotes.rst +++ docs/ReleaseNotes.rst @@ -106,6 +106,12 @@ This check warns the uses of the deprecated member types of ``std::ios_base`` and replaces those that have a non-deprecated equivalent. +- New :doc:`readability-const-value-return + ` check. + + Checks for functions with a ``const``-qualified return type and recommends + removal of the ``const`` keyword. + - New :doc:`readability-magic-numbers ` check. Index: docs/clang-tidy/checks/list.rst =================================================================== --- docs/clang-tidy/checks/list.rst +++ docs/clang-tidy/checks/list.rst @@ -9,8 +9,8 @@ abseil-no-internal-dependencies abseil-no-namespace abseil-redundant-strcat-calls - abseil-string-find-startswith abseil-str-cat-append + abseil-string-find-startswith android-cloexec-accept android-cloexec-accept4 android-cloexec-creat @@ -219,6 +219,7 @@ portability-simd-intrinsics readability-avoid-const-params-in-decls readability-braces-around-statements + readability-const-value-return readability-container-size-empty readability-delete-null-pointer readability-deleted-default Index: docs/clang-tidy/checks/readability-const-value-return.rst =================================================================== --- /dev/null +++ docs/clang-tidy/checks/readability-const-value-return.rst @@ -0,0 +1,25 @@ +.. title:: clang-tidy - readability-const-value-return + +readability-const-value-return +=================================== + +Checks for functions with a ``const``-qualified return type and recommends +removal of the ``const`` keyword. Such use of ``const`` is superfluous, and +prevents valuable compiler optimizations. + +Examples: + +.. code-block:: c++ + + const int foo(); + const Clazz foo(); + Clazz *const foo(); + +Note that this does not apply to returning pointers or references to const +values. These are fine: + +.. code-block:: c++ + + const int* foo(); + const int& foo(); + const Clazz* foo(); Index: test/clang-tidy/Inputs/readability-const-value-return/macro-def.h =================================================================== --- /dev/null +++ test/clang-tidy/Inputs/readability-const-value-return/macro-def.h @@ -0,0 +1,6 @@ +#ifndef MACRO_DEF_H +#define MACRO_DEF_H + +#define DEFFF const int + +#endif // MACRO_DEF_H Index: test/clang-tidy/readability-const-value-return.cpp =================================================================== --- /dev/null +++ test/clang-tidy/readability-const-value-return.cpp @@ -0,0 +1,139 @@ +// RUN: %check_clang_tidy %s readability-const-value-return %t -- -- \ +// RUN: -I %S/Inputs/readability-const-value-return + +#include "macro-def.h" + +// p# = positive test +// n# = negative test + +const int p1() { +// CHECK-MESSAGES: [[@LINE-1]]:1: warning: avoid marking return types as ``const`` for values, as it often disables optimizations and causes unnecessary copies +// CHECK-FIXES: int p1() { + return 1; +} + +const int p15(); +// CHECK-FIXES: int p15(); + +template class Klazz {}; +class Clazz { + public: + Clazz *const p2() { + // CHECK-MESSAGES: [[@LINE-1]]:10: warning: avoid marking return types as `` + // CHECK-FIXES: Clazz *p2() { + return this; + } + + Clazz *const p3(); + // CHECK-FIXES: Clazz *p3(); + + const int p4() const { + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: avoid marking return types as ``c + // CHECK-FIXES: int p4() const { + return 4; + } + + const Klazz* const p5() const; + // CHECK-FIXES: const Klazz* p5() const; + + const Clazz operator++(int x) { // p12 + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: avoid marking return types as ``con + // CHECK-FIXES: Clazz operator++(int x) { + } + + struct Strukt { + int i; + }; + + const Strukt p6() {} + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: avoid marking return types as ``con + // CHECK-FIXES: Strukt p6() {} + + const Strukt* const p7(); + // CHECK-FIXES: const Strukt* p7(); + + static const int p8() {} + // CHECK-MESSAGES: [[@LINE-1]]:10: warning: avoid marking return types as ``co + // CHECK-FIXES: static int p8() {} + + static const Strukt p9() {} + // CHECK-MESSAGES: [[@LINE-1]]:10: warning: avoid marking return types as ``co + // CHECK-FIXES: static Strukt p9() {} + + int n0() const { return 0; } + const Klazz& n11(const Klazz) const; +}; + +Clazz *const Clazz::p3() { + // CHECK-MESSAGES: [[@LINE-1]]:8: warning: avoid marking return types as ``con + // CHECK-FIXES: Clazz *Clazz::p3() { + return this; +} + +const Klazz* const Clazz::p5() const {} +// CHECK-MESSAGES: [[@LINE-1]]:25: warning: avoid marking return types as ``cons +// CHECK-FIXES: const Klazz* Clazz::p5() const {} + +const Clazz::Strukt* const Clazz::p7() {} +// CHECK-MESSAGES: [[@LINE-1]]:22: warning: avoid marking return types as ``cons +// CHECK-FIXES: const Clazz::Strukt* Clazz::p7() {} + +Clazz *const p10(); +// CHECK-FIXES: Clazz *p10(); + +Clazz *const p10() { + // CHECK-MESSAGES: [[@LINE-1]]:8: warning: avoid marking return types as ``con + // CHECK-FIXES: Clazz *p10() { + return new Clazz(); +} + +const Clazz bar; +const Clazz *const p11() { + // CHECK-MESSAGES: [[@LINE-1]]:14: warning: avoid marking return types as ``co + // CHECK-FIXES: const Clazz *p11() { + return &bar; +} + +const Klazz p12() {} +// CHECK-MESSAGES: [[@LINE-1]]:1: warning: avoid marking return types as ``const +// CHECK-FIXES: Klazz p12() {} + +const Klazz* const p13() {} +// CHECK-MESSAGES: [[@LINE-1]]:25: warning: avoid marking return types as ``cons +// CHECK-FIXES: const Klazz* p13() {} + +const auto p14() { +// CHECK-MESSAGES: [[@LINE-1]]:1: warning: avoid marking return types as ``cons +// CHECK-FIXES: auto p14() { + const int i = 0; + return i; +} + +//another decl at top. +const int p15(); +// CHECK-FIXES: int p15(); +const int p15() { +// CHECK-MESSAGES: [[@LINE-1]]:1: warning: avoid marking return types as ``const +// CHECK-FIXES: int p15() { + return 0; +} + +const int n1 = 1; +const Clazz n2 = Clazz(); +const Clazz* n3 = new Clazz(); +Clazz *const n4 = new Clazz(); +const Clazz *const n5 = new Clazz(); +constexpr int n6 = 6; +constexpr int n7() { return 8; } +const int eight = 8; +constexpr const int* n8() { return &eight; } +Klazz n9(); +const Klazz* n10(); +const Klazz& Clazz::n11(const Klazz) const {} +const int n14(); + +#define CONSTINT const int +CONSTINT n12() {} + +// DEFFF in header. +DEFFF n13() {}