diff --git a/clang-tools-extra/clang-tidy/performance/MoveConstArgCheck.cpp b/clang-tools-extra/clang-tidy/performance/MoveConstArgCheck.cpp --- a/clang-tools-extra/clang-tidy/performance/MoveConstArgCheck.cpp +++ b/clang-tools-extra/clang-tidy/performance/MoveConstArgCheck.cpp @@ -103,6 +103,7 @@ AlreadyCheckedMoves.insert(CallMove); const Expr *Arg = CallMove->getArg(0); + const QualType ArgType = Arg->getType(); SourceManager &SM = Result.Context->getSourceManager(); CharSourceRange MoveRange = @@ -112,12 +113,11 @@ if (!FileMoveRange.isValid()) return; - bool IsConstArg = Arg->getType().isConstQualified(); - bool IsTriviallyCopyable = - Arg->getType().isTriviallyCopyableType(*Result.Context); + bool IsConstArg = ArgType.isConstQualified(); + bool IsTriviallyCopyable = ArgType.isTriviallyCopyableType(*Result.Context); if (IsConstArg || IsTriviallyCopyable) { - if (const CXXRecordDecl *R = Arg->getType()->getAsCXXRecordDecl()) { + if (const CXXRecordDecl *R = ArgType->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. @@ -151,7 +151,7 @@ << IsConstArg << IsVariable << IsTriviallyCopyable << IsRVRefParam << (IsConstArg && IsVariable && !IsTriviallyCopyable) << Var - << Arg->getType(); + << ArgType; if (!IsRVRefParam) replaceCallWithArg(CallMove, Diag, SM, getLangOpts()); } @@ -196,11 +196,19 @@ if ((*InvocationParmType)->isRValueReferenceType()) return; - auto Diag = diag(FileMoveRange.getBegin(), - "passing result of std::move() as a const reference " - "argument; no move will actually happen"); + { + 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()); + } - replaceCallWithArg(CallMove, Diag, SM, getLangOpts()); + if (const CXXRecordDecl *RecordDecl = ArgType->getAsCXXRecordDecl(); + !RecordDecl->hasMoveConstructor() || !RecordDecl->hasMoveAssignment()) + diag(RecordDecl->getSourceRange().getBegin(), + "'%0' is not move constructible", DiagnosticIDs::Note) + << RecordDecl->getName(); } } diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -374,6 +374,10 @@ `IgnoreTemplateInstantiations` option to optionally ignore virtual function overrides that are part of template instantiations. +- Improved :doc:`performance-move-const-arg + `: warning when no move + constructor is available. + - Improved :doc:`performance-no-automatic-move `: warn on ``const &&`` constructors. diff --git a/clang-tools-extra/test/clang-tidy/checkers/performance/move-const-arg.cpp b/clang-tools-extra/test/clang-tidy/checkers/performance/move-const-arg.cpp --- a/clang-tools-extra/test/clang-tidy/checkers/performance/move-const-arg.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/performance/move-const-arg.cpp @@ -340,3 +340,51 @@ // CHECK-FIXES: choice(a, std::move(a)); // CHECK-MESSAGES: :[[@LINE-3]]:24: warning: std::move of the variable 'a' of the trivially-copyable type 'int' has no effect [performance-move-const-arg] } + +namespace issue_62550 { + +struct NonMoveConstructable { + NonMoveConstructable(); + NonMoveConstructable(const NonMoveConstructable&); + NonMoveConstructable& operator=(const NonMoveConstructable&); + NonMoveConstructable& operator=(NonMoveConstructable&&); +}; + +void testNonMoveConstructible() { + NonMoveConstructable t1; + NonMoveConstructable t2{std::move(t1)}; + // CHECK-MESSAGES: :[[@LINE-1]]:27: warning: passing result of std::move() as a const reference argument; no move will actually happen [performance-move-const-arg] +} + +struct NonMoveAssignable { + NonMoveAssignable(); + NonMoveAssignable(const NonMoveAssignable&); + NonMoveAssignable(NonMoveAssignable&&); + + NonMoveAssignable& operator=(const NonMoveAssignable&); +}; + +void testNonMoveAssignable() { + NonMoveAssignable t1; + NonMoveAssignable t2; + + t2 = std::move(t1); + // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: passing result of std::move() as a const reference argument; no move will actually happen [performance-move-const-arg] +} + +struct NonMoveable { + NonMoveable(); + NonMoveable(const NonMoveable&); + NonMoveable& operator=(const NonMoveable&); +}; + +void testNonMoveable() { + NonMoveable t1; + NonMoveable t2{std::move(t1)}; + // 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] + + t1 = std::move(t2); + // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: passing result of std::move() as a const reference argument; no move will actually happen [performance-move-const-arg] +} + +} // namespace issue_62550