AMS21 created this revision.
AMS21 added reviewers: PiotrZSL, carlosgalvezp.
Herald added a subscriber: xazax.hun.
Herald added a reviewer: njames93.
Herald added a project: All.
AMS21 requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.
We now display a simple note if the reason is that the used class does not
support move semantics.
This fixes llvm#62550
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D153220
Files:
clang-tools-extra/clang-tidy/performance/MoveConstArgCheck.cpp
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/test/clang-tidy/checkers/performance/move-const-arg.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/performance/move-const-arg.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/performance/move-const-arg.cpp
+++ 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
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ 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
+ <clang-tidy/checks/performance/move-const-arg>`: warning when no move
+ constructor is available.
+
- Improved :doc:`performance-no-automatic-move
<clang-tidy/checks/performance/no-automatic-move>`: warn on ``const &&``
constructors.
Index: clang-tools-extra/clang-tidy/performance/MoveConstArgCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/performance/MoveConstArgCheck.cpp
+++ 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();
}
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
