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_CONST_VALUE_RETURN_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_CONST_VALUE_RETURN_H + +#include "../ClangTidy.h" + +namespace clang { +namespace tidy { +namespace readability { + +/// Checks for functions that return a const-qualified value 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: + ConstValueReturnCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + 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_CONST_VALUE_RETURN_H Index: clang-tidy/readability/ConstValueReturnCheck.cpp =================================================================== --- /dev/null +++ clang-tidy/readability/ConstValueReturnCheck.cpp @@ -0,0 +1,126 @@ +//===--- 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 "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/Lex/Lexer.h" + +using namespace clang::ast_matchers; + +namespace clang { +namespace tidy { +namespace readability { + +void ConstValueReturnCheck::registerMatchers(MatchFinder *Finder) { + // Returning const pointers (not pointers to const) does not make much sense, + // returning const references even less so, but it is harmless and we don't + // bother checking it. + // Template parameter types may end up being pointers or references, so we + // skip those too. + Finder->addMatcher(functionDecl(returns(qualType( + isConstQualified(), + unless(anyOf( + isAnyPointer(), + references(type()), + templateTypeParmType(), + hasCanonicalType(templateTypeParmType()) + ))))).bind("func"), this); +} + +static bool isWithinRange(const Token& Tok, SourceRange Range, + const SourceManager &Sources) { + return (Sources.isBeforeInTranslationUnit( + Range.getBegin(), Tok.getLocation()) && + Sources.isBeforeInTranslationUnit( + Tok.getLocation(), Range.getEnd())); +} + +// Looks for const tokens within SearchRange, ignoring those within +// ReturnRange. If exactly one is found, it is returned. +static llvm::Optional findConstToken( + SourceRange SearchRange, SourceRange ReturnRange, + const MatchFinder::MatchResult &Result) { + const SourceManager &Sources = *Result.SourceManager; + FileID FID; + unsigned Offset; + std::tie(FID, Offset) = Sources.getDecomposedLoc(SearchRange.getBegin()); + StringRef FileData = Sources.getBufferData(FID); + + Lexer RawLexer(Sources.getLocForStartOfFile(FID), + Result.Context->getLangOpts(), + FileData.begin(), FileData.begin() + Offset, + FileData.end()); + + Token Tok; + llvm::Optional ConstTok; + int ConstFound = 0; + while (!RawLexer.LexFromRawLexer(Tok)) { + // Stop at the end of SearchRange. + if (Sources.isBeforeInTranslationUnit( + SearchRange.getEnd(), Tok.getLocation())) + break; + if (Tok.is(tok::raw_identifier)) { + IdentifierInfo &Info = Result.Context->Idents.get(StringRef( + Sources.getCharacterData(Tok.getLocation()), Tok.getLength())); + Tok.setIdentifierInfo(&Info); + Tok.setKind(Info.getTokenID()); + } + // Ignore consts inside ReturnRange. + if (isWithinRange(Tok, ReturnRange, Sources)) + continue; + if (Tok.is(tok::kw_const)) { + ++ConstFound; + ConstTok = Tok; + } + } + + if (ConstFound > 1) + ConstTok.reset(); + + return ConstTok; +} + +void ConstValueReturnCheck::check(const MatchFinder::MatchResult &Result) { + const auto *Func = Result.Nodes.getNodeAs("func"); + auto Diag = diag(Func->getLocation(), + "function %0 has a const value return type; " + "this can cause unnecessary copies") + << Func; + + auto ReturnType = Func->getReturnType(); + if (!ReturnType.isLocalConstQualified()) + return; + + const auto FuncRange = Func->getSourceRange(); + if (FuncRange.isInvalid()) + return; + + // When using trailing return type syntax, an invalid range is returned, + // so we cannot generate the fixit. + const auto ReturnRange = Func->getReturnTypeSourceRange(); + if (ReturnRange.isInvalid()) + return; + + // getReturnTypeSourceRange does not include the qualifiers, so we have to + // find the "const" ourselves. + auto NameLoc = Func->getNameInfo().getLoc(); + auto MaybeConst = findConstToken({FuncRange.getBegin(), NameLoc}, + ReturnRange, Result); + if (!MaybeConst) + return; + + auto TokLoc = MaybeConst->getLocation(); + Diag << FixItHint::CreateRemoval( + CharSourceRange::getTokenRange(TokLoc, TokLoc)); +} + +} // 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" @@ -46,6 +47,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: docs/clang-tidy/checks/list.rst =================================================================== --- docs/clang-tidy/checks/list.rst +++ docs/clang-tidy/checks/list.rst @@ -150,6 +150,7 @@ performance-unnecessary-value-param 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,28 @@ +.. title:: clang-tidy - readability-const-value-return + +readability-const-value-return +============================== + +Checks for functions that return a ``const``-qualified value type. That use of +``const`` is superfluous, and prevents moving the object. Therefore, it should +be avoided. + +Examples: + +.. code-block:: c++ + + const Foo f_bad(); + Foo f_good(); + + Foo foo; + foo = f_bad(); // This requires a copy. + foo = f_good(); // This can use a move. + + +Note that this does not apply to returning pointers or references to const +objects: + +.. code-block:: c++ + + const Foo* f_ptr(); // No issue. + const Foo& f_ref(); // No issue. Index: test/clang-tidy/readability-const-value-return.cpp =================================================================== --- /dev/null +++ test/clang-tidy/readability-const-value-return.cpp @@ -0,0 +1,59 @@ +// RUN: %check_clang_tidy %s readability-const-value-return %t + +namespace std { +template class function; +template class function; +} + +struct X { + int a; +}; + +// These should trigger the check. +const X f_returns_const_value(); +// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: function 'f_returns_const_value' has a const value return type; this can cause unnecessary copies [readability-const-value-return] +// CHECK-FIXES: {{^}}X f_returns_const_value();{{$}} + +X const f_returns_const_value2(); +// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: function 'f_returns_const_value2' has a const value return type; this can cause unnecessary copies [readability-const-value-return] +// CHECK-FIXES: {{^}}X f_returns_const_value2();{{$}} + +// When using trailing return type syntax, we cannot generate the fixit, because we cannot get the return type range. +auto f_returns_const_value3() -> const X; +// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'f_returns_const_value3' has a const value return type; this can cause unnecessary copies [readability-const-value-return] + +auto f_returns_const_value4() -> X const; +// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'f_returns_const_value4' has a const value return type; this can cause unnecessary copies [readability-const-value-return] + +// Make sure we get the right "const". +const std::function f_returns_const_fun(); +// CHECK-MESSAGES: :[[@LINE-1]]:40: warning: function 'f_returns_const_fun' has a const value return type; this can cause unnecessary copies [readability-const-value-return] +// CHECK-FIXES: {{^}}std::function f_returns_const_fun();{{$}} + +std::function const f_returns_const_fun2(); +// CHECK-MESSAGES: :[[@LINE-1]]:40: warning: function 'f_returns_const_fun2' has a const value return type; this can cause unnecessary copies [readability-const-value-return] +// CHECK-FIXES: {{^}}std::function f_returns_const_fun2();{{$}} + +// These should not trigger the check. +X f_returns_nonconst_value(); +const X* f_returns_pointer_to_const(); +X* const f_returns_const_pointer(); +const X& f_returns_reference_to_const(); + +typedef X* Xptr; +typedef X& Xref; +const Xptr f_returns_typedef_pointer(); +const Xref f_returns_typedef_reference(); + +// A template parameter type may end up being a pointer or a reference, so we skip it too. +template +const T f_returns_template_param(); + +template +struct S { + using Q = T; + const Q f_returns_template_param_alias(); +}; + +using R = X&; +const R f_returns_reference_alias();