https://github.com/segoon created https://github.com/llvm/llvm-project/pull/228767
None >From da46f9e43552447cde4202167b7d51e37bdb0312 Mon Sep 17 00:00:00 2001 From: Vasily Kulikov <[email protected]> Date: Mon, 12 May 2025 13:05:43 +0300 Subject: [PATCH 1/3] [clang-tidy] Extend performance-use-std-move to copy construction Replace the standalone performance-lost-std-move check with an extension of performance-use-std-move. Match actual nontrivial copies and applicable ordinary moves, and cache control-flow, sequencing, alias, and escape analysis. Track fresh loop-local objects and independent nonthrowing restorations without identifying types or reset functions by name. Restrict fixes for overload, lifetime, exception, aliasing, template, and macro risks. Add utility insertion and configurable type exclusions. Document covered cases, advisory diagnostics, semantic assumptions, and remaining gaps with examples. Add regression coverage across C++11-23, including compilation of fixed output. Validation: assertion-enabled clang-tidy build; 59 performance and related move/pass-by-value lit tests passed. Excluded the unrelated module PCM test because the external compiler is Clang 17. Scanned 477 clang-tidy translation units without compiler errors or crashes, followed by a focused scan after final conservative guards. Checked real-library fixes, malformed-code recovery, formatting, and unused includes. --- .../performance/UseStdMoveCheck.cpp | 760 +++++++++++++++--- .../clang-tidy/performance/UseStdMoveCheck.h | 33 +- clang-tools-extra/docs/ReleaseNotes.md | 6 + .../checks/performance/use-std-move.md | 284 ++++++- .../performance/use-std-move-captures.cpp | 31 + .../performance/use-std-move-constrained.cpp | 53 ++ .../performance/use-std-move-construction.cpp | 419 ++++++++++ .../performance/use-std-move-flow.cpp | 361 +++++++++ .../performance/use-std-move-options.cpp | 51 ++ .../checkers/performance/use-std-move.cpp | 10 +- 10 files changed, 1858 insertions(+), 150 deletions(-) create mode 100644 clang-tools-extra/test/clang-tidy/checkers/performance/use-std-move-captures.cpp create mode 100644 clang-tools-extra/test/clang-tidy/checkers/performance/use-std-move-constrained.cpp create mode 100644 clang-tools-extra/test/clang-tidy/checkers/performance/use-std-move-construction.cpp create mode 100644 clang-tools-extra/test/clang-tidy/checkers/performance/use-std-move-flow.cpp create mode 100644 clang-tools-extra/test/clang-tidy/checkers/performance/use-std-move-options.cpp diff --git a/clang-tools-extra/clang-tidy/performance/UseStdMoveCheck.cpp b/clang-tools-extra/clang-tidy/performance/UseStdMoveCheck.cpp index e887a9862ca5f..c1138a7ee8b4f 100644 --- a/clang-tools-extra/clang-tidy/performance/UseStdMoveCheck.cpp +++ b/clang-tools-extra/clang-tidy/performance/UseStdMoveCheck.cpp @@ -7,34 +7,36 @@ //===----------------------------------------------------------------------===// #include "UseStdMoveCheck.h" - -#include "../utils/DeclRefExprUtils.h" - +#include "../utils/ExprSequence.h" +#include "../utils/Matchers.h" +#include "../utils/OptionsUtils.h" +#include "../utils/TypeTraits.h" +#include "clang/AST/ASTContext.h" +#include "clang/AST/Attr.h" #include "clang/AST/Expr.h" #include "clang/AST/ExprCXX.h" +#include "clang/AST/RecursiveASTVisitor.h" +#include "clang/AST/StmtCXX.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" #include "clang/ASTMatchers/ASTMatchers.h" -#include "clang/Analysis/Analyses/CFGReachabilityAnalysis.h" +#include "clang/ASTMatchers/ASTMatchersInternal.h" +#include "clang/ASTMatchers/ASTMatchersMacros.h" +#include "clang/Analysis/Analyses/ExprMutationAnalyzer.h" +#include "clang/Analysis/CFG.h" +#include "clang/Basic/LLVM.h" #include "clang/Lex/Lexer.h" -#include "llvm/ADT/DenseMap.h" +#include "llvm/ADT/ArrayRef.h" #include "llvm/ADT/STLExtras.h" +#include "llvm/ADT/SmallPtrSet.h" +#include "llvm/ADT/SmallVector.h" +#include <memory> +#include <utility> using namespace clang::ast_matchers; namespace clang::tidy::performance { namespace { -AST_MATCHER(CXXRecordDecl, hasAccessibleNonTrivialMoveAssignment) { - const CXXRecordDecl *ND = Node.getDefinition(); - if (!ND) - return false; - if (!ND->hasNonTrivialMoveAssignment()) - return false; - for (const CXXMethodDecl *CM : ND->methods()) - if (CM->isMoveAssignmentOperator()) - return !CM->isDeleted() && CM->getAccess() == AS_public; - llvm_unreachable("Move Assignment Operator Not Found"); -} - AST_MATCHER(QualType, isLValueReferenceType) { return Node->isLValueReferenceType(); } @@ -43,143 +45,645 @@ AST_MATCHER(DeclRefExpr, refersToEnclosingVariableOrCapture) { return Node.refersToEnclosingVariableOrCapture(); } +AST_MATCHER_P(Expr, hasValueSource, + ast_matchers::internal::Matcher<DeclRefExpr>, Inner) { + const Expr *Value = Node.IgnoreParenImpCasts(); + while (const auto *Comma = dyn_cast<BinaryOperator>(Value)) { + if (Comma->getOpcode() != BO_Comma) + break; + Value = Comma->getRHS()->IgnoreParenImpCasts(); + } + const auto *Reference = dyn_cast<DeclRefExpr>(Value); + return Reference && Inner.matches(*Reference, Finder, Builder); +} + AST_MATCHER(CXXOperatorCallExpr, isCopyAssignmentOperator) { if (const auto *MD = dyn_cast_or_null<CXXMethodDecl>(Node.getDirectCallee())) return MD->isCopyAssignmentOperator(); return false; } -// Ignore nodes inside macros. -AST_POLYMORPHIC_MATCHER(isInMacro, - AST_POLYMORPHIC_SUPPORTED_TYPES(Stmt, Decl)) { - return Node.getBeginLoc().isMacroID() || Node.getEndLoc().isMacroID(); -} } // namespace -using utils::decl_ref_expr::allDeclRefExprs; +// Follow wrappers so that, for example, a const reference initializer and a +// parenthesized reference initializer are treated just like `T &ref = value`. +static bool +mayEscape(const DeclRefExpr *Reference, ASTContext &Context, + const llvm::DenseMap<const VarDecl *, const VarDecl *> &Aliases) { + SmallVector<const Expr *, 4> WorkList{Reference}; + while (!WorkList.empty()) { + const Expr *Expression = WorkList.pop_back_val(); + for (const DynTypedNode &Parent : Context.getParents(*Expression)) { + if (const auto *Variable = Parent.get<VarDecl>()) { + if ((Variable->getType()->isReferenceType() && + !Aliases.contains(Variable)) || + Variable->getType()->isPointerType()) + return true; + continue; + } + const auto *ParentExpr = Parent.get<Expr>(); + if (!ParentExpr) + continue; + if (isa<LambdaExpr>(ParentExpr)) + return true; + if (const auto *Init = dyn_cast<InitListExpr>(ParentExpr)) { + // The syntactic form can reference the source directly even when the + // semantic form contains a value copy. Only the latter binds storage. + if (Init->getSemanticForm()) + continue; + // Parent maps can associate a syntactic operand with the semantic + // list, bypassing the intervening copy constructor. Only a direct + // glvalue or pointer element can retain access to the source. + if ((Expression->isGLValue() || + Expression->getType()->isPointerType()) && + llvm::any_of(Init->inits(), [&](const Expr *Element) { + return Element && Element->IgnoreParenImpCasts() == + Expression->IgnoreParenImpCasts(); + })) + return true; + continue; + } + if (isa<CXXThrowExpr, CXXNewExpr>(ParentExpr) && + Expression->getType()->isPointerType()) + return true; + if (const auto *Unary = dyn_cast<UnaryOperator>(ParentExpr); + Unary && Unary->getOpcode() == UO_AddrOf) { + WorkList.push_back(Unary); + continue; + } + if (const auto *Assignment = dyn_cast<BinaryOperator>(ParentExpr); + Assignment && Assignment->isAssignmentOp() && + Assignment->getRHS() == Expression && + Expression->getType()->isPointerType()) + return true; + if (isa<BinaryOperator, UnaryOperator>(ParentExpr) && + (ParentExpr->isGLValue() || ParentExpr->getType()->isPointerType())) { + WorkList.push_back(ParentExpr); + continue; + } + if (isa<ParenExpr, CastExpr, ExprWithCleanups, MaterializeTemporaryExpr, + CXXBindTemporaryExpr, MemberExpr, AbstractConditionalOperator>( + ParentExpr)) { + WorkList.push_back(ParentExpr); + continue; + } + if (const auto *Construction = dyn_cast<CXXConstructExpr>(ParentExpr)) { + const CXXConstructorDecl *Constructor = Construction->getConstructor(); + if (Constructor->isCopyConstructor()) + continue; + for (unsigned I = 0; I < Construction->getNumArgs(); ++I) + if (Construction->getArg(I) == Expression && + (Expression->getType()->isPointerType() || + I >= Constructor->getNumParams() || + Constructor->getParamDecl(I)->getType()->isReferenceType())) + return true; + } + if (const auto *Call = dyn_cast<CallExpr>(ParentExpr)) { + if (const auto *Operator = dyn_cast<CXXOperatorCallExpr>(Call); + Operator && Operator->getOperator() == OO_Amp && + Operator->getNumArgs() == 1) + return true; + if (const auto *MemberCall = dyn_cast<CXXMemberCallExpr>(Call); + MemberCall && + MemberCall->getImplicitObjectArgument()->IgnoreParenImpCasts() == + Reference && + (MemberCall->isGLValue() || MemberCall->getType()->isPointerType())) + WorkList.push_back(MemberCall); + const FunctionDecl *Callee = Call->getDirectCallee(); + if (const auto *Operator = dyn_cast<CXXOperatorCallExpr>(Call); + Operator && isa_and_nonnull<CXXMethodDecl>(Callee) && + Operator->getArg(0) == Expression && + (Operator->isGLValue() || Operator->getType()->isPointerType())) + WorkList.push_back(Operator); + if (const auto *Method = dyn_cast_or_null<CXXMethodDecl>(Callee); + Method && Method->isCopyAssignmentOperator()) + continue; + unsigned Offset = isa<CXXOperatorCallExpr>(Call) && Callee && + isa<CXXMethodDecl>(Callee) + ? 1 + : 0; + for (unsigned I = Offset; I < Call->getNumArgs(); ++I) { + if (Call->getArg(I) != Expression) + continue; + if (Callee && I - Offset < Callee->getNumParams() && + Callee->getParamDecl(I - Offset)->hasAttr<NoEscapeAttr>()) + continue; + // Unknown callees may retain a reference to the argument. + if (Expression->getType()->isPointerType() || !Callee || + I - Offset >= Callee->getNumParams() || + Callee->getParamDecl(I - Offset)->getType()->isReferenceType()) + return true; + } + } + } + } + return false; +} -void UseStdMoveCheck::registerMatchers(MatchFinder *Finder) { - const auto AssignOperatorExpr = - cxxOperatorCallExpr( - isCopyAssignmentOperator(), - hasArgument(0, hasType(cxxRecordDecl( - hasAccessibleNonTrivialMoveAssignment()))), - hasArgument( - 1, declRefExpr( - to(varDecl( - hasLocalStorage(), - hasType(qualType(unless(anyOf( - isLValueReferenceType(), - isConstQualified() // Not valid to move const obj. - )))))), - unless(refersToEnclosingVariableOrCapture())) - .bind("assign-value")), - forCallable(functionDecl().bind("within-func")), unless(isInMacro())) - .bind("assign"); - Finder->addMatcher(AssignOperatorExpr, this); -} - -const CFG *UseStdMoveCheck::getCFG(const FunctionDecl *FD, - ASTContext *Context) { - std::unique_ptr<CFG> &TheCFG = CFGCache[FD]; - if (!TheCFG) { - const CFG::BuildOptions Options; - std::unique_ptr<CFG> FCFG = - CFG::buildCFG(nullptr, FD->getBody(), Context, Options); - if (!FCFG) +static const VarDecl *initializingVariable(const Expr *Expression, + ASTContext &Context) { + while (true) { + const DynTypedNodeList Parents = Context.getParents(*Expression); + if (Parents.size() != 1) + return nullptr; + if (const auto *Variable = Parents[0].get<VarDecl>()) + return Variable; + const auto *Parent = Parents[0].get<Expr>(); + if (!Parent || !isa<ExprWithCleanups, CXXBindTemporaryExpr, + MaterializeTemporaryExpr, ImplicitCastExpr>(Parent)) return nullptr; - TheCFG.swap(FCFG); + Expression = Parent; } - return TheCFG.get(); } -void UseStdMoveCheck::check(const MatchFinder::MatchResult &Result) { - const auto *AssignExpr = Result.Nodes.getNodeAs<Expr>("assign"); - const auto *AssignValue = Result.Nodes.getNodeAs<DeclRefExpr>("assign-value"); - const auto *WithinFunctionDecl = - Result.Nodes.getNodeAs<FunctionDecl>("within-func"); - - const CFG *TheCFG = getCFG(WithinFunctionDecl, Result.Context); - if (!TheCFG) - return; +// Keep the destination in the source's lexical scope so that moving ownership +// into an inner block cannot release a resource before the source leaves scope. +static const Stmt *variableScope(const VarDecl *Variable, ASTContext &Context) { + // Lambda parameters can have both the call operator and the lambda itself + // as AST parents. Their lexical scope is nevertheless the callable's body. + if (isa<ParmVarDecl>(Variable)) { + const auto *Function = + dyn_cast_or_null<FunctionDecl>(Variable->getParentFunctionOrMethod()); + return Function ? Function->getBody() : nullptr; + } + DynTypedNode Node = DynTypedNode::create(*Variable); + while (true) { + const DynTypedNodeList Parents = Context.getParents(Node); + if (Parents.size() != 1) + return nullptr; + Node = Parents[0]; + if (const auto *Scope = Node.get<Stmt>(); + Scope && isa<CompoundStmt, ForStmt, CXXForRangeStmt, IfStmt, SwitchStmt, + WhileStmt, CXXCatchStmt>(Scope)) + return Scope; + if (const auto *Function = Node.get<FunctionDecl>()) + return Function->getBody(); + } +} - // The algorithm to look for a convertible move-assign operator is the - // following: each node starts in the `Ready` state, with a number of - // `RemainingSuccessors` equal to its number of successors. - // - // Starting from the exit node, we walk the CFG backward. Whenever - // we meet a new block, we check if it either: - // 1. touches the `AssignValue`, in which case we stop the search, and mark - // each predecessor as not `Ready`. No predecessor walk. - // 2. contains a convertible copy-assign operator, in which case we generate a - // fix, and mark each predecessor as not Ready. No predecessor walk. - // 3. does not interact with `AssignValue`, in which case we decrement the - // `RemainingSuccessors` of each predecessor. And if it happens to turn to - // 0 while still being `Ready`, we add it to the `WorkList`. - - struct BlockState { - bool Ready; - unsigned RemainingSuccessors; - }; - llvm::DenseMap<const CFGBlock *, BlockState> CFGState; - for (const auto *B : *TheCFG) - CFGState.try_emplace(B, BlockState{true, B->succ_size()}); - - const CFGBlock &TheExit = TheCFG->getExit(); - std::vector<const CFGBlock *> WorkList = {&TheExit}; +static bool cannotThrow(const FunctionDecl *Function) { + return Function->getType()->castAs<FunctionProtoType>()->canThrow() == + CT_Cannot; +} - while (!WorkList.empty()) { - const CFGBlock *B = WorkList.back(); - WorkList.pop_back(); - const BlockState &BS = CFGState.find(B)->second; - if (!BS.Ready) +// This deliberately handles only ordinary, unambiguously applicable special +// members. General counterfactual overload resolution would require Sema. +static const CXXMethodDecl *findMove(const CXXMethodDecl *Copy, + const Expr *Target = nullptr) { + const CXXRecordDecl *Record = Copy->getParent()->getDefinition(); + if (!Record || Copy->isTrivial() || Record->isInvalidDecl()) + return nullptr; + const CXXMethodDecl *Move = nullptr; + for (const CXXMethodDecl *Method : Record->methods()) { + const auto *Constructor = dyn_cast<CXXConstructorDecl>(Method); + if (Copy->isCopyAssignmentOperator() + ? !Method->isMoveAssignmentOperator() + : !Constructor || !Constructor->isMoveConstructor()) continue; + QualType Pointee = Method->getParamDecl(0)->getType()->getPointeeType(); + if (Pointee.isNull() || Pointee.isConstQualified() || + Pointee.isVolatileQualified()) + continue; + if (Target && + ((Target->getType().isConstQualified() && !Method->isConst()) || + (Target->getType().isVolatileQualified() && !Method->isVolatile()) || + (Method->getRefQualifier() == RQ_RValue && Target->isLValue()) || + (Method->getRefQualifier() == RQ_LValue && !Target->isLValue()))) + continue; + if (Move || Method->isDeleted() || Method->isVariadic() || + Method->isExplicitObjectMemberFunction() || + Method->getAccess() != AS_public || + Method->getTrailingRequiresClause() || Method->isInvalidDecl()) + return nullptr; + Move = Method; + } + return Move; +} - assert(BS.RemainingSuccessors == 0 && - "All successors have been processed."); - bool ReferencesAssignedValue = false; - for (const CFGElement &Elt : llvm::reverse(*B)) { - if (Elt.getKind() != CFGElement::Kind::Statement) - continue; +static bool contains(const Stmt *Parent, const Stmt *Child) { + if (Parent == Child) + return true; + return Parent && llvm::any_of(Parent->children(), [&](const Stmt *S) { + return contains(S, Child); + }); +} - const Stmt *EltStmt = Elt.castAs<CFGStmt>().getStmt(); - if (EltStmt == AssignExpr) { - const StringRef AssignValueName = AssignValue->getDecl()->getName(); - diag(AssignValue->getBeginLoc(), "'%0' could be moved here") - << AssignValueName - << FixItHint::CreateReplacement( - AssignValue->getLocation(), - ("std::move(" + AssignValueName + ")").str()); - ReferencesAssignedValue = true; - break; - } +// EH edges can put unordered operands into distinct CFG blocks. Check their +// common expression, rather than trusting the CFG's chosen evaluation order. +static bool unorderedWith(const Expr *Copy, const DeclRefExpr *Reference, + ASTContext &Context) { + const Expr *Expression = Copy; + while (true) { + const DynTypedNodeList Parents = Context.getParents(*Expression); + if (Parents.size() != 1) + return true; + const auto *Parent = Parents[0].get<Expr>(); + if (!Parent) + return false; + if (const auto *Call = dyn_cast<CallExpr>(Parent)) { + if (const auto *Member = dyn_cast<CXXMemberCallExpr>(Call); + Member && contains(Member->getImplicitObjectArgument(), Reference)) + return true; + for (const Expr *Arg : Call->arguments()) + if (!contains(Arg, Copy) && contains(Arg, Reference)) + return true; + } else if (const auto *Construction = dyn_cast<CXXConstructExpr>(Parent)) { + if (!Construction->isListInitialization()) + for (const Expr *Arg : Construction->arguments()) + if (!contains(Arg, Copy) && contains(Arg, Reference)) + return true; + } else if (const auto *Binary = dyn_cast<BinaryOperator>(Parent)) { + if (Binary->getOpcode() != BO_Comma && Binary->getOpcode() != BO_LAnd && + Binary->getOpcode() != BO_LOr && + !(Binary->isAssignmentOp() && Context.getLangOpts().CPlusPlus17)) + if ((contains(Binary->getLHS(), Copy) && + contains(Binary->getRHS(), Reference)) || + (contains(Binary->getRHS(), Copy) && + contains(Binary->getLHS(), Reference))) + return true; + } + Expression = Parent; + } +} + +struct UseStdMoveCheck::FunctionAnalysis + : RecursiveASTVisitor<UseStdMoveCheck::FunctionAnalysis> { + std::unique_ptr<CFG> Graph; + std::unique_ptr<utils::ExprSequence> Sequence; + std::unique_ptr<utils::StmtToBlockMap> BlockMap; + llvm::DenseMap<const VarDecl *, SmallVector<const DeclRefExpr *, 8>> + References; + llvm::DenseMap<const VarDecl *, const VarDecl *> Aliases; + SmallVector<const DeclStmt *, 8> Declarations; + SmallVector<const CallExpr *, 8> Calls; + llvm::DenseMap<const VarDecl *, bool> Escaped; + + bool TraverseLambdaExpr(LambdaExpr *Lambda) { + // Capture initializers execute in this function; the body does not. In + // particular, a by-value capture's body refers to different storage. + for (Expr *Init : Lambda->capture_inits()) + TraverseStmt(Init); + return true; + } - // The reference is being referenced after the assignment. - if (!allDeclRefExprs(*cast<VarDecl>(AssignValue->getDecl()), *EltStmt, - *Result.Context) - .empty()) { - ReferencesAssignedValue = true; - break; + bool VisitDeclRefExpr(DeclRefExpr *Reference) { + if (const auto *Variable = dyn_cast<VarDecl>(Reference->getDecl())) + References[Variable].push_back(Reference); + return true; + } + + bool VisitVarDecl(VarDecl *Variable) { + if (Variable->hasLocalStorage() && + Variable->getType()->isLValueReferenceType() && Variable->hasInit()) + if (const auto *Reference = + dyn_cast<DeclRefExpr>(Variable->getInit()->IgnoreParenImpCasts())) + if (const auto *Source = dyn_cast<VarDecl>(Reference->getDecl())) + Aliases.try_emplace(Variable, Source); + return true; + } + + bool VisitDeclStmt(DeclStmt *Declaration) { + Declarations.push_back(Declaration); + return true; + } + + bool VisitCallExpr(CallExpr *Call) { + Calls.push_back(Call); + return true; + } + + const VarDecl *root(const VarDecl *Variable) const { + llvm::SmallPtrSet<const VarDecl *, 8> Seen; + while (Aliases.contains(Variable) && Seen.insert(Variable).second) + Variable = Aliases.lookup(Variable); + return Variable; + } + + void collectAliases(ASTContext &Context) { + decltype(References) Merged; + for (const auto &[Declaration, Refs] : References) + for (const DeclRefExpr *Reference : Refs) + if (!ExprMutationAnalyzer::isUnevaluated(Reference, Context)) + Merged[root(Declaration)].push_back(Reference); + References = std::move(Merged); + } + + ArrayRef<const DeclRefExpr *> referencesTo(const VarDecl *Variable) const { + auto Found = References.find(Variable); + return Found == References.end() ? ArrayRef<const DeclRefExpr *>() + : Found->second; + } + + bool mayHaveEscaped(const VarDecl *Variable, ASTContext &Context) { + auto [Found, Inserted] = Escaped.try_emplace(Variable, false); + if (Inserted) + for (const DeclRefExpr *Reference : referencesTo(Variable)) + if (mayEscape(Reference, Context, Aliases)) { + Found->second = true; + break; + } + return Found->second; + } + + bool isLastUse(const Expr *Copy, const DeclRefExpr *Source, + ASTContext &Context) const { + const auto *Variable = cast<VarDecl>(Source->getDecl()); + const ArrayRef<const DeclRefExpr *> Refs = referencesTo(Variable); + const CFGBlock *CopyBlock = BlockMap->blockContainingStmt(Copy); + // The CFG can include constructor initializers, while this analysis only + // collects uses from the body. Do not reason about an untracked copy site. + if (!CopyBlock || !llvm::is_contained(Refs, Source)) + return false; + for (const DeclRefExpr *Reference : Refs) + if (Reference != Source && unorderedWith(Copy, Reference, Context)) + return false; + + SmallVector<const Stmt *, 8> Restorations; + llvm::SmallPtrSet<const DeclRefExpr *, 8> RestorationRefs; + for (const DeclStmt *Declaration : Declarations) + for (const Decl *D : Declaration->decls()) + if (D == Variable) + Restorations.push_back(Declaration); + for (const CallExpr *Call : Calls) { + const auto *Method = + dyn_cast_or_null<CXXMethodDecl>(Call->getDirectCallee()); + if (!Method || !cannotThrow(Method)) + continue; + const Expr *Object = nullptr; + if (const auto *Assignment = dyn_cast<CXXOperatorCallExpr>(Call); + Assignment && (Method->isCopyAssignmentOperator() || + Method->isMoveAssignmentOperator())) { + Object = Assignment->getArg(0); + // Only an independent RHS restores the old value. An arbitrary + // reference-taking call is not a restoration contract. + if (llvm::any_of(Refs, [&](const DeclRefExpr *Ref) { + return contains(Assignment->getArg(1), Ref); + })) + continue; + } else if (const auto *Member = dyn_cast<CXXMemberCallExpr>(Call); + Member && Method->hasAttr<ReinitializesAttr>()) { + Object = Member->getImplicitObjectArgument(); + if (llvm::any_of(Refs, [&](const DeclRefExpr *Ref) { + return llvm::any_of(Call->arguments(), [&](const Expr *Arg) { + return contains(Arg, Ref); + }); + })) + continue; + } + const auto *Reference = + Object ? dyn_cast<DeclRefExpr>(Object->IgnoreParenImpCasts()) + : nullptr; + if (Reference && Reference->getDecl() == Variable) { + Restorations.push_back(Call); + RestorationRefs.insert(Reference); } } - if (ReferencesAssignedValue) { - // Cancel all predecessors. - for (const auto &S : B->preds()) { - if (!S.isReachable()) + + llvm::SmallPtrSet<const CFGBlock *, 16> Visited; + SmallVector<std::pair<const CFGBlock *, bool>, 16> WorkList{ + {CopyBlock, true}}; + while (!WorkList.empty()) { + const auto [Block, First] = WorkList.pop_back_val(); + if (!First && !Visited.insert(Block).second) + continue; + SmallVector<const Stmt *, 4> Saving; + for (const Stmt *Restoration : Restorations) + if (BlockMap->blockContainingStmt(Restoration) == Block && + (!First || Sequence->inSequence(Copy, Restoration))) + Saving.push_back(Restoration); + for (const DeclRefExpr *Reference : Refs) { + if ((First && Reference == Source) || + RestorationRefs.contains(Reference)) continue; - CFGState.find(&*S)->second.Ready = false; - } - } else { - // Or process the ready ones. - for (const auto &S : B->preds()) { - if (!S.isReachable()) + const CFGBlock *UseBlock = BlockMap->blockContainingStmt(Reference); + if (!UseBlock) + return false; + if (UseBlock != Block || + (First && Sequence->inSequence(Reference, Copy))) continue; - auto &W = CFGState.find(&*S)->second; - if (W.Ready && --W.RemainingSuccessors == 0) - WorkList.push_back(&*S); + if (!llvm::any_of(Saving, [&](const Stmt *Restoration) { + return Sequence->inSequence(Restoration, Reference); + })) + return false; } + if (Saving.empty()) + for (const auto &Successor : Block->succs()) + if (Successor) + WorkList.emplace_back(Successor, false); } + return true; + } + + bool handlerUses(const Expr *Copy, const VarDecl *Variable, + ASTContext &Context) const { + const ArrayRef<const DeclRefExpr *> Refs = referencesTo(Variable); + DynTypedNode Node = DynTypedNode::create(*Copy); + while (true) { + const DynTypedNodeList Parents = Context.getParents(Node); + if (Parents.size() != 1) + return true; + Node = Parents[0]; + if (const auto *Try = Node.get<CXXTryStmt>()) + for (unsigned I = 0; I < Try->getNumHandlers(); ++I) + if (llvm::any_of(Refs, [&](const DeclRefExpr *Ref) { + return contains(Try->getHandler(I), Ref); + })) + return true; + if (Node.get<FunctionDecl>()) + return false; + } + } +}; + +UseStdMoveCheck::UseStdMoveCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context), + Inserter(Options.getLocalOrGlobal("IncludeStyle", + utils::IncludeSorter::IS_LLVM), + areDiagsSelfContained()), + AllowedTypes( + utils::options::parseStringList(Options.get("AllowedTypes", ""))) {} + +UseStdMoveCheck::~UseStdMoveCheck() = default; + +void UseStdMoveCheck::registerPPCallbacks(const SourceManager &SM, + Preprocessor *PP, + Preprocessor *ModuleExpanderPP) { + Inserter.registerPreprocessor(PP); +} + +void UseStdMoveCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "IncludeStyle", Inserter.getStyle()); + Options.store(Opts, "AllowedTypes", + utils::options::serializeStringList(AllowedTypes)); +} + +void UseStdMoveCheck::onEndOfTranslationUnit() { + AnalysisCache.clear(); + Diagnosed.clear(); +} + +void UseStdMoveCheck::registerMatchers(MatchFinder *Finder) { + const auto Source = + declRefExpr(to(varDecl(hasLocalStorage(), + hasType(qualType(unless(anyOf( + isLValueReferenceType(), isConstQualified(), + isVolatileQualified())))))), + hasType(qualType(unless( + anyOf(isConstQualified(), isVolatileQualified())))), + unless(refersToEnclosingVariableOrCapture())) + .bind("source"); + Finder->addMatcher(cxxOperatorCallExpr(isCopyAssignmentOperator(), + hasArgument(1, hasValueSource(Source))) + .bind("assignment"), + this); + Finder->addMatcher( + cxxConstructExpr(hasDeclaration(cxxConstructorDecl(isCopyConstructor())), + hasArgument(0, hasValueSource(Source))) + .bind("construction"), + this); +} + +UseStdMoveCheck::FunctionAnalysis * +UseStdMoveCheck::getFunctionAnalysis(const FunctionDecl *FD, + ASTContext *Context) { + auto [Found, Inserted] = AnalysisCache.try_emplace(FD); + if (!Inserted) + return Found->second.get(); + CFG::BuildOptions Options; + Options.AddEHEdges = true; + Options.AddImplicitDtors = true; + Options.AddTemporaryDtors = true; + Options.setAllAlwaysAdd(); + auto Analysis = std::make_unique<FunctionAnalysis>(); + Analysis->Graph = CFG::buildCFG(FD, FD->getBody(), Context, Options); + if (!Analysis->Graph) + return nullptr; + Analysis->Sequence = std::make_unique<utils::ExprSequence>( + Analysis->Graph.get(), FD->getBody(), Context); + Analysis->BlockMap = + std::make_unique<utils::StmtToBlockMap>(Analysis->Graph.get(), Context); + Analysis->TraverseStmt(FD->getBody()); + Analysis->collectAliases(*Context); + Found->second = std::move(Analysis); + return Found->second.get(); +} + +void UseStdMoveCheck::check(const MatchFinder::MatchResult &Result) { + const auto *Construction = + Result.Nodes.getNodeAs<CXXConstructExpr>("construction"); + const auto *Assignment = + Result.Nodes.getNodeAs<CXXOperatorCallExpr>("assignment"); + const Expr *Copy = + Construction ? static_cast<const Expr *>(Construction) : Assignment; + const auto *Source = Result.Nodes.getNodeAs<DeclRefExpr>("source"); + const auto *Variable = cast<VarDecl>(Source->getDecl()); + const auto *Function = + dyn_cast_or_null<FunctionDecl>(Variable->getParentFunctionOrMethod()); + if (!Function || !Function->hasBody() || Function->isInvalidDecl() || + Function->isDependentContext() || + isa<CoroutineBodyStmt>(Function->getBody()) || + ExprMutationAnalyzer::isUnevaluated(Copy, *Result.Context)) + return; + const auto *CopyMethod = + Construction ? Construction->getConstructor() + : cast<CXXMethodDecl>(Assignment->getDirectCallee()); + const Expr *TargetExpr = Assignment ? Assignment->getArg(0) : nullptr; + const CXXMethodDecl *Move = findMove(CopyMethod, TargetExpr); + if (!Move || (Construction && Construction->isElidable()) || + !Result.Context->hasSameUnqualifiedType( + Result.Context->getCanonicalTagType(CopyMethod->getParent()), + Variable->getType().getNonReferenceType())) + return; + // A copy constructor can accept additional explicit arguments. Replacing + // only its first argument does not establish applicability of the move. + if (Construction) + for (unsigned I = 1; I < Construction->getNumArgs(); ++I) + if (!isa<CXXDefaultArgExpr>(Construction->getArg(I))) + return; + const Expr *Argument = + Construction ? Construction->getArg(0) : Assignment->getArg(1); + if (!Argument->isLValue()) + return; + if (!AllowedTypes.empty() && + !match(qualType(hasDeclaration( + namedDecl(matchers::matchesAnyListedRegexName(AllowedTypes)))), + Variable->getType().getNonReferenceType().getCanonicalType(), + *Result.Context) + .empty()) + return; + + const VarDecl *Target = + Construction ? initializingVariable(Construction, *Result.Context) + : nullptr; + if (Target == Variable) + return; + const auto *TargetReference = + TargetExpr ? dyn_cast<DeclRefExpr>(TargetExpr->IgnoreParenImpCasts()) + : nullptr; + if (TargetReference && TargetReference->getDecl() == Variable) + return; + const auto *MoveConstructor = dyn_cast<CXXConstructorDecl>(Move); + if (MoveConstructor && + (!Target || Target->getInitStyle() == VarDecl::CInit) && + (MoveConstructor->isExplicit() || + MoveConstructor->getExplicitSpecifier().getKind() == + ExplicitSpecKind::Unresolved)) + return; + + FunctionAnalysis *Analysis = getFunctionAnalysis(Function, Result.Context); + if (!Analysis || Analysis->mayHaveEscaped(Variable, *Result.Context) || + !Analysis->isLastUse(Copy, Source, *Result.Context)) + return; + // A cast, alias, conditional expression, or member call can still designate + // the source. Reject receivers depending on any tracked alias of it. + if (TargetExpr && llvm::any_of(Analysis->referencesTo(Variable), + [&](const DeclRefExpr *Ref) { + return contains(TargetExpr, Ref); + })) + return; + const bool NewExceptionEdge = cannotThrow(CopyMethod) && !cannotThrow(Move); + if (NewExceptionEdge && + Analysis->handlerUses(Copy, Variable, *Result.Context)) + return; + + const SourceLocation Location = + Result.SourceManager->getExpansionLoc(Source->getBeginLoc()); + // Different specializations can share the same spelling. A diagnostic never + // authorizes rewriting other, unobserved instantiations. + if (!Diagnosed.emplace(Location.getRawEncoding(), Variable->getName().str()) + .second) + return; + DiagnosticBuilder Diagnostic = diag(Location, "'%0' could be moved here") + << Variable->getName(); + if (NewExceptionEdge || Source->getBeginLoc().isMacroID() || + Source->getEndLoc().isMacroID() || Copy->getBeginLoc().isMacroID() || + Copy->getEndLoc().isMacroID() || + Function->getTemplatedKind() != FunctionDecl::TK_NonTemplate || + Variable->getType()->isRValueReferenceType()) + return; + if (Construction) { + const Stmt *Scope = variableScope(Variable, *Result.Context); + if (!Target || Target->isInitCapture() || !Target->hasLocalStorage() || + !Scope || variableScope(Target, *Result.Context) != Scope) + return; + if (Construction->isListInitialization()) + for (const CXXConstructorDecl *Constructor : + CopyMethod->getParent()->ctors()) + if (Constructor->getNumParams() != 0 && + utils::type_traits::isStdInitializerList( + Constructor->getParamDecl(0)->getType())) + return; } + const SourceManager &SM = *Result.SourceManager; + const CharSourceRange Range = Lexer::makeFileCharRange( + CharSourceRange::getTokenRange(Source->getSourceRange()), SM, + getLangOpts()); + if (Range.isInvalid()) + return; + Diagnostic << FixItHint::CreateInsertion(Range.getBegin(), "std::move(") + << FixItHint::CreateInsertion(Range.getEnd(), ")") + << Inserter.createIncludeInsertion(SM.getFileID(Range.getBegin()), + "<utility>"); } } // namespace clang::tidy::performance diff --git a/clang-tools-extra/clang-tidy/performance/UseStdMoveCheck.h b/clang-tools-extra/clang-tidy/performance/UseStdMoveCheck.h index 0bc1543d6e372..91a2265941be4 100644 --- a/clang-tools-extra/clang-tidy/performance/UseStdMoveCheck.h +++ b/clang-tools-extra/clang-tidy/performance/UseStdMoveCheck.h @@ -1,4 +1,4 @@ -//===--- InefficientCopyAssign.h - clang-tidy -----------------------------===// +//===--- UseStdMoveCheck.h - clang-tidy -----------------------------------===// // // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. // See https://llvm.org/LICENSE.txt for license information. @@ -10,24 +10,43 @@ #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_USESTDMOVECHECK_H #include "../ClangTidyCheck.h" - -#include "clang/Analysis/CFG.h" +#include "../utils/IncludeInserter.h" +#include "llvm/ADT/DenseMap.h" +#include <memory> +#include <set> +#include <string> +#include <utility> +#include <vector> namespace clang::tidy::performance { +/// Finds copy construction and assignment on the last use of a variable. +/// +/// For the user-facing documentation see: +/// https://clang.llvm.org/extra/clang-tidy/checks/performance/use-std-move.html class UseStdMoveCheck : public ClangTidyCheck { public: - UseStdMoveCheck(StringRef Name, ClangTidyContext *Context) - : ClangTidyCheck(Name, Context) {} + UseStdMoveCheck(StringRef Name, ClangTidyContext *Context); + ~UseStdMoveCheck() override; bool isLanguageVersionSupported(const LangOptions &LangOpts) const override { return LangOpts.CPlusPlus11; } void registerMatchers(ast_matchers::MatchFinder *Finder) override; void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP, + Preprocessor *ModuleExpanderPP) override; + void storeOptions(ClangTidyOptions::OptionMap &Opts) override; + void onEndOfTranslationUnit() override; private: - llvm::DenseMap<const FunctionDecl *, std::unique_ptr<CFG>> CFGCache; - const CFG *getCFG(const FunctionDecl *, ASTContext *); + struct FunctionAnalysis; + FunctionAnalysis *getFunctionAnalysis(const FunctionDecl *FD, + ASTContext *Context); + llvm::DenseMap<const FunctionDecl *, std::unique_ptr<FunctionAnalysis>> + AnalysisCache; + std::set<std::pair<unsigned, std::string>> Diagnosed; + utils::IncludeInserter Inserter; + const std::vector<StringRef> AllowedTypes; }; } // namespace clang::tidy::performance diff --git a/clang-tools-extra/docs/ReleaseNotes.md b/clang-tools-extra/docs/ReleaseNotes.md index 41aa783cabd24..0a66d0efb00a1 100644 --- a/clang-tools-extra/docs/ReleaseNotes.md +++ b/clang-tools-extra/docs/ReleaseNotes.md @@ -264,6 +264,12 @@ infrastructure are described first, followed by tool-specific sections. offered when an argument covers only part of a macro expansion, as it then has no source text of its own. +- Improved {doc}`performance-use-std-move + <clang-tidy/checks/performance/use-std-move>` to detect nontrivial copy + construction on the last use of an automatic variable, in addition to copy + assignment, track aliases and restored values, insert `<utility>` for fixes, + and support the `AllowedTypes` and `IncludeStyle` options. + - Improved {doc}`readability-convert-member-functions-to-static <clang-tidy/checks/readability/convert-member-functions-to-static>` check by fixing a crash when checking a const-qualified method declared with the diff --git a/clang-tools-extra/docs/clang-tidy/checks/performance/use-std-move.md b/clang-tools-extra/docs/clang-tidy/checks/performance/use-std-move.md index 2e892d03d0475..353e59d1028b7 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/performance/use-std-move.md +++ b/clang-tools-extra/docs/clang-tidy/checks/performance/use-std-move.md @@ -3,21 +3,283 @@ # performance-use-std-move -Suggests insertion of `std::move(...)` to turn copy assignment operator calls -into move assignment ones, when deemed valid and profitable. +Finds nontrivial copy construction and copy assignment that could select a move +operation instead, when the current value of an automatic variable has no +subsequent use on the analyzed paths. The check works with user-defined types; +it does not recognize containers or reset functions by their names. ```cpp -void assign(std::vector<int>& out) { - std::vector<int> some = make_vector(); - use_vector(some); - out = some; +void construct(std::string source) { + inspect(source.size()); + std::string target(source); // suggests std::move(source) } -// becomes +void assign(std::vector<int>& target, std::vector<int> source) { + target = source; // suggests std::move(source) +} +``` + +The diagnostic is `'source' could be moved here`. A diagnostic is a performance +recommendation, not a proof that the transformation preserves every observable +behavior of an arbitrary class. + +## Covered cases + +The following table describes coverage after the eligibility, last-use, and +alias checks below. **Advisory** means a diagnostic without an automatic fix. + +| Copy site | Behavior | +| --- | --- | +| Direct initialization of a local, `T target(source)` | Fix when the source and destination have the same lexical scope | +| Copy initialization, `T target = source` | Fix when the applicable move constructor is nonexplicit | +| List initialization, `T target{source}` | Fix unless initializer-list overloads could change constructor selection | +| Copy assignment, `target = source` | Fix for an applicable move assignment and an owned source | +| By-value function or constructor argument | Advisory; changing the argument can change overload resolution | +| Explicit temporary, `accept(T(source))` | Advisory | +| Heap construction, `new T(source)` | Advisory | +| Aggregate element, `Aggregate{source}` | Advisory | +| A return expression containing a genuine copy | Advisory; implicit moves and elidable copies are excluded | +| Copy into a shorter-lived local scope | Advisory | +| Named rvalue-reference source | Advisory; the caller's object may be affected | +| Copy capture, `[source]`, `[=]`, or `[copy = source]` | Advisory; no capture rewriting | +| Macro expansion | Advisory when analyzable; the macro spelling is not rewritten | +| Concrete function-template instantiation | Advisory, deduplicated at the copy spelling | +| A lambda's own parameters and locals | Analyzed in the lambda's callable context | + +Aliases of types and parentheses do not prevent detection: + +```cpp +using Text = std::string; +void aliases(Text source) { + Text target((source)); // becomes ((std::move(source))) +} +``` + +A trivial move can replace a nontrivial copy. The check examines the selected +copy operation instead of treating a nontrivial destructor as evidence of an +expensive copy. Nontriviality is a cost heuristic: the check does not measure +runtime, inspect allocator costs, or guarantee that moving is faster. + +A move must accept a mutable, nonvolatile rvalue reference and be public, +nondeleted, unconstrained, and unambiguously applicable within the supported +special-member cases. The source and destination types +must agree after removing qualifiers. Constructor explicitness and the move +assignment's receiver qualifiers are taken into account. A move that merely +exists but cannot be selected is not sufficient: + +```cpp +struct Object { + Object(const Object&); + explicit Object(Object&&); +}; +void explicitMove(Object source) { + Object target(source); // direct initialization can use the explicit move +} +void ineffectiveMove(Object source) { + Object target = source; // std::move would still select the copy: no diagnostic +} +``` + +## Last uses, aliases, and restored values + +The analysis uses a control-flow graph and expression sequencing. It checks +branches separately, considers exception paths, and does not equate textual +source order with execution order. + +```cpp +void branches(bool condition, std::string source) { + if (condition) + consume(source); // advisory: last use on this path + else + consume(source); // advisory: last use on the alternative path +} + +void laterUse(std::string source) { + consume(source); // no diagnostic + inspect(source); // source is still used +} +``` + +Local reference aliases, including transitive aliases, are tracked. An alias +that is used only before the copy need not prevent the recommendation. Passing +an alias to an unknown reference-taking function may retain access and prevents +a recommendation even if that call precedes the copy. + +```cpp +void localAlias(Value source) { + const Value& first = source; + const Value& second = first; + inspect(second.value); + Value target(source); // eligible +} + +void retainedAlias(Value source) { + retain(source); // retain(const Value&) may retain a reference + Value target(source); // no diagnostic +} +``` + +Pointer aliases and retained references or pointers returned by member functions +are handled conservatively. Taking an address for a call with an applicable +`[[clang::noescape]]` parameter contract on an ordinary function or member call +need not be a permanent escape. Constructor arguments are conservatively treated +as possible escapes without using that annotation. +The check trusts that annotation; it does not verify the callee's implementation. + +Objects created within a loop are distinguished from values reused across +iterations: + +```cpp +void fresh(int count) { + for (int i = 0; i < count; ++i) { + Value source; + Value target(source); // eligible: a new source exists each iteration + } +} -void assign(std::vector<int>& out) { - std::vector<int> some = make_vector(); - use_vector(some); - out = std::move(some); +void repeated(Value source, int count) { + for (int i = 0; i < count; ++i) + consume(source); // no diagnostic: later iterations reuse the same value } ``` + +Under conventional value semantics, an independent, nonthrowing copy or move +assignment restores a value. A nonthrowing member function with +`[[clang::reinitializes]]` supplies an explicit restoration contract. Ordinary +mutating calls, compound assignments, and arbitrary non-const reference +parameters do not supply that contract. Throwing restorations are conservatively +not used to prove that the old value is dead. + +```cpp +void restoration(Value source) { + consume(source); // advisory: this value is not used again + source = Value{}; // independent, nonthrowing value assignment + inspect(source.value); // uses the restored value +} + +void incompleteRestoration(bool condition, Value source) { + consume(source); // no diagnostic + if (condition) + source = Value{}; + inspect(source.value); // not restored on every path +} +``` + +Unevaluated expressions, such as an ordinary `sizeof(source)`, do not count as +runtime reads. Expressions whose evaluation order is unknown are rejected; +ordered expressions are not rejected merely because the source appears twice. + +```cpp +consume((inspect(source.value), source)); // advisory: comma orders the read +consumeTwo(source, source); // no diagnostic: argument order varies +``` + +## Automatic fixes and their guarantees + +Construction fixes are restricted to owned local variables or value parameters +and directly initialized automatic destinations in the same lexical scope. +Assignment fixes are withheld for rvalue-reference sources, which can alias a +caller-visible target. Known self-assignment is not diagnosed, including casts +and local aliases. An assignment receiver depending on the source is also +excluded because it could designate the same object. Fixes preserve +the source expression's spelling, validate the editable file range, and insert +`<utility>` when needed. + +Scopes introduced by control statements also matter: + +```cpp +void shorterLoopLifetime(Value source) { + for (Value target(source); again(); ) // advisory: target dies when the loop ends + break; +} +``` + +Function arguments, captures, macros, and function-template instantiations are +not automatically rewritten. In particular, a movable observed template +instantiation does not establish that rewriting the template is valid for all +future instantiations. + +If a potentially throwing move replaces a nonthrowing copy, it can introduce +an exceptional path absent from the original graph. A use of the source in an +enclosing handler prevents the recommendation; otherwise the diagnostic is +advisory. + +```cpp +void newExceptionPath(Value source) { + try { + Value target(source); // no recommendation if copy is noexcept, move can throw + } catch (...) { + inspect(source.value); + } +} +``` + +These restrictions reduce mechanical rewriting risks. They do **not** establish +arbitrary semantic equivalence. The check assumes conventional value behavior +for copy, move, and independent assignment. It cannot generally determine: + +- Whether constructors, assignments, or destructors have intentional observable + side effects, or whether different exception behavior is acceptable. +- Whether the source must retain ownership until a particular point. Moving a + shared owner of a lock or scope guard can release a resource earlier even + without another direct source reference. +- Whether an arbitrary member function retains `this` or an internal reference + through hidden state. Explicit retained reference results are tracked, but + interprocedural escape analysis is not performed. +- Whether a move preserves aliases into the object's internal storage. +- Whether a recommendation improves performance on the actual workload. + +Same lexical scope, an available move, and `noexcept` are useful restrictions, +not proofs of all these properties. Review recommendations involving +lifetime-sensitive or unusual value types; exclude them with `AllowedTypes` or +normal `NOLINT` suppression. + +## Exclusions and remaining gaps + +No recommendation is made for const or volatile sources, lvalue-reference +source variables, global/static/thread-local/external source storage, trivial +copies, unavailable moves, existing moves, or elidable construction. A copy +through an lvalue-reference alias is excluded even when the alias refers to a +local; aliases are tracked to assess copies from the original variable. + +The following cases require additional analysis and are not supported: + +| Gap | Example or required analysis | +| --- | --- | +| Forwarding APIs and reference-taking functions that copy internally | `container.emplace_back(source)`; needs generic interprocedural copy/escape summaries | +| Object fields and array or container elements as sources | `consume(owner.field)`, `consume(array[0])`; needs access-path and owner-lifetime analysis | +| Arbitrary pointer-alias liveness | `T* alias = &source`; conservatively excluded even if all pointer uses precede the copy | +| Unannotated reset methods and throwing restorations | Method names and mutation alone do not establish restoration | +| RHS-dependent restoration | `source = transform(source)`; not treated as an independent restoration | +| Restoration through aliases or computed receivers | `alias = T{}`; needs a proof of whole-object identity, not just an alias into a base subobject | +| Complex move overloads and constrained special members | Requires counterfactual overload resolution; a private move is excluded even in an otherwise authorized access context | +| Additional explicit copy-constructor arguments, variadic moves, and explicit-object move assignment | Need argument and overload modeling beyond ordinary special-member calls | +| General path feasibility | Incompatible branch conditions can cause conservative missed recommendations | +| Constructor member initializers outside the analyzed body | Need additional CFG and construction-context support | +| Coroutines | Suspension, frame storage, escaping references, and destruction need lifetime modeling | +| Template-wide and macro-wide fixes | Require validity beyond observed instantiations or expansions | +| Uninstantiated dependent code and a generic `std::forward` recommendation | Need instantiated copy and move facts; a concrete rvalue-reference instantiation can receive an advisory `std::move` diagnostic | +| Language sequencing not represented by the supported expression analysis | Conservatively missed, particularly overloaded operator expressions | + +CFG construction or an expression-to-block mapping failure causes the check to +skip the recommendation. Invalid and incomplete code may therefore have less +coverage. These gaps are deliberate conservative misses; the check has no mode +that bypasses alias or last-use checks to generate automatic fixes. + +## Options + +### AllowedTypes + +A semicolon-separated list of regular expressions matching types that should +not be diagnosed. The default is an empty list. Patterns containing `::` are +matched against a qualified name; other patterns match the unqualified name. +Aliases resolve to the underlying record name. The option applies to both +construction and assignment. + +For example, `ScopeGuard;::application::.*Lease` excludes an unqualified +`ScopeGuard` and matching application lease types. The check has no built-in +list of non-owning views or scope guards. + +### IncludeStyle + +A string specifying the include style, `llvm` or `google`. The default is `llvm`. diff --git a/clang-tools-extra/test/clang-tidy/checkers/performance/use-std-move-captures.cpp b/clang-tools-extra/test/clang-tidy/checkers/performance/use-std-move-captures.cpp new file mode 100644 index 0000000000000..0ece85e03f67f --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/performance/use-std-move-captures.cpp @@ -0,0 +1,31 @@ +// RUN: %check_clang_tidy %s performance-use-std-move %t -std=c++14,c++17,c++20,c++23 +// RUN: %clang -std=c++14 -pedantic-errors -fsyntax-only -nostdinc++ -isystem %clang_tidy_headers/std %t.cpp + +struct Value { + Value(); + Value(const Value &); + Value(Value &&) noexcept; +}; +void initCapture(Value source) { + auto closure = [copy = source] {}; + // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: 'source' could be moved here + // CHECK-FIXES: auto closure = [copy = source] {}; +} +void renamedCaptureWithUse(Value source) { + auto closure = [copy = source] {}; + Value target(source); + // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: 'source' could be moved here + // CHECK-FIXES: Value target(std::move(source)); +} +void movedCapture(Value source) { + auto closure = [copy = static_cast<Value &&>(source)] {}; +} + +void genericLambda() { + auto copy = [](auto source) { + Value target(source); + // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: 'source' could be moved here [performance-use-std-move] + // CHECK-FIXES: Value target(source); + }; + copy(Value{}); +} diff --git a/clang-tools-extra/test/clang-tidy/checkers/performance/use-std-move-constrained.cpp b/clang-tools-extra/test/clang-tidy/checkers/performance/use-std-move-constrained.cpp new file mode 100644 index 0000000000000..b1b3e590bbd28 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/performance/use-std-move-constrained.cpp @@ -0,0 +1,53 @@ +// RUN: %check_clang_tidy %s performance-use-std-move %t -std=c++20,c++23 +// RUN: %clang -std=c++20 -fsyntax-only -nostdinc++ -isystem %clang_tidy_headers/std %t.cpp + +// CHECK-FIXES: #include <utility> + +template <bool Enabled> struct Constrained { + Constrained(const Constrained &); + Constrained(Constrained &&) requires Enabled; + Constrained &operator=(const Constrained &); + Constrained &operator=(Constrained &&) requires Enabled; +}; + +// Counterfactual constraint satisfaction is outside the supported move search. +void enabledMove(Constrained<true> source) { + Constrained<true> target(source); +} +void disabledMove(Constrained<false> source) { + Constrained<false> target(source); +} +void enabledAssignment(Constrained<true> &target, Constrained<true> source) { + target = source; +} +void disabledAssignment(Constrained<false> &target, Constrained<false> source) { + target = source; +} + +struct Ordinary { + Ordinary(const Ordinary &); + Ordinary(Ordinary &&); +}; +void ordinaryMove(Ordinary source) { + Ordinary target(source); + // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: 'source' could be moved here [performance-use-std-move] + // CHECK-FIXES: Ordinary target(std::move(source)); +} + +template <bool Explicit> struct ConditionalExplicit { + ConditionalExplicit(const ConditionalExplicit &); + explicit(Explicit) ConditionalExplicit(ConditionalExplicit &&); +}; +void explicitDirectInitialization(ConditionalExplicit<true> source) { + ConditionalExplicit<true> target(source); + // CHECK-MESSAGES: :[[@LINE-1]]:36: warning: 'source' could be moved here [performance-use-std-move] + // CHECK-FIXES: ConditionalExplicit<true> target(std::move(source)); +} +void explicitCopyInitialization(ConditionalExplicit<true> source) { + ConditionalExplicit<true> target = source; +} +void nonexplicitCopyInitialization(ConditionalExplicit<false> source) { + ConditionalExplicit<false> target = source; + // CHECK-MESSAGES: :[[@LINE-1]]:39: warning: 'source' could be moved here [performance-use-std-move] + // CHECK-FIXES: ConditionalExplicit<false> target = std::move(source); +} diff --git a/clang-tools-extra/test/clang-tidy/checkers/performance/use-std-move-construction.cpp b/clang-tools-extra/test/clang-tidy/checkers/performance/use-std-move-construction.cpp new file mode 100644 index 0000000000000..877bb589b8f47 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/performance/use-std-move-construction.cpp @@ -0,0 +1,419 @@ +// RUN: %check_clang_tidy %s performance-use-std-move %t -std=c++11,c++14,c++17,c++20,c++23 +// RUN: %clang -std=c++11 -pedantic-errors -fsyntax-only -nostdinc++ -isystem %clang_tidy_headers/std %t.cpp + +#include <initializer_list> +#include <utility> + +struct Movable { + Movable(); + Movable(const Movable &); + Movable(Movable &&); + Movable &operator=(const Movable &); + Movable &operator=(Movable &&); + void touch() const; +}; +void consume(Movable); +void consumeTwice(Movable, Movable); +void borrow(const Movable &); +bool again(); + +void localInitialization() { + Movable source; + source.touch(); + Movable target(source); + // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: 'source' could be moved here [performance-use-std-move] + // CHECK-FIXES: Movable target(std::move(source)); + target.touch(); +} + +void valueParameter(Movable source) { + consume(source); + // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: 'source' could be moved here [performance-use-std-move] + // CHECK-FIXES: consume(source); +} + +void rvalueReference(Movable &&source) { + Movable target(source); + // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: 'source' could be moved here [performance-use-std-move] + // CHECK-FIXES: Movable target(source); +} + +using Alias = Movable; +void parenthesizedAlias(Alias source) { + Alias target((source)); + // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: 'source' could be moved here [performance-use-std-move] + // CHECK-FIXES: Alias target((std::move(source))); +} + +void lastOfSeveralCopies(Movable source) { + consume(source); + Movable target = source; + // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: 'source' could be moved here [performance-use-std-move] + // CHECK-FIXES: Movable target = std::move(source); +} + +void branches(bool condition, Movable source) { + if (condition) { + consume(source); + // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: 'source' could be moved here [performance-use-std-move] + } else { + source.touch(); + consume(source); + // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: 'source' could be moved here [performance-use-std-move] + } +} + +void usedLater(Movable source) { + consume(source); + source.touch(); +} +void usedAfterBranch(bool condition, Movable source) { + if (condition) + consume(source); + source.touch(); +} +void unorderedUses(Movable source) { consumeTwice(source, source); } +void loop(Movable source) { + while (again()) + consume(source); +} +void rangeLoop(Movable source) { + int values[2] = {}; + for (int value : values) + consume(source); +} +void backwardJump(Movable source) { +repeat: + consume(source); + if (again()) + goto repeat; +} + +void referenceAlias(Movable source) { + Movable &alias = (source); + consume(source); + alias.touch(); +} +void constReferenceAlias(Movable source) { + const Movable &alias = source; + consume(source); + alias.touch(); +} +void pointerAlias(Movable source) { + const Movable *alias = &source; + consume(source); + alias->touch(); +} +void escapedReference(Movable source) { + borrow(source); + consume(source); +} +void referenceCapture(Movable source) { + auto later = [&source] { source.touch(); }; + consume(source); + later(); +} +void copyCapture(Movable source) { + auto later = [source] { source.touch(); }; + // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: 'source' could be moved here [performance-use-std-move] + // CHECK-FIXES: auto later = [source] { source.touch(); }; + later(); +} +void implicitCopyCapture(Movable source) { + auto later = [=] { source.touch(); }; + // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: 'source' could be moved here [performance-use-std-move] + // CHECK-FIXES: auto later = [=] { source.touch(); }; + later(); +} + +void alreadyMoved(Movable source) { consume(std::move(source)); } +void constSource(const Movable source) { consume((source)); } +void referenceSource(Movable &source) { consume(source); } +void builtInType(int source) { int target = source; } + +struct CopyOnly { + CopyOnly(const CopyOnly &); +}; +void consume(CopyOnly); +void copyOnly(CopyOnly source) { consume(source); } +struct DeletedMove { + Movable field; + DeletedMove(const DeletedMove &); + DeletedMove(DeletedMove &&) = delete; +}; +void consume(DeletedMove); +void deletedMove(DeletedMove source) { consume(source); } +struct PrivateMove { + PrivateMove(const PrivateMove &); + +private: + PrivateMove(PrivateMove &&); +}; +void consume(PrivateMove); +void privateMove(PrivateMove source) { consume(source); } +struct Trivial { + int value; +}; +void consume(Trivial); +void trivialCopy(Trivial source) { consume(source); } +struct TrivialMove { + TrivialMove(const TrivialMove &); + TrivialMove(TrivialMove &&) = default; +}; +void consume(TrivialMove); +void trivialMove(TrivialMove source) { + consume(source); + // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: 'source' could be moved here [performance-use-std-move] + // CHECK-FIXES: consume(source); +} + +struct ReferenceHolder { + ReferenceHolder(const Movable &); +}; +void referenceConstructor(Movable source) { ReferenceHolder target(source); } +void referenceReceiver(Movable source) { borrow(source); } + +Movable global; +void globalSource() { Movable target(global); } +void staticSource() { + static Movable source; + Movable target(source); +} +void threadLocalSource() { + thread_local Movable source; + Movable target(source); +} +void externalSource() { + extern Movable source; + Movable target(source); +} + +#define COPY(source) consume((source)) +void macro(Movable source) { + COPY(source); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'source' could be moved here [performance-use-std-move] + // CHECK-FIXES: COPY(source); +} +void macroBeforeUse(Movable source) { + COPY(source); + source.touch(); +} + +Movable returnLocal() { + Movable source; + return source; +} + +void overloaded(Movable); +void overloaded(Movable &&); +void overloadedReceiver(Movable source) { + overloaded(source); + // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: 'source' could be moved here [performance-use-std-move] + // CHECK-FIXES: overloaded(source); +} + +template <class T> void templateCopy(T source) { + T target(source); + // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: 'source' could be moved here [performance-use-std-move] + // CHECK-FIXES: T target(source); +} +template void templateCopy<Movable>(Movable); +template void templateCopy<CopyOnly>(CopyOnly); +template void templateCopy<TrivialMove>(TrivialMove); +template <class T> void forwardingReference(T &&source) { + consume(source); + // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: 'source' could be moved here [performance-use-std-move] + // CHECK-FIXES: consume(source); +} +template void forwardingReference<Movable>(Movable &&); +template void forwardingReference<Movable &>(Movable &); + +struct Derived : Movable {}; +void copyBase(Derived source) { Movable target(source); } + +struct AmbiguousMove { + AmbiguousMove(const AmbiguousMove &); + AmbiguousMove(AmbiguousMove &&, int = 0); + AmbiguousMove(AmbiguousMove &&, double = 0); +}; +void ambiguousMove(AmbiguousMove source) { AmbiguousMove target(source); } + +struct CheapCopy { + CheapCopy(const CheapCopy &) = default; + CheapCopy(CheapCopy &&); +}; +void cheapCopy(CheapCopy source) { CheapCopy target(source); } + +void shorterDestinationLifetime(Movable source) { + { + Movable target(source); + // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: 'source' could be moved here [performance-use-std-move] + // CHECK-FIXES: Movable target(source); + } +} + +void shorterForInitializerLifetime(Movable source) { + for (Movable target(source); again();) + break; + // CHECK-MESSAGES: :[[@LINE-2]]:23: warning: 'source' could be moved here [performance-use-std-move] + // CHECK-FIXES: for (Movable target(source); again();) +} + +void shorterImplicitScopeLifetime(Movable source, bool condition) { + if (condition) + Movable target(source); + // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: 'source' could be moved here [performance-use-std-move] + // CHECK-FIXES: Movable target(source); +} + +struct MemberOwner { + Movable field; + MemberOwner(const MemberOwner &); + MemberOwner(MemberOwner &&); +}; +void memberAlias(MemberOwner source) { + const Movable &alias = source.field; + MemberOwner target(source); + alias.touch(); +} + +struct ReferenceOwner { + ReferenceOwner(const ReferenceOwner &); + ReferenceOwner(ReferenceOwner &&); + const ReferenceOwner &reference() const; +}; +void returnedAlias(ReferenceOwner source) { + const ReferenceOwner &alias = source.reference(); + ReferenceOwner target(source); + alias.reference(); +} + +void constRvalueReference(const Movable &&source) { consume(source); } + +struct ExplicitMove { + ExplicitMove(const ExplicitMove &); + explicit ExplicitMove(ExplicitMove &&); +}; +void explicitMove(ExplicitMove source) { + ExplicitMove target(source); + // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: 'source' could be moved here [performance-use-std-move] + // CHECK-FIXES: ExplicitMove target(std::move(source)); +} +void explicitCopyInitialization(ExplicitMove source) { ExplicitMove target = source; } + +struct MoveOnly { + MoveOnly(const MoveOnly &) = delete; + MoveOnly(MoveOnly &&); +}; +void consume(MoveOnly); +void movedMoveOnly(MoveOnly source) { consume(std::move(source)); } + +void assignmentAlias(Movable &target, Movable source) { + const Movable &alias = source; + target = source; + alias.touch(); +} + +void assignmentUnorderedUses(Movable &target, Movable source) { + consumeTwice(target = source, source); + // CHECK-FIXES: consumeTwice(target = source, source); +} + +void discardedReferenceResult(ReferenceOwner source) { + source.reference(); + ReferenceOwner target(source); + // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: 'source' could be moved here [performance-use-std-move] + // CHECK-FIXES: ReferenceOwner target(std::move(source)); +} + +struct PointerOwner { + PointerOwner(const PointerOwner &); + PointerOwner(PointerOwner &&); + const PointerOwner *pointer() const; +}; +void returnedPointerAlias(PointerOwner source) { + const PointerOwner *alias = source.pointer(); + PointerOwner target(source); + alias->pointer(); +} + +void assignedPointerAlias(PointerOwner source) { + const PointerOwner *alias; + alias = source.pointer(); + PointerOwner target(source); + alias->pointer(); +} + +void conditionalAlias(bool condition, PointerOwner source, + const PointerOwner *other) { + const PointerOwner *alias = condition ? source.pointer() : other; + PointerOwner target(source); + alias->pointer(); +} + +void exceptionUse(Movable source) { + try { + consume(source); + } catch (...) { + source.touch(); + } +} + +struct ExtraArguments { + ExtraArguments(const ExtraArguments &, int = 0); + ExtraArguments(ExtraArguments &&); +}; +void explicitExtraArgument(ExtraArguments source) { + ExtraArguments target(source, 1); // The move cannot accept both arguments. +} +void defaultExtraArgument(ExtraArguments source) { + ExtraArguments target(source); + // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: 'source' could be moved here [performance-use-std-move] + // CHECK-FIXES: ExtraArguments target(std::move(source)); +} + +struct VariadicMove { + VariadicMove(const VariadicMove &); + VariadicMove(VariadicMove &&, ...); +}; +void variadicMove(VariadicMove source) { VariadicMove target(source); } + +struct NontrivialDestructor { + int value; + NontrivialDestructor(const NontrivialDestructor &) = default; + NontrivialDestructor(NontrivialDestructor &&) = default; + ~NontrivialDestructor(); +}; +void cheapCopyWithDestructor(NontrivialDestructor source) { + NontrivialDestructor target(source); +} + +Movable conditionalRvalueReference(Movable &&source, bool condition) { + return condition ? source : Movable{}; + // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: 'source' could be moved here [performance-use-std-move] + // CHECK-FIXES: return condition ? source : Movable{}; +} + +struct ListOverload { + ListOverload(const ListOverload &); + ListOverload(ListOverload &&); + ListOverload(std::initializer_list<int>); + operator int() &&; +}; +void changedListOverload(ListOverload source) { + ListOverload target{source}; + // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: 'source' could be moved here [performance-use-std-move] + // CHECK-FIXES: ListOverload target{source}; +} + +struct ConstMove { + ConstMove(const ConstMove &); + ConstMove(const ConstMove &&); +}; +void constMove(ConstMove source) { ConstMove target(source); } + +struct VolatileMove { + VolatileMove(const VolatileMove &); + VolatileMove(volatile VolatileMove &&); +}; +void volatileMove(VolatileMove source) { VolatileMove target(source); } diff --git a/clang-tools-extra/test/clang-tidy/checkers/performance/use-std-move-flow.cpp b/clang-tools-extra/test/clang-tidy/checkers/performance/use-std-move-flow.cpp new file mode 100644 index 0000000000000..e70c711d13484 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/performance/use-std-move-flow.cpp @@ -0,0 +1,361 @@ +// RUN: %check_clang_tidy %s performance-use-std-move %t -std=c++11,c++14,c++17,c++20,c++23 +// RUN: %clang -std=c++11 -pedantic-errors -fsyntax-only -nostdinc++ -isystem %clang_tidy_headers/std %t.cpp + +#include <utility> + +struct Value { + int value; + Value(); + Value(const Value &); + Value(Value &&) noexcept; + Value &operator=(const Value &) noexcept; + Value &operator=(Value &&) noexcept; + [[clang::reinitializes]] void refresh() noexcept; + [[clang::reinitializes]] void throwingRefresh(); + void mutate() noexcept; + Value &target() noexcept; +}; +void consume(Value); +void consumeTwo(Value, Value); +void read(int); +void retain(const Value &); +void inspect(Value * __attribute__((noescape))); +bool again(); + +void localAliasUsedBeforeCopy(Value source) { + const Value &alias = (source); + read(alias.value); + Value target(source); + // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: 'source' could be moved here + // CHECK-FIXES: Value target(std::move(source)); +} + +void transitiveAlias(Value source) { + const Value &first = source; + const Value &second = first; + read(second.value); + Value target(source); + // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: 'source' could be moved here + // CHECK-FIXES: Value target(std::move(source)); +} + +void transitiveAliasUsedLater(Value source) { + const Value &first = source; + const Value &second = first; + Value target(source); + read(second.value); +} + +void aliasEscapesBeforeCopy(Value source) { + const Value &alias = source; + retain(alias); + Value target(source); +} + +void transientAddress(Value source) { + inspect(&source); + Value target(source); + // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: 'source' could be moved here + // CHECK-FIXES: Value target(std::move(source)); +} + +void pointerAlias(Value source) { + Value *alias = &source; + read(alias->value); + Value target(source); +} + +void freshObjectInLoop(int count) { + for (int i = 0; i < count; ++i) { + Value source; + Value target(source); + // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: 'source' could be moved here + // CHECK-FIXES: Value target(std::move(source)); + } +} + +void restoredInLoop(Value source) { + while (again()) { + consume(source); + // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: 'source' could be moved here + // CHECK-FIXES: consume(source); + source = Value{}; + } +} + +void annotationRestores(Value source) { + consume(source); + // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: 'source' could be moved here + source.refresh(); + read(source.value); +} + +void throwingRestoration(Value source) { + try { + consume(source); + source.throwingRefresh(); + } catch (...) { + read(source.value); + } +} + +void mutationIsNotRestoration(Value source) { + consume(source); + source.mutate(); + read(source.value); +} + +void restorationOnOnlyOnePath(bool condition, Value source) { + consume(source); + if (condition) + source = Value{}; + read(source.value); +} + +void restorationOnBothPaths(bool condition, Value source) { + consume(source); + // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: 'source' could be moved here + if (condition) + source = Value{}; + else + source.refresh(); + read(source.value); +} + +void orderedComma(Value source) { + consume((read(source.value), source)); + // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: 'source' could be moved here + // CHECK-FIXES: consume((read(source.value), source)); +} + +void nestedOrderedComma(Value source) { + Value target((read(0), (read(source.value), source))); + // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: 'source' could be moved here + // CHECK-FIXES: Value target((read(0), (read(source.value), std::move(source)))); +} + +void commaBeforeUse(Value source) { + consume((read(source.value), source)); + read(source.value); +} + +Value conditionalCopy(bool condition, Value source) { + return condition ? source : Value{}; + // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: 'source' could be moved here + // CHECK-FIXES: return condition ? source : Value{}; +} + +void capture(Value source) { + auto closure = [source] { return source.value; }; + // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: 'source' could be moved here + // CHECK-FIXES: auto closure = [source] { return source.value; }; +} + +void defaultCapture(Value source) { + auto closure = [=] { return source.value; }; + // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: 'source' could be moved here + // CHECK-FIXES: auto closure = [=] { return source.value; }; +} + +void captureBeforeUse(Value source) { + auto closure = [source] { return source.value; }; + read(source.value); +} + +void referenceCapture(Value source) { + auto closure = [&source] { return source.value; }; + Value target(source); + read(closure()); +} + +void lambdaLocal() { + auto closure = [](Value source) { + Value target(source); + // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: 'source' could be moved here + // CHECK-FIXES: Value target(std::move(source)); + }; +} + +void heapCopy(Value source) { + Value *target = new Value(source); + // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: 'source' could be moved here + // CHECK-FIXES: Value *target = new Value(source); + delete target; +} + +void explicitTemporary(Value source) { + retain(Value(source)); + // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: 'source' could be moved here + // CHECK-FIXES: retain(Value(source)); +} + +struct Aggregate { Value field; }; +void aggregate(Value source) { + Aggregate target{source}; + // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: 'source' could be moved here + // CHECK-FIXES: Aggregate target{source}; +} + +void listInitialization(Value source) { + Value target{source}; + // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: 'source' could be moved here + // CHECK-FIXES: Value target{std::move(source)}; +} + +void unevaluatedUse(Value source) { + Value target(source); + // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: 'source' could be moved here + // CHECK-FIXES: Value target(std::move(source)); + (void)sizeof(source); + (void)noexcept(source.refresh()); +} + +void selfAssignment(Value source) { + source = source; + // CHECK-FIXES: source = source; +} + +void selfAssignmentThroughAlias(Value source) { + Value &alias = source; + alias = source; +} + +void transitiveSelfAssignment(Value source) { + Value &first = source; + Value &second = first; + second = source; +} + +void castSelfAssignment(Value source) { + static_cast<Value &>(source) = source; +} +void castAliasSelfAssignment(Value source) { + Value &alias = source; + static_cast<Value &>(alias) = source; +} +void sourceDependentReceiver(Value source) { + source.target() = source; // target() could return source itself. +} +void conditionalSelfAssignment(Value source, Value &other, bool condition) { + (condition ? source : other) = source; +} + +struct PointerHolder { + PointerHolder(const Value *); +}; +void escapedThroughConstructor(Value source) { + PointerHolder holder(&source); + Value target(source); +} + +struct VariadicHolder { + VariadicHolder(int, ...); +}; +void escapedThroughVariadicConstructor(Value source) { + VariadicHolder holder(0, &source); + Value target(source); +} + +struct ReferenceAggregate { + const Value &reference; +}; +void escapedThroughAggregate(Value source) { + ReferenceAggregate holder{source}; + Value target(source); +} +void pointerArithmeticEscape(Value source) { + const Value *alias = &source + 0; + Value target(source); +} +void commaReferenceEscape(Value source) { + const Value &alias = (read(0), source); + Value target(source); +} +void thrownPointerEscape(Value source) { + try { + throw &source; + } catch (const Value *pointer) { + // Even without a later named use, the handler could retain the pointer. + inspect(const_cast<Value *>(pointer)); + } + Value target(source); +} + +struct RvalueAssignment { + RvalueAssignment &operator=(const RvalueAssignment &) &; + RvalueAssignment &operator=(RvalueAssignment &&) &&; +}; +void wrongReceiver(RvalueAssignment &target, RvalueAssignment source) { + target = source; +} + +struct NewException { + int value; + NewException(const NewException &) noexcept; + NewException(NewException &&); +}; +void introducedHandler(NewException source) { + try { + NewException target(source); + } catch (...) { + read(source.value); + } +} +void introducedExceptionWithoutHandler(NewException source) { + NewException target(source); + // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: 'source' could be moved here + // CHECK-FIXES: NewException target(source); +} + +#define COPY_TWICE(x) consumeTwo(x, x) +void repeatedMacro(Value source) { COPY_TWICE(source); } + +struct ConversionOwner { Value field; }; +void fieldGap(ConversionOwner source) { consume(source.field); } +void pointerGap(Value *source) { consume(*source); } + +struct ValueHolder { + ValueHolder(Value); +}; +void byValueConstructor(Value source) { + ValueHolder target(source); + // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: 'source' could be moved here + // CHECK-FIXES: ValueHolder target(source); +} +void constructorAfterLoop() { + Value source; + while (again()) + source.mutate(); + ValueHolder target(source); + // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: 'source' could be moved here + // CHECK-FIXES: ValueHolder target(source); +} +void guardedConstructor(Value source) { + if (source.value) + ValueHolder target(source); + // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: 'source' could be moved here + // CHECK-FIXES: ValueHolder target(source); +} +void fieldDestination(Value source) { + Aggregate target; + target.field = source; + // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: 'source' could be moved here + // CHECK-FIXES: target.field = std::move(source); +} +struct MemberInitializers { + Value first; + Value second; + MemberInitializers(Value source) : first(source), second(source) {} +}; + +void dependentRestoration(Value source) { + consume(source); + source = Value(source); + read(source.value); +} +void restorationThroughAlias(Value source) { + Value &alias = source; + consume(source); // Alias assignment is not currently a restoration proof. + alias = Value{}; + read(source.value); +} diff --git a/clang-tools-extra/test/clang-tidy/checkers/performance/use-std-move-options.cpp b/clang-tools-extra/test/clang-tidy/checkers/performance/use-std-move-options.cpp new file mode 100644 index 0000000000000..4ccd87ec5260e --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/performance/use-std-move-options.cpp @@ -0,0 +1,51 @@ +// RUN: %check_clang_tidy %s performance-use-std-move %t -- -config='{CheckOptions: {performance-use-std-move.AllowedTypes: "Ignored;::view::.*"}}' +// RUN: %check_clang_tidy %s performance-use-std-move %t -- -config='{CheckOptions: {performance-use-std-move.AllowedTypes: "Ignored;::view::.*", performance-use-std-move.IncludeStyle: google}}' +// RUN: %clang -std=c++11 -fsyntax-only -nostdinc++ -isystem %clang_tidy_headers/std %t.cpp + +// CHECK-FIXES: #include <utility> + +struct Movable { + Movable(); + Movable(const Movable &); + Movable(Movable &&); + Movable &operator=(const Movable &); + Movable &operator=(Movable &&); +}; +struct Ignored { + Ignored(); + Ignored(const Ignored &); + Ignored(Ignored &&); + Ignored &operator=(const Ignored &); + Ignored &operator=(Ignored &&); +}; +namespace view { +struct CustomView { + CustomView(); + CustomView(const CustomView &); + CustomView(CustomView &&); +}; +} // namespace view + +void insertUtility() { + Movable source; + Movable target(source); + // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: 'source' could be moved here [performance-use-std-move] + // CHECK-FIXES: Movable target(std::move(source)); +} +void excludedCopy(Ignored source) { + Ignored target(source); + // CHECK-FIXES: Ignored target(source); +} +void excludedAssignment(Ignored &target, Ignored source) { + target = source; + // CHECK-FIXES: target = source; +} +void excludedView(view::CustomView source) { + view::CustomView target(source); + // CHECK-FIXES: view::CustomView target(source); +} +void insertUtilityAssignment(Movable &target, Movable source) { + target = source; + // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: 'source' could be moved here [performance-use-std-move] + // CHECK-FIXES: target = std::move(source); +} diff --git a/clang-tools-extra/test/clang-tidy/checkers/performance/use-std-move.cpp b/clang-tools-extra/test/clang-tidy/checkers/performance/use-std-move.cpp index 87a5c90030d8f..f05c73aecb752 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/performance/use-std-move.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/performance/use-std-move.cpp @@ -59,7 +59,7 @@ void NonProfitableNonTrivialMoveAssignPointer(NonTrivialMoveAssign*& target, Non void ConvertibleNonTrivialMoveAssignFromLValue(NonTrivialMoveAssign& target, NonTrivialMoveAssign&& source) { // CHECK-MESSAGES: [[@LINE+1]]:12: warning: 'source' could be moved here [performance-use-std-move] target = source; - // CHECK-FIXES: target = std::move(source); + // CHECK-FIXES: target = source; } // Check moving already moved values @@ -129,8 +129,9 @@ void ConvertibleNonTrivialMoveAssignShadowed(NoMoveAssign& target, NonTrivialMov #define ASSIGN(x, y) x = y void ConvertibleNonTrivialMoveAssignWithinMacro(NonTrivialMoveAssign& target, NonTrivialMoveAssign source) { - // No message expected, assignment within a macro. ASSIGN(target, source); + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: 'source' could be moved here [performance-use-std-move] + // CHECK-FIXES: ASSIGN(target, source); } template<class T> @@ -239,9 +240,10 @@ void NonConvertibleNonTrivialMoveAssignRValue(NonTrivialMoveAssign& target, NonT target = source; } -void NonProfitableTrivialMoveAssign(TrivialMoveAssign& target, TrivialMoveAssign source) { - // No message expected, moving is possible but pedantic. +void ConvertibleTrivialMoveAssign(TrivialMoveAssign& target, TrivialMoveAssign source) { target = source; + // CHECK-MESSAGES: [[@LINE-1]]:12: warning: 'source' could be moved here [performance-use-std-move] + // CHECK-FIXES: target = std::move(source); } // Check moving in presence of control flow or use >From bccdf94195ab03915345ef139e5f9bb49ae9364b Mon Sep 17 00:00:00 2001 From: Vasily Kulikov <[email protected]> Date: Sun, 4 Oct 2026 00:43:13 +0400 Subject: [PATCH 2/3] touch --- pyproject.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/pyproject.toml b/pyproject.toml index 2992b81d026e7..4a642611a8992 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -28,6 +28,7 @@ executionEnvironments = [ { root = "utils/docs", pythonVersion = "3.8" }, ] + [dependency-groups] dev = [ "psutil>=7.2.2", >From 33a0c812629883a2a79454ce0d1f8af182df7f93 Mon Sep 17 00:00:00 2001 From: Vasily Kulikov <[email protected]> Date: Sun, 4 Oct 2026 00:43:21 +0400 Subject: [PATCH 3/3] revert --- pyproject.toml | 1 - 1 file changed, 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index 4a642611a8992..2992b81d026e7 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -28,7 +28,6 @@ executionEnvironments = [ { root = "utils/docs", pythonVersion = "3.8" }, ] - [dependency-groups] dev = [ "psutil>=7.2.2", _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
