Index: clang-tidy/hicpp/CMakeLists.txt =================================================================== --- clang-tidy/hicpp/CMakeLists.txt +++ clang-tidy/hicpp/CMakeLists.txt @@ -17,6 +17,7 @@ clangTidyGoogleModule clangTidyMiscModule clangTidyModernizeModule + clangTidyPerformanceModule clangTidyReadabilityModule clangTidyUtils ) Index: clang-tidy/hicpp/HICPPTidyModule.cpp =================================================================== --- clang-tidy/hicpp/HICPPTidyModule.cpp +++ clang-tidy/hicpp/HICPPTidyModule.cpp @@ -18,9 +18,7 @@ #include "../cppcoreguidelines/SpecialMemberFunctionsCheck.h" #include "../google/DefaultArgumentsCheck.h" #include "../google/ExplicitConstructorCheck.h" -#include "../misc/MoveConstantArgumentCheck.h" #include "../misc/NewDeleteOverloadsCheck.h" -#include "../misc/NoexceptMoveConstructorCheck.h" #include "../misc/StaticAssertCheck.h" #include "../misc/UndelegatedConstructor.h" #include "../modernize/DeprecatedHeadersCheck.h" @@ -31,6 +29,8 @@ #include "../modernize/UseNoexceptCheck.h" #include "../modernize/UseNullptrCheck.h" #include "../modernize/UseOverrideCheck.h" +#include "../performance/MoveConstArgCheck.h" +#include "../performance/NoexceptMoveConstructorCheck.h" #include "../readability/BracesAroundStatementsCheck.h" #include "../readability/FunctionSizeCheck.h" #include "../readability/IdentifierNamingCheck.h" @@ -63,11 +63,11 @@ "hicpp-invalid-access-moved"); CheckFactories.registerCheck( "hicpp-member-init"); - CheckFactories.registerCheck( + CheckFactories.registerCheck( "hicpp-move-const-arg"); CheckFactories.registerCheck( "hicpp-new-delete-operators"); - CheckFactories.registerCheck( + CheckFactories.registerCheck( "hicpp-noexcept-move"); CheckFactories .registerCheck( Index: clang-tidy/misc/CMakeLists.txt =================================================================== --- clang-tidy/misc/CMakeLists.txt +++ clang-tidy/misc/CMakeLists.txt @@ -11,9 +11,7 @@ MacroRepeatedSideEffectsCheck.cpp MiscTidyModule.cpp MisplacedWideningCastCheck.cpp - MoveConstantArgumentCheck.cpp NewDeleteOverloadsCheck.cpp - NoexceptMoveConstructorCheck.cpp NonCopyableObjects.cpp RedundantExpressionCheck.cpp SizeofContainerCheck.cpp Index: clang-tidy/misc/MiscTidyModule.cpp =================================================================== --- clang-tidy/misc/MiscTidyModule.cpp +++ clang-tidy/misc/MiscTidyModule.cpp @@ -18,9 +18,7 @@ #include "MacroRepeatedSideEffectsCheck.h" #include "MisplacedConstCheck.h" #include "MisplacedWideningCastCheck.h" -#include "MoveConstantArgumentCheck.h" #include "NewDeleteOverloadsCheck.h" -#include "NoexceptMoveConstructorCheck.h" #include "NonCopyableObjects.h" #include "RedundantExpressionCheck.h" #include "SizeofContainerCheck.h" @@ -67,12 +65,8 @@ "misc-macro-repeated-side-effects"); CheckFactories.registerCheck( "misc-misplaced-widening-cast"); - CheckFactories.registerCheck( - "misc-move-const-arg"); CheckFactories.registerCheck( "misc-new-delete-overloads"); - CheckFactories.registerCheck( - "misc-noexcept-move-constructor"); CheckFactories.registerCheck( "misc-non-copyable-objects"); CheckFactories.registerCheck( Index: clang-tidy/misc/MoveConstantArgumentCheck.h =================================================================== --- clang-tidy/misc/MoveConstantArgumentCheck.h +++ clang-tidy/misc/MoveConstantArgumentCheck.h @@ -1,43 +0,0 @@ -//===--- MoveConstantArgumentCheck.h - clang-tidy -------------------------===// -// -// 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_MISC_MOVECONSTANTARGUMENTCHECK_H -#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_MOVECONSTANTARGUMENTCHECK_H - -#include "../ClangTidy.h" - -namespace clang { -namespace tidy { -namespace misc { - -/// Find casts of calculation results to bigger type. Typically from int to -/// -/// There is one option: -/// -/// - `CheckTriviallyCopyableMove`: Whether to check for trivially-copyable -// types as their objects are not moved but copied. Enabled by default. -class MoveConstantArgumentCheck : public ClangTidyCheck { -public: - MoveConstantArgumentCheck(StringRef Name, ClangTidyContext *Context) - : ClangTidyCheck(Name, Context), - CheckTriviallyCopyableMove( - Options.get("CheckTriviallyCopyableMove", true)) {} - void storeOptions(ClangTidyOptions::OptionMap &Opts) override; - void registerMatchers(ast_matchers::MatchFinder *Finder) override; - void check(const ast_matchers::MatchFinder::MatchResult &Result) override; - -private: - const bool CheckTriviallyCopyableMove; -}; - -} // namespace misc -} // namespace tidy -} // namespace clang - -#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_MOVECONSTANTARGUMENTCHECK_H Index: clang-tidy/misc/MoveConstantArgumentCheck.cpp =================================================================== --- clang-tidy/misc/MoveConstantArgumentCheck.cpp +++ clang-tidy/misc/MoveConstantArgumentCheck.cpp @@ -1,122 +0,0 @@ -//===--- MoveConstantArgumentCheck.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 "MoveConstantArgumentCheck.h" - -#include "clang/Lex/Lexer.h" - -using namespace clang::ast_matchers; - -namespace clang { -namespace tidy { -namespace misc { - -static void ReplaceCallWithArg(const CallExpr *Call, DiagnosticBuilder &Diag, - const SourceManager &SM, - const LangOptions &LangOpts) { - const Expr *Arg = Call->getArg(0); - - CharSourceRange BeforeArgumentsRange = Lexer::makeFileCharRange( - CharSourceRange::getCharRange(Call->getLocStart(), Arg->getLocStart()), - SM, LangOpts); - CharSourceRange AfterArgumentsRange = Lexer::makeFileCharRange( - CharSourceRange::getCharRange(Call->getLocEnd(), - Call->getLocEnd().getLocWithOffset(1)), - SM, LangOpts); - - if (BeforeArgumentsRange.isValid() && AfterArgumentsRange.isValid()) { - Diag << FixItHint::CreateRemoval(BeforeArgumentsRange) - << FixItHint::CreateRemoval(AfterArgumentsRange); - } -} - -void MoveConstantArgumentCheck::storeOptions( - ClangTidyOptions::OptionMap &Opts) { - Options.store(Opts, "CheckTriviallyCopyableMove", CheckTriviallyCopyableMove); -} - -void MoveConstantArgumentCheck::registerMatchers(MatchFinder *Finder) { - if (!getLangOpts().CPlusPlus) - return; - - auto MoveCallMatcher = - callExpr(callee(functionDecl(hasName("::std::move"))), argumentCountIs(1), - unless(isInTemplateInstantiation())) - .bind("call-move"); - - Finder->addMatcher(MoveCallMatcher, this); - - auto ConstParamMatcher = forEachArgumentWithParam( - MoveCallMatcher, parmVarDecl(hasType(references(isConstQualified())))); - - Finder->addMatcher(callExpr(ConstParamMatcher).bind("receiving-expr"), this); - Finder->addMatcher(cxxConstructExpr(ConstParamMatcher).bind("receiving-expr"), - this); -} - -void MoveConstantArgumentCheck::check(const MatchFinder::MatchResult &Result) { - const auto *CallMove = Result.Nodes.getNodeAs("call-move"); - const auto *ReceivingExpr = Result.Nodes.getNodeAs("receiving-expr"); - const Expr *Arg = CallMove->getArg(0); - SourceManager &SM = Result.Context->getSourceManager(); - - CharSourceRange MoveRange = - CharSourceRange::getCharRange(CallMove->getSourceRange()); - CharSourceRange FileMoveRange = - Lexer::makeFileCharRange(MoveRange, SM, getLangOpts()); - if (!FileMoveRange.isValid()) - return; - - bool IsConstArg = Arg->getType().isConstQualified(); - bool IsTriviallyCopyable = - Arg->getType().isTriviallyCopyableType(*Result.Context); - - if (IsConstArg || IsTriviallyCopyable) { - if (const CXXRecordDecl *R = Arg->getType()->getAsCXXRecordDecl()) { - // According to [expr.prim.lambda]p3, "whether the closure type is - // trivially copyable" property can be changed by the implementation of - // the language, so we shouldn't rely on it when issuing diagnostics. - if (R->isLambda()) - return; - // Don't warn when the type is not copyable. - for (const auto *Ctor : R->ctors()) { - if (Ctor->isCopyConstructor() && Ctor->isDeleted()) - return; - } - } - - if (!IsConstArg && IsTriviallyCopyable && !CheckTriviallyCopyableMove) - return; - - bool IsVariable = isa(Arg); - const auto *Var = - IsVariable ? dyn_cast(Arg)->getDecl() : nullptr; - auto Diag = diag(FileMoveRange.getBegin(), - "std::move of the %select{|const }0" - "%select{expression|variable %4}1 " - "%select{|of the trivially-copyable type %5 }2" - "has no effect; remove std::move()" - "%select{| or make the variable non-const}3") - << IsConstArg << IsVariable << IsTriviallyCopyable - << (IsConstArg && IsVariable && !IsTriviallyCopyable) << Var - << Arg->getType(); - - ReplaceCallWithArg(CallMove, Diag, SM, getLangOpts()); - } else if (ReceivingExpr) { - auto Diag = diag(FileMoveRange.getBegin(), - "passing result of std::move() as a const reference " - "argument; no move will actually happen"); - - ReplaceCallWithArg(CallMove, Diag, SM, getLangOpts()); - } -} - -} // namespace misc -} // namespace tidy -} // namespace clang Index: clang-tidy/misc/NoexceptMoveConstructorCheck.h =================================================================== --- clang-tidy/misc/NoexceptMoveConstructorCheck.h +++ clang-tidy/misc/NoexceptMoveConstructorCheck.h @@ -1,38 +0,0 @@ -//===--- NoexceptMoveConstructorCheck.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_MISC_NOEXCEPTMOVECONSTRUCTORCHECK_H -#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_NOEXCEPTMOVECONSTRUCTORCHECK_H - -#include "../ClangTidy.h" - -namespace clang { -namespace tidy { -namespace misc { - -/// The check flags user-defined move constructors and assignment operators not -/// marked with `noexcept` or marked with `noexcept(expr)` where `expr` -/// evaluates to `false` (but is not a `false` literal itself). -/// -/// Move constructors of all the types used with STL containers, for example, -/// need to be declared `noexcept`. Otherwise STL will choose copy constructors -/// instead. The same is valid for move assignment operations. -class NoexceptMoveConstructorCheck : public ClangTidyCheck { -public: - NoexceptMoveConstructorCheck(StringRef Name, ClangTidyContext *Context) - : ClangTidyCheck(Name, Context) {} - void registerMatchers(ast_matchers::MatchFinder *Finder) override; - void check(const ast_matchers::MatchFinder::MatchResult &Result) override; -}; - -} // namespace misc -} // namespace tidy -} // namespace clang - -#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_NOEXCEPTMOVECONSTRUCTORCHECK_H Index: clang-tidy/misc/NoexceptMoveConstructorCheck.cpp =================================================================== --- clang-tidy/misc/NoexceptMoveConstructorCheck.cpp +++ clang-tidy/misc/NoexceptMoveConstructorCheck.cpp @@ -1,77 +0,0 @@ -//===--- NoexceptMoveConstructorCheck.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 "NoexceptMoveConstructorCheck.h" -#include "clang/AST/ASTContext.h" -#include "clang/ASTMatchers/ASTMatchFinder.h" - -using namespace clang::ast_matchers; - -namespace clang { -namespace tidy { -namespace misc { - -void NoexceptMoveConstructorCheck::registerMatchers(MatchFinder *Finder) { - // Only register the matchers for C++11; the functionality currently does not - // provide any benefit to other languages, despite being benign. - if (!getLangOpts().CPlusPlus11) - return; - - Finder->addMatcher( - cxxMethodDecl(anyOf(cxxConstructorDecl(), hasOverloadedOperatorName("=")), - unless(isImplicit()), unless(isDeleted())) - .bind("decl"), - this); -} - -void NoexceptMoveConstructorCheck::check( - const MatchFinder::MatchResult &Result) { - if (const auto *Decl = Result.Nodes.getNodeAs("decl")) { - StringRef MethodType = "assignment operator"; - if (const auto *Ctor = dyn_cast(Decl)) { - if (!Ctor->isMoveConstructor()) - return; - MethodType = "constructor"; - } else if (!Decl->isMoveAssignmentOperator()) { - return; - } - - const auto *ProtoType = Decl->getType()->getAs(); - - if (isUnresolvedExceptionSpec(ProtoType->getExceptionSpecType())) - return; - - switch (ProtoType->getNoexceptSpec(*Result.Context)) { - case FunctionProtoType::NR_NoNoexcept: - diag(Decl->getLocation(), "move %0s should be marked noexcept") - << MethodType; - // FIXME: Add a fixit. - break; - case FunctionProtoType::NR_Throw: - // Don't complain about nothrow(false), but complain on nothrow(expr) - // where expr evaluates to false. - if (const Expr *E = ProtoType->getNoexceptExpr()) { - if (isa(E)) - break; - diag(E->getExprLoc(), - "noexcept specifier on the move %0 evaluates to 'false'") - << MethodType; - } - break; - case FunctionProtoType::NR_Nothrow: - case FunctionProtoType::NR_Dependent: - case FunctionProtoType::NR_BadNoexcept: - break; - } - } -} - -} // namespace misc -} // namespace tidy -} // namespace clang Index: clang-tidy/modernize/PassByValueCheck.cpp =================================================================== --- clang-tidy/modernize/PassByValueCheck.cpp +++ clang-tidy/modernize/PassByValueCheck.cpp @@ -187,7 +187,7 @@ return; // If the parameter is trivial to copy, don't move it. Moving a trivivally - // copyable type will cause a problem with misc-move-const-arg + // copyable type will cause a problem with performance-move-const-arg if (ParamDecl->getType().getNonReferenceType().isTriviallyCopyableType( *Result.Context)) return; Index: clang-tidy/performance/CMakeLists.txt =================================================================== --- clang-tidy/performance/CMakeLists.txt +++ clang-tidy/performance/CMakeLists.txt @@ -7,7 +7,9 @@ InefficientAlgorithmCheck.cpp InefficientStringConcatenationCheck.cpp InefficientVectorOperationCheck.cpp + MoveConstArgCheck.cpp MoveConstructorInitCheck.cpp + NoexceptMoveConstructorCheck.cpp PerformanceTidyModule.cpp TypePromotionInMathFnCheck.cpp UnnecessaryCopyInitialization.cpp Index: clang-tidy/performance/MoveConstArgCheck.h =================================================================== --- clang-tidy/performance/MoveConstArgCheck.h +++ clang-tidy/performance/MoveConstArgCheck.h @@ -0,0 +1,43 @@ +//===--- MoveConstArgCheck.h - clang-tidy -------------------------===// +// +// 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_MISC_MOVECONSTANTARGUMENTCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_MOVECONSTANTARGUMENTCHECK_H + +#include "../ClangTidy.h" + +namespace clang { +namespace tidy { +namespace performance { + +/// Find casts of calculation results to bigger type. Typically from int to +/// +/// There is one option: +/// +/// - `CheckTriviallyCopyableMove`: Whether to check for trivially-copyable +// types as their objects are not moved but copied. Enabled by default. +class MoveConstArgCheck : public ClangTidyCheck { +public: + MoveConstArgCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context), + CheckTriviallyCopyableMove( + Options.get("CheckTriviallyCopyableMove", true)) {} + void storeOptions(ClangTidyOptions::OptionMap &Opts) override; + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + +private: + const bool CheckTriviallyCopyableMove; +}; + +} // namespace performance +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_MOVECONSTANTARGUMENTCHECK_H Index: clang-tidy/performance/MoveConstArgCheck.cpp =================================================================== --- clang-tidy/performance/MoveConstArgCheck.cpp +++ clang-tidy/performance/MoveConstArgCheck.cpp @@ -0,0 +1,121 @@ +//===--- MoveConstArgCheck.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 "MoveConstArgCheck.h" + +#include "clang/Lex/Lexer.h" + +using namespace clang::ast_matchers; + +namespace clang { +namespace tidy { +namespace performance { + +static void ReplaceCallWithArg(const CallExpr *Call, DiagnosticBuilder &Diag, + const SourceManager &SM, + const LangOptions &LangOpts) { + const Expr *Arg = Call->getArg(0); + + CharSourceRange BeforeArgumentsRange = Lexer::makeFileCharRange( + CharSourceRange::getCharRange(Call->getLocStart(), Arg->getLocStart()), + SM, LangOpts); + CharSourceRange AfterArgumentsRange = Lexer::makeFileCharRange( + CharSourceRange::getCharRange(Call->getLocEnd(), + Call->getLocEnd().getLocWithOffset(1)), + SM, LangOpts); + + if (BeforeArgumentsRange.isValid() && AfterArgumentsRange.isValid()) { + Diag << FixItHint::CreateRemoval(BeforeArgumentsRange) + << FixItHint::CreateRemoval(AfterArgumentsRange); + } +} + +void MoveConstArgCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "CheckTriviallyCopyableMove", CheckTriviallyCopyableMove); +} + +void MoveConstArgCheck::registerMatchers(MatchFinder *Finder) { + if (!getLangOpts().CPlusPlus) + return; + + auto MoveCallMatcher = + callExpr(callee(functionDecl(hasName("::std::move"))), argumentCountIs(1), + unless(isInTemplateInstantiation())) + .bind("call-move"); + + Finder->addMatcher(MoveCallMatcher, this); + + auto ConstParamMatcher = forEachArgumentWithParam( + MoveCallMatcher, parmVarDecl(hasType(references(isConstQualified())))); + + Finder->addMatcher(callExpr(ConstParamMatcher).bind("receiving-expr"), this); + Finder->addMatcher(cxxConstructExpr(ConstParamMatcher).bind("receiving-expr"), + this); +} + +void MoveConstArgCheck::check(const MatchFinder::MatchResult &Result) { + const auto *CallMove = Result.Nodes.getNodeAs("call-move"); + const auto *ReceivingExpr = Result.Nodes.getNodeAs("receiving-expr"); + const Expr *Arg = CallMove->getArg(0); + SourceManager &SM = Result.Context->getSourceManager(); + + CharSourceRange MoveRange = + CharSourceRange::getCharRange(CallMove->getSourceRange()); + CharSourceRange FileMoveRange = + Lexer::makeFileCharRange(MoveRange, SM, getLangOpts()); + if (!FileMoveRange.isValid()) + return; + + bool IsConstArg = Arg->getType().isConstQualified(); + bool IsTriviallyCopyable = + Arg->getType().isTriviallyCopyableType(*Result.Context); + + if (IsConstArg || IsTriviallyCopyable) { + if (const CXXRecordDecl *R = Arg->getType()->getAsCXXRecordDecl()) { + // According to [expr.prim.lambda]p3, "whether the closure type is + // trivially copyable" property can be changed by the implementation of + // the language, so we shouldn't rely on it when issuing diagnostics. + if (R->isLambda()) + return; + // Don't warn when the type is not copyable. + for (const auto *Ctor : R->ctors()) { + if (Ctor->isCopyConstructor() && Ctor->isDeleted()) + return; + } + } + + if (!IsConstArg && IsTriviallyCopyable && !CheckTriviallyCopyableMove) + return; + + bool IsVariable = isa(Arg); + const auto *Var = + IsVariable ? dyn_cast(Arg)->getDecl() : nullptr; + auto Diag = diag(FileMoveRange.getBegin(), + "std::move of the %select{|const }0" + "%select{expression|variable %4}1 " + "%select{|of the trivially-copyable type %5 }2" + "has no effect; remove std::move()" + "%select{| or make the variable non-const}3") + << IsConstArg << IsVariable << IsTriviallyCopyable + << (IsConstArg && IsVariable && !IsTriviallyCopyable) << Var + << Arg->getType(); + + ReplaceCallWithArg(CallMove, Diag, SM, getLangOpts()); + } else if (ReceivingExpr) { + auto Diag = diag(FileMoveRange.getBegin(), + "passing result of std::move() as a const reference " + "argument; no move will actually happen"); + + ReplaceCallWithArg(CallMove, Diag, SM, getLangOpts()); + } +} + +} // namespace performance +} // namespace tidy +} // namespace clang Index: clang-tidy/performance/NoexceptMoveConstructorCheck.h =================================================================== --- clang-tidy/performance/NoexceptMoveConstructorCheck.h +++ clang-tidy/performance/NoexceptMoveConstructorCheck.h @@ -0,0 +1,38 @@ +//===--- NoexceptMoveConstructorCheck.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_PERFORMANCE_NOEXCEPTMOVECONSTRUCTORCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_NOEXCEPTMOVECONSTRUCTORCHECK_H + +#include "../ClangTidy.h" + +namespace clang { +namespace tidy { +namespace performance { + +/// The check flags user-defined move constructors and assignment operators not +/// marked with `noexcept` or marked with `noexcept(expr)` where `expr` +/// evaluates to `false` (but is not a `false` literal itself). +/// +/// Move constructors of all the types used with STL containers, for example, +/// need to be declared `noexcept`. Otherwise STL will choose copy constructors +/// instead. The same is valid for move assignment operations. +class NoexceptMoveConstructorCheck : public ClangTidyCheck { +public: + NoexceptMoveConstructorCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; +}; + +} // namespace performance +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_NOEXCEPTMOVECONSTRUCTORCHECK_H Index: clang-tidy/performance/NoexceptMoveConstructorCheck.cpp =================================================================== --- clang-tidy/performance/NoexceptMoveConstructorCheck.cpp +++ clang-tidy/performance/NoexceptMoveConstructorCheck.cpp @@ -0,0 +1,77 @@ +//===--- NoexceptMoveConstructorCheck.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 "NoexceptMoveConstructorCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" + +using namespace clang::ast_matchers; + +namespace clang { +namespace tidy { +namespace performance { + +void NoexceptMoveConstructorCheck::registerMatchers(MatchFinder *Finder) { + // Only register the matchers for C++11; the functionality currently does not + // provide any benefit to other languages, despite being benign. + if (!getLangOpts().CPlusPlus11) + return; + + Finder->addMatcher( + cxxMethodDecl(anyOf(cxxConstructorDecl(), hasOverloadedOperatorName("=")), + unless(isImplicit()), unless(isDeleted())) + .bind("decl"), + this); +} + +void NoexceptMoveConstructorCheck::check( + const MatchFinder::MatchResult &Result) { + if (const auto *Decl = Result.Nodes.getNodeAs("decl")) { + StringRef MethodType = "assignment operator"; + if (const auto *Ctor = dyn_cast(Decl)) { + if (!Ctor->isMoveConstructor()) + return; + MethodType = "constructor"; + } else if (!Decl->isMoveAssignmentOperator()) { + return; + } + + const auto *ProtoType = Decl->getType()->getAs(); + + if (isUnresolvedExceptionSpec(ProtoType->getExceptionSpecType())) + return; + + switch (ProtoType->getNoexceptSpec(*Result.Context)) { + case FunctionProtoType::NR_NoNoexcept: + diag(Decl->getLocation(), "move %0s should be marked noexcept") + << MethodType; + // FIXME: Add a fixit. + break; + case FunctionProtoType::NR_Throw: + // Don't complain about nothrow(false), but complain on nothrow(expr) + // where expr evaluates to false. + if (const Expr *E = ProtoType->getNoexceptExpr()) { + if (isa(E)) + break; + diag(E->getExprLoc(), + "noexcept specifier on the move %0 evaluates to 'false'") + << MethodType; + } + break; + case FunctionProtoType::NR_Nothrow: + case FunctionProtoType::NR_Dependent: + case FunctionProtoType::NR_BadNoexcept: + break; + } + } +} + +} // namespace performance +} // namespace tidy +} // namespace clang Index: clang-tidy/performance/PerformanceTidyModule.cpp =================================================================== --- clang-tidy/performance/PerformanceTidyModule.cpp +++ clang-tidy/performance/PerformanceTidyModule.cpp @@ -16,7 +16,9 @@ #include "InefficientAlgorithmCheck.h" #include "InefficientStringConcatenationCheck.h" #include "InefficientVectorOperationCheck.h" +#include "MoveConstArgCheck.h" #include "MoveConstructorInitCheck.h" +#include "NoexceptMoveConstructorCheck.h" #include "TypePromotionInMathFnCheck.h" #include "UnnecessaryCopyInitialization.h" #include "UnnecessaryValueParamCheck.h" @@ -40,8 +42,12 @@ "performance-inefficient-string-concatenation"); CheckFactories.registerCheck( "performance-inefficient-vector-operation"); + CheckFactories.registerCheck( + "performance-move-const-arg"); CheckFactories.registerCheck( "performance-move-constructor-init"); + CheckFactories.registerCheck( + "performance-noexcept-move-constructor"); CheckFactories.registerCheck( "performance-type-promotion-in-math-fn"); CheckFactories.registerCheck( Index: docs/ReleaseNotes.rst =================================================================== --- docs/ReleaseNotes.rst +++ docs/ReleaseNotes.rst @@ -62,6 +62,12 @@ Add new check to detect the use of OSSpinlock. +- The 'misc-move-const-arg' check was renamed to `performance-move-const-arg + `_ + +- The 'misc-noexcept-move-constructor' check was renamed to `performance-noexcept-move-constructor + `_ + - The 'misc-move-constructor-init' check was renamed to `performance-move-constructor-init `_ Index: docs/clang-tidy/checks/hicpp-move-const-arg.rst =================================================================== --- docs/clang-tidy/checks/hicpp-move-const-arg.rst +++ docs/clang-tidy/checks/hicpp-move-const-arg.rst @@ -1,10 +1,10 @@ .. title:: clang-tidy - hicpp-move-const-arg .. meta:: - :http-equiv=refresh: 5;URL=misc-move-const-arg.html + :http-equiv=refresh: 5;URL=performance-move-const-arg.html hicpp-move-const-arg ==================== The `hicpp-move-const-arg` check is an alias, please see -`misc-move-const-arg `_ for more information. +`performance-move-const-arg `_ for more information. It enforces the `rule 17.3.1 `_. Index: docs/clang-tidy/checks/list.rst =================================================================== --- docs/clang-tidy/checks/list.rst +++ docs/clang-tidy/checks/list.rst @@ -93,7 +93,7 @@ hicpp-function-size (redirects to readability-function-size) hicpp-invalid-access-moved (redirects to bugprone-use-after-move) hicpp-member-init (redirects to cppcoreguidelines-pro-type-member-init) - hicpp-move-const-arg (redirects to misc-move-const-arg) + hicpp-move-const-arg (redirects to performance-move-const-arg) hicpp-named-parameter (redirects to readability-named-parameter) hicpp-new-delete-operators (redirects to misc-new-delete-overloads) hicpp-no-array-decay (redirects to cppcoreguidelines-pro-bounds-array-to-pointer-decay) @@ -124,9 +124,7 @@ misc-macro-repeated-side-effects misc-misplaced-const misc-misplaced-widening-cast - misc-move-const-arg misc-new-delete-overloads - misc-noexcept-move-constructor misc-non-copyable-objects misc-redundant-expression misc-sizeof-container @@ -183,7 +181,9 @@ performance-inefficient-algorithm performance-inefficient-string-concatenation performance-inefficient-vector-operation + performance-move-const-arg performance-move-constructor-init + performance-noexcept-move-constructor performance-type-promotion-in-math-fn performance-unnecessary-copy-initialization performance-unnecessary-value-param Index: docs/clang-tidy/checks/misc-move-const-arg.rst =================================================================== --- docs/clang-tidy/checks/misc-move-const-arg.rst +++ docs/clang-tidy/checks/misc-move-const-arg.rst @@ -1,37 +0,0 @@ -.. title:: clang-tidy - misc-move-const-arg - -misc-move-const-arg -=================== - -The check warns - -- if ``std::move()`` is called with a constant argument, - -- if ``std::move()`` is called with an argument of a trivially-copyable type, - -- if the result of ``std::move()`` is passed as a const reference argument. - -In all three cases, the check will suggest a fix that removes the -``std::move()``. - -Here are examples of each of the three cases: - -.. code-block:: c++ - - const string s; - return std::move(s); // Warning: std::move of the const variable has no effect - - int x; - return std::move(x); // Warning: std::move of the variable of a trivially-copyable type has no effect - - void f(const string &s); - string s; - f(std::move(s)); // Warning: passing result of std::move as a const reference argument; no move will actually happen - -Options -------- - -.. option:: CheckTriviallyCopyableMove - - If non-zero, enables detection of trivially copyable types that do not - have a move constructor. Default is non-zero. Index: docs/clang-tidy/checks/misc-noexcept-move-constructor.rst =================================================================== --- docs/clang-tidy/checks/misc-noexcept-move-constructor.rst +++ docs/clang-tidy/checks/misc-noexcept-move-constructor.rst @@ -1,13 +0,0 @@ -.. title:: clang-tidy - misc-noexcept-move-constructor - -misc-noexcept-move-constructor -============================== - - -The check flags user-defined move constructors and assignment operators not -marked with ``noexcept`` or marked with ``noexcept(expr)`` where ``expr`` -evaluates to ``false`` (but is not a ``false`` literal itself). - -Move constructors of all the types used with STL containers, for example, -need to be declared ``noexcept``. Otherwise STL will choose copy constructors -instead. The same is valid for move assignment operations. Index: docs/clang-tidy/checks/performance-move-const-arg.rst =================================================================== --- docs/clang-tidy/checks/performance-move-const-arg.rst +++ docs/clang-tidy/checks/performance-move-const-arg.rst @@ -0,0 +1,37 @@ +.. title:: clang-tidy - performance-move-const-arg + +performance-move-const-arg +========================== + +The check warns + +- if ``std::move()`` is called with a constant argument, + +- if ``std::move()`` is called with an argument of a trivially-copyable type, + +- if the result of ``std::move()`` is passed as a const reference argument. + +In all three cases, the check will suggest a fix that removes the +``std::move()``. + +Here are examples of each of the three cases: + +.. code-block:: c++ + + const string s; + return std::move(s); // Warning: std::move of the const variable has no effect + + int x; + return std::move(x); // Warning: std::move of the variable of a trivially-copyable type has no effect + + void f(const string &s); + string s; + f(std::move(s)); // Warning: passing result of std::move as a const reference argument; no move will actually happen + +Options +------- + +.. option:: CheckTriviallyCopyableMove + + If non-zero, enables detection of trivially copyable types that do not + have a move constructor. Default is non-zero. Index: docs/clang-tidy/checks/performance-noexcept-move-constructor.rst =================================================================== --- docs/clang-tidy/checks/performance-noexcept-move-constructor.rst +++ docs/clang-tidy/checks/performance-noexcept-move-constructor.rst @@ -0,0 +1,13 @@ +.. title:: clang-tidy - performance-noexcept-move-constructor + +performance-noexcept-move-constructor +===================================== + + +The check flags user-defined move constructors and assignment operators not +marked with ``noexcept`` or marked with ``noexcept(expr)`` where ``expr`` +evaluates to ``false`` (but is not a ``false`` literal itself). + +Move constructors of all the types used with STL containers, for example, +need to be declared ``noexcept``. Otherwise STL will choose copy constructors +instead. The same is valid for move assignment operations. Index: test/clang-tidy/misc-move-const-arg-trivially-copyable.cpp =================================================================== --- test/clang-tidy/misc-move-const-arg-trivially-copyable.cpp +++ test/clang-tidy/misc-move-const-arg-trivially-copyable.cpp @@ -1,98 +0,0 @@ -// RUN: %check_clang_tidy %s misc-move-const-arg %t \ -// RUN: -config='{CheckOptions: \ -// RUN: [{key: misc-move-const-arg.CheckTriviallyCopyableMove, value: 0}]}' \ -// RUN: -- -std=c++14 - -namespace std { - -template struct remove_reference; -template struct remove_reference { typedef _Tp type; }; -template struct remove_reference<_Tp &> { typedef _Tp type; }; -template struct remove_reference<_Tp &&> { typedef _Tp type; }; - -template -constexpr typename std::remove_reference<_Tp>::type &&move(_Tp &&__t) { - return static_cast::type &&>(__t); -} - -template -constexpr _Tp && -forward(typename remove_reference<_Tp>::type &__t) noexcept { - return static_cast<_Tp &&>(__t); -} - -} // namespace std - -class NoMoveSemantics { - public: - NoMoveSemantics(); - NoMoveSemantics(const NoMoveSemantics &); - - NoMoveSemantics &operator=(const NoMoveSemantics &); -}; - -void callByConstRef(const NoMoveSemantics &); -void callByConstRef(int i, const NoMoveSemantics &); - -void moveToConstReferencePositives() { - NoMoveSemantics obj; - - // Basic case. It is here just to have a single "detected and fixed" case. - callByConstRef(std::move(obj)); - // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: passing result of std::move() as a const reference argument; no move will actually happen [misc-move-const-arg] - // CHECK-FIXES: callByConstRef(obj); -} - -struct TriviallyCopyable { - int i; -}; - -void f(TriviallyCopyable) {} - -void g() { - TriviallyCopyable obj; - f(std::move(obj)); -} - -class MoveSemantics { - public: - MoveSemantics(); - MoveSemantics(MoveSemantics &&); - - MoveSemantics &operator=(MoveSemantics &&); -}; - -void fmovable(MoveSemantics); - -void lambda1() { - auto f = [](MoveSemantics m) { - fmovable(std::move(m)); - }; - f(MoveSemantics()); -} - -template struct function {}; - -template -class function { -public: - function() = default; - void operator()(Args... args) const { - fmovable(std::forward(args)...); - } -}; - -void functionInvocation() { - function callback; - MoveSemantics m; - callback(std::move(m)); -} - -void lambda2() { - function callback; - - auto f = [callback = std::move(callback)](MoveSemantics m) mutable { - callback(std::move(m)); - }; - f(MoveSemantics()); -} Index: test/clang-tidy/misc-move-const-arg.cpp =================================================================== --- test/clang-tidy/misc-move-const-arg.cpp +++ test/clang-tidy/misc-move-const-arg.cpp @@ -1,234 +0,0 @@ -// RUN: %check_clang_tidy %s misc-move-const-arg %t - -namespace std { -template struct remove_reference; - -template struct remove_reference { typedef _Tp type; }; - -template struct remove_reference<_Tp &> { typedef _Tp type; }; - -template struct remove_reference<_Tp &&> { typedef _Tp type; }; - -template -constexpr typename std::remove_reference<_Tp>::type &&move(_Tp &&__t) { - return static_cast::type &&>(__t); -} - -template -constexpr _Tp && -forward(typename remove_reference<_Tp>::type &__t) noexcept { - return static_cast<_Tp &&>(__t); -} - -} // namespace std - -class A { -public: - A() {} - A(const A &rhs) {} - A(A &&rhs) {} -}; - -struct TriviallyCopyable { - int i; -}; - -void f(TriviallyCopyable) {} - -void g() { - TriviallyCopyable obj; - f(std::move(obj)); - // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: std::move of the variable 'obj' of the trivially-copyable type 'TriviallyCopyable' has no effect; remove std::move() [misc-move-const-arg] - // CHECK-FIXES: f(obj); -} - -int f1() { - return std::move(42); - // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: std::move of the expression of the trivially-copyable type 'int' has no effect; remove std::move() [misc-move-const-arg] - // CHECK-FIXES: return 42; -} - -int f2(int x2) { - return std::move(x2); - // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: std::move of the variable 'x2' of the trivially-copyable type 'int' - // CHECK-FIXES: return x2; -} - -int *f3(int *x3) { - return std::move(x3); - // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: std::move of the variable 'x3' of the trivially-copyable type 'int *' - // CHECK-FIXES: return x3; -} - -A f4(A x4) { return std::move(x4); } - -A f5(const A x5) { - return std::move(x5); - // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: std::move of the const variable 'x5' has no effect; remove std::move() or make the variable non-const [misc-move-const-arg] - // CHECK-FIXES: return x5; -} - -template T f6(const T x6) { return std::move(x6); } - -void f7() { int a = f6(10); } - -#define M1(x) x -void f8() { - const A a; - M1(A b = std::move(a);) - // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: std::move of the const variable 'a' has no effect; remove std::move() or make the variable non-const - // CHECK-FIXES: M1(A b = a;) -} - -#define M2(x) std::move(x) -int f9() { return M2(1); } - -template T f10(const int x10) { - return std::move(x10); - // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: std::move of the const variable 'x10' of the trivially-copyable type 'const int' has no effect; remove std::move() [misc-move-const-arg] - // CHECK-FIXES: return x10; -} -void f11() { - f10(1); - f10(1); -} - -class NoMoveSemantics { - public: - NoMoveSemantics(); - NoMoveSemantics(const NoMoveSemantics &); - - NoMoveSemantics &operator=(const NoMoveSemantics &); -}; - -void callByConstRef(const NoMoveSemantics &); -void callByConstRef(int i, const NoMoveSemantics &); - -void moveToConstReferencePositives() { - NoMoveSemantics obj; - - // Basic case. - callByConstRef(std::move(obj)); - // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: passing result of std::move() as - // CHECK-FIXES: callByConstRef(obj); - - // Also works for second argument. - callByConstRef(1, std::move(obj)); - // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: passing result of std::move() as - // CHECK-FIXES: callByConstRef(1, obj); - - // Works if std::move() applied to a temporary. - callByConstRef(std::move(NoMoveSemantics())); - // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: passing result of std::move() as - // CHECK-FIXES: callByConstRef(NoMoveSemantics()); - - // Works if calling a copy constructor. - NoMoveSemantics other(std::move(obj)); - // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: passing result of std::move() as - // CHECK-FIXES: NoMoveSemantics other(obj); - - // Works if calling assignment operator. - other = std::move(obj); - // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: passing result of std::move() as - // CHECK-FIXES: other = obj; -} - -class MoveSemantics { - public: - MoveSemantics(); - MoveSemantics(MoveSemantics &&); - - MoveSemantics &operator=(MoveSemantics &&); -}; - -void callByValue(MoveSemantics); - -void callByRValueRef(MoveSemantics &&); - -template -void templateFunction(T obj) { - T other = std::move(obj); -} - -#define M3(T, obj) \ - do { \ - T other = std::move(obj); \ - } while (true) - -#define CALL(func) (func)() - -void moveToConstReferenceNegatives() { - // No warning when actual move takes place. - MoveSemantics move_semantics; - callByValue(std::move(move_semantics)); - callByRValueRef(std::move(move_semantics)); - MoveSemantics other(std::move(move_semantics)); - other = std::move(move_semantics); - - // No warning if std::move() not used. - NoMoveSemantics no_move_semantics; - callByConstRef(no_move_semantics); - - // No warning if instantiating a template. - templateFunction(no_move_semantics); - - // No warning inside of macro expansions. - M3(NoMoveSemantics, no_move_semantics); - - // No warning inside of macro expansion, even if the macro expansion is inside - // a lambda that is, in turn, an argument to a macro. - CALL([no_move_semantics] { M3(NoMoveSemantics, no_move_semantics); }); - - auto lambda = [] {}; - auto lambda2 = std::move(lambda); -} - -class MoveOnly { -public: - MoveOnly(const MoveOnly &other) = delete; - MoveOnly &operator=(const MoveOnly &other) = delete; - MoveOnly(MoveOnly &&other) = default; - MoveOnly &operator=(MoveOnly &&other) = default; -}; -template -void Q(T); -void moveOnlyNegatives(MoveOnly val) { - Q(std::move(val)); -} - -void fmovable(MoveSemantics); - -void lambda1() { - auto f = [](MoveSemantics m) { - fmovable(std::move(m)); - }; - f(MoveSemantics()); -} - -template struct function {}; - -template -class function { -public: - function() = default; - void operator()(Args... args) const { - fmovable(std::forward(args)...); - } -}; - -void functionInvocation() { - function callback; - MoveSemantics m; - callback(std::move(m)); -} - -void lambda2() { - function callback; - - auto f = [callback = std::move(callback)](MoveSemantics m) mutable { - // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: std::move of the variable 'callback' of the trivially-copyable type 'function' has no effect; remove std::move() - // CHECK-FIXES: auto f = [callback = callback](MoveSemantics m) mutable { - callback(std::move(m)); - }; - f(MoveSemantics()); -} Index: test/clang-tidy/misc-noexcept-move-constructor.cpp =================================================================== --- test/clang-tidy/misc-noexcept-move-constructor.cpp +++ test/clang-tidy/misc-noexcept-move-constructor.cpp @@ -1,54 +0,0 @@ -// RUN: %check_clang_tidy %s misc-noexcept-move-constructor %t - -class A { - A(A &&); - // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: move constructors should be marked noexcept [misc-noexcept-move-constructor] - A &operator=(A &&); - // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: move assignment operators should -}; - -struct B { - static constexpr bool kFalse = false; - B(B &&) noexcept(kFalse); - // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: noexcept specifier on the move constructor evaluates to 'false' [misc-noexcept-move-constructor] -}; - -class OK {}; - -void f() { - OK a; - a = OK(); -} - -class OK1 { - public: - OK1(); - OK1(const OK1 &); - OK1(OK1&&) noexcept; - OK1 &operator=(OK1 &&) noexcept; - void f(); - void g() noexcept; -}; - -class OK2 { - static constexpr bool kTrue = true; - -public: - OK2(OK2 &&) noexcept(true) {} - OK2 &operator=(OK2 &&) noexcept(kTrue) { return *this; } -}; - -struct OK3 { - OK3(OK3 &&) noexcept(false) {} - OK3 &operator=(OK3 &&) = delete; -}; - -struct OK4 { - OK4(OK4 &&) noexcept = default; - OK4 &operator=(OK4 &&) noexcept = default; -}; - -struct OK5 { - OK5(OK5 &&) noexcept(true) = default; - OK5 &operator=(OK5 &&) noexcept(true) = default; -}; Index: test/clang-tidy/modernize-pass-by-value.cpp =================================================================== --- test/clang-tidy/modernize-pass-by-value.cpp +++ test/clang-tidy/modernize-pass-by-value.cpp @@ -202,7 +202,7 @@ template struct array { T A[N]; }; // Test that types that are trivially copyable will not use std::move. This will -// cause problems with misc-move-const-arg, as it will revert it. +// cause problems with performance-move-const-arg, as it will revert it. struct T { T(array a) : a_(a) {} // CHECK-FIXES: T(array a) : a_(a) {} Index: test/clang-tidy/performance-move-const-arg-trivially-copyable.cpp =================================================================== --- test/clang-tidy/performance-move-const-arg-trivially-copyable.cpp +++ test/clang-tidy/performance-move-const-arg-trivially-copyable.cpp @@ -0,0 +1,98 @@ +// RUN: %check_clang_tidy %s performance-move-const-arg %t \ +// RUN: -config='{CheckOptions: \ +// RUN: [{key: performance-move-const-arg.CheckTriviallyCopyableMove, value: 0}]}' \ +// RUN: -- -std=c++14 + +namespace std { + +template struct remove_reference; +template struct remove_reference { typedef _Tp type; }; +template struct remove_reference<_Tp &> { typedef _Tp type; }; +template struct remove_reference<_Tp &&> { typedef _Tp type; }; + +template +constexpr typename std::remove_reference<_Tp>::type &&move(_Tp &&__t) { + return static_cast::type &&>(__t); +} + +template +constexpr _Tp && +forward(typename remove_reference<_Tp>::type &__t) noexcept { + return static_cast<_Tp &&>(__t); +} + +} // namespace std + +class NoMoveSemantics { + public: + NoMoveSemantics(); + NoMoveSemantics(const NoMoveSemantics &); + + NoMoveSemantics &operator=(const NoMoveSemantics &); +}; + +void callByConstRef(const NoMoveSemantics &); +void callByConstRef(int i, const NoMoveSemantics &); + +void moveToConstReferencePositives() { + NoMoveSemantics obj; + + // Basic case. It is here just to have a single "detected and fixed" case. + callByConstRef(std::move(obj)); + // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: passing result of std::move() as a const reference argument; no move will actually happen [performance-move-const-arg] + // CHECK-FIXES: callByConstRef(obj); +} + +struct TriviallyCopyable { + int i; +}; + +void f(TriviallyCopyable) {} + +void g() { + TriviallyCopyable obj; + f(std::move(obj)); +} + +class MoveSemantics { + public: + MoveSemantics(); + MoveSemantics(MoveSemantics &&); + + MoveSemantics &operator=(MoveSemantics &&); +}; + +void fmovable(MoveSemantics); + +void lambda1() { + auto f = [](MoveSemantics m) { + fmovable(std::move(m)); + }; + f(MoveSemantics()); +} + +template struct function {}; + +template +class function { +public: + function() = default; + void operator()(Args... args) const { + fmovable(std::forward(args)...); + } +}; + +void functionInvocation() { + function callback; + MoveSemantics m; + callback(std::move(m)); +} + +void lambda2() { + function callback; + + auto f = [callback = std::move(callback)](MoveSemantics m) mutable { + callback(std::move(m)); + }; + f(MoveSemantics()); +} Index: test/clang-tidy/performance-move-const-arg.cpp =================================================================== --- test/clang-tidy/performance-move-const-arg.cpp +++ test/clang-tidy/performance-move-const-arg.cpp @@ -0,0 +1,248 @@ +// RUN: %check_clang_tidy %s performance-move-const-arg %t + +namespace std { +template +struct remove_reference; + +template +struct remove_reference { + typedef _Tp type; +}; + +template +struct remove_reference<_Tp &> { + typedef _Tp type; +}; + +template +struct remove_reference<_Tp &&> { + typedef _Tp type; +}; + +template +constexpr typename std::remove_reference<_Tp>::type &&move(_Tp &&__t) { + return static_cast::type &&>(__t); +} + +template +constexpr _Tp && +forward(typename remove_reference<_Tp>::type &__t) noexcept { + return static_cast<_Tp &&>(__t); +} + +} // namespace std + +class A { +public: + A() {} + A(const A &rhs) {} + A(A &&rhs) {} +}; + +struct TriviallyCopyable { + int i; +}; + +void f(TriviallyCopyable) {} + +void g() { + TriviallyCopyable obj; + f(std::move(obj)); + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: std::move of the variable 'obj' of the trivially-copyable type 'TriviallyCopyable' has no effect; remove std::move() [performance-move-const-arg] + // CHECK-FIXES: f(obj); +} + +int f1() { + return std::move(42); + // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: std::move of the expression of the trivially-copyable type 'int' has no effect; remove std::move() [performance-move-const-arg] + // CHECK-FIXES: return 42; +} + +int f2(int x2) { + return std::move(x2); + // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: std::move of the variable 'x2' of the trivially-copyable type 'int' + // CHECK-FIXES: return x2; +} + +int *f3(int *x3) { + return std::move(x3); + // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: std::move of the variable 'x3' of the trivially-copyable type 'int *' + // CHECK-FIXES: return x3; +} + +A f4(A x4) { return std::move(x4); } + +A f5(const A x5) { + return std::move(x5); + // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: std::move of the const variable 'x5' has no effect; remove std::move() or make the variable non-const [performance-move-const-arg] + // CHECK-FIXES: return x5; +} + +template +T f6(const T x6) { + return std::move(x6); +} + +void f7() { int a = f6(10); } + +#define M1(x) x +void f8() { + const A a; + M1(A b = std::move(a);) + // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: std::move of the const variable 'a' has no effect; remove std::move() or make the variable non-const + // CHECK-FIXES: M1(A b = a;) +} + +#define M2(x) std::move(x) +int f9() { return M2(1); } + +template +T f10(const int x10) { + return std::move(x10); + // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: std::move of the const variable 'x10' of the trivially-copyable type 'const int' has no effect; remove std::move() [performance-move-const-arg] + // CHECK-FIXES: return x10; +} +void f11() { + f10(1); + f10(1); +} + +class NoMoveSemantics { +public: + NoMoveSemantics(); + NoMoveSemantics(const NoMoveSemantics &); + + NoMoveSemantics &operator=(const NoMoveSemantics &); +}; + +void callByConstRef(const NoMoveSemantics &); +void callByConstRef(int i, const NoMoveSemantics &); + +void moveToConstReferencePositives() { + NoMoveSemantics obj; + + // Basic case. + callByConstRef(std::move(obj)); + // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: passing result of std::move() as + // CHECK-FIXES: callByConstRef(obj); + + // Also works for second argument. + callByConstRef(1, std::move(obj)); + // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: passing result of std::move() as + // CHECK-FIXES: callByConstRef(1, obj); + + // Works if std::move() applied to a temporary. + callByConstRef(std::move(NoMoveSemantics())); + // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: passing result of std::move() as + // CHECK-FIXES: callByConstRef(NoMoveSemantics()); + + // Works if calling a copy constructor. + NoMoveSemantics other(std::move(obj)); + // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: passing result of std::move() as + // CHECK-FIXES: NoMoveSemantics other(obj); + + // Works if calling assignment operator. + other = std::move(obj); + // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: passing result of std::move() as + // CHECK-FIXES: other = obj; +} + +class MoveSemantics { +public: + MoveSemantics(); + MoveSemantics(MoveSemantics &&); + + MoveSemantics &operator=(MoveSemantics &&); +}; + +void callByValue(MoveSemantics); + +void callByRValueRef(MoveSemantics &&); + +template +void templateFunction(T obj) { + T other = std::move(obj); +} + +#define M3(T, obj) \ + do { \ + T other = std::move(obj); \ + } while (true) + +#define CALL(func) (func)() + +void moveToConstReferenceNegatives() { + // No warning when actual move takes place. + MoveSemantics move_semantics; + callByValue(std::move(move_semantics)); + callByRValueRef(std::move(move_semantics)); + MoveSemantics other(std::move(move_semantics)); + other = std::move(move_semantics); + + // No warning if std::move() not used. + NoMoveSemantics no_move_semantics; + callByConstRef(no_move_semantics); + + // No warning if instantiating a template. + templateFunction(no_move_semantics); + + // No warning inside of macro expansions. + M3(NoMoveSemantics, no_move_semantics); + + // No warning inside of macro expansion, even if the macro expansion is inside + // a lambda that is, in turn, an argument to a macro. + CALL([no_move_semantics] { M3(NoMoveSemantics, no_move_semantics); }); + + auto lambda = [] {}; + auto lambda2 = std::move(lambda); +} + +class MoveOnly { +public: + MoveOnly(const MoveOnly &other) = delete; + MoveOnly &operator=(const MoveOnly &other) = delete; + MoveOnly(MoveOnly &&other) = default; + MoveOnly &operator=(MoveOnly &&other) = default; +}; +template +void Q(T); +void moveOnlyNegatives(MoveOnly val) { + Q(std::move(val)); +} + +void fmovable(MoveSemantics); + +void lambda1() { + auto f = [](MoveSemantics m) { + fmovable(std::move(m)); + }; + f(MoveSemantics()); +} + +template struct function {}; + +template +class function { +public: + function() = default; + void operator()(Args... args) const { + fmovable(std::forward(args)...); + } +}; + +void functionInvocation() { + function callback; + MoveSemantics m; + callback(std::move(m)); +} + +void lambda2() { + function callback; + + auto f = [callback = std::move(callback)](MoveSemantics m) mutable { + // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: std::move of the variable 'callback' of the trivially-copyable type 'function' has no effect; remove std::move() + // CHECK-FIXES: auto f = [callback = callback](MoveSemantics m) mutable { + callback(std::move(m)); + }; + f(MoveSemantics()); +} Index: test/clang-tidy/performance-noexcept-move-constructor.cpp =================================================================== --- test/clang-tidy/performance-noexcept-move-constructor.cpp +++ test/clang-tidy/performance-noexcept-move-constructor.cpp @@ -0,0 +1,54 @@ +// RUN: %check_clang_tidy %s performance-noexcept-move-constructor %t + +class A { + A(A &&); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: move constructors should be marked noexcept [performance-noexcept-move-constructor] + A &operator=(A &&); + // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: move assignment operators should +}; + +struct B { + static constexpr bool kFalse = false; + B(B &&) noexcept(kFalse); + // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: noexcept specifier on the move constructor evaluates to 'false' [performance-noexcept-move-constructor] +}; + +class OK {}; + +void f() { + OK a; + a = OK(); +} + +class OK1 { +public: + OK1(); + OK1(const OK1 &); + OK1(OK1 &&) noexcept; + OK1 &operator=(OK1 &&) noexcept; + void f(); + void g() noexcept; +}; + +class OK2 { + static constexpr bool kTrue = true; + +public: + OK2(OK2 &&) noexcept(true) {} + OK2 &operator=(OK2 &&) noexcept(kTrue) { return *this; } +}; + +struct OK3 { + OK3(OK3 &&) noexcept(false) {} + OK3 &operator=(OK3 &&) = delete; +}; + +struct OK4 { + OK4(OK4 &&) noexcept = default; + OK4 &operator=(OK4 &&) noexcept = default; +}; + +struct OK5 { + OK5(OK5 &&) noexcept(true) = default; + OK5 &operator=(OK5 &&) noexcept(true) = default; +};