Sockke updated this revision to Diff 364404.
Sockke added a comment.

format.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D107450/new/

https://reviews.llvm.org/D107450

Files:
  clang-tools-extra/clang-tidy/performance/MoveConstArgCheck.cpp
  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
@@ -70,7 +70,11 @@
   // CHECK-FIXES: return x3;
 }
 
-A f4(A x4) { return std::move(x4); }
+A f4(A x4) { 
+  return std::move(x4); 
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: passing result of std::move() as a const reference argument; no move will actually happen
+  // CHECK-FIXES: return x4;
+}
 
 A f5(const A x5) {
   return std::move(x5);
@@ -246,3 +250,73 @@
   };
   f(MoveSemantics());
 }
+
+void showInt(int &&) {}
+int testInt() {
+  int a = 10;
+  int b = std::move(a);
+  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: std::move of the variable 'a' of the trivially-copyable type 'int' has no effect; remove std::move()
+  // CHECK-FIXES: int b = a;
+  showInt(std::move(a));
+  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: std::move of the variable 'a' of the trivially-copyable type 'int' has no effect; remove std::move()
+  return std::move(a);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: std::move of the variable 'a' of the trivially-copyable type 'int' has no effect; remove std::move()
+  // CHECK-FIXES: return a;
+}
+
+struct Tmp {};
+void showTmp(Tmp &&) {}
+Tmp testTmp() {
+  Tmp t;
+  Tmp t1 = std::move(t);
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: std::move of the variable 't' of the trivially-copyable type 'Tmp' has no effect; remove std::move()
+  // CHECK-FIXES: Tmp t1 = t;
+  Tmp t2 = std::move(Tmp());
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: std::move of the expression of the trivially-copyable type 'Tmp' has no effect; remove std::move()
+  // CHECK-FIXES: Tmp t2 = Tmp();
+  showTmp(std::move(t));
+  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: std::move of the variable 't' of the trivially-copyable type 'Tmp' has no effect; remove std::move()
+  return std::move(t);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: std::move of the variable 't' of the trivially-copyable type 'Tmp' has no effect; remove std::move()
+  // CHECK-FIXES: return t;
+}
+
+struct Tmp_UnTriviallyC {
+  Tmp_UnTriviallyC() {}
+  Tmp_UnTriviallyC(const Tmp_UnTriviallyC &) {}
+};
+void showTmp_UnTriviallyC(Tmp_UnTriviallyC &&) {}
+Tmp_UnTriviallyC testTmp_UnTriviallyC() {
+  Tmp_UnTriviallyC tn;
+  Tmp_UnTriviallyC tn1 = std::move(tn);
+  // CHECK-MESSAGES: :[[@LINE-1]]:26: warning: passing result of std::move() as a const reference argument; no move will actually happen
+  // CHECK-FIXES: Tmp_UnTriviallyC tn1 = tn;
+  Tmp_UnTriviallyC tn2 = std::move(Tmp_UnTriviallyC());
+  // CHECK-MESSAGES: :[[@LINE-1]]:26: warning: passing result of std::move() as a const reference argument; no move will actually happen
+  // CHECK-FIXES: Tmp_UnTriviallyC tn2 = Tmp_UnTriviallyC();
+  showTmp_UnTriviallyC(std::move(tn));
+  // Expect no warning given here.
+  return std::move(tn);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: passing result of std::move() as a const reference argument; no move will actually happen 
+  // CHECK-FIXES: return tn;
+}
+
+struct Tmp_UnTriviallyCR {
+  Tmp_UnTriviallyCR() {}
+  Tmp_UnTriviallyCR(const Tmp_UnTriviallyCR &) {}
+  Tmp_UnTriviallyCR(Tmp_UnTriviallyCR &&) {}
+};
+void showTmp_UnTriviallyCR(Tmp_UnTriviallyCR &&) {}
+Tmp_UnTriviallyCR testTmp_UnTriviallyCR() {
+  Tmp_UnTriviallyCR tnr;
+  Tmp_UnTriviallyCR tnr1 = std::move(tnr);
+  // Expect no warning given here.
+  Tmp_UnTriviallyCR tnr2 = std::move(Tmp_UnTriviallyCR());
+  // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: passing result of std::move() as a const reference argument; no move will actually happen
+  // CHECK-FIXES: Tmp_UnTriviallyCR tnr2 = Tmp_UnTriviallyCR();
+  showTmp_UnTriviallyCR(std::move(tnr));
+  // Expect no warning given here.
+  return std::move(tnr);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: passing result of std::move() as a const reference argument; no move will actually happen
+  // CHECK-FIXES: return tnr;
+}
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
@@ -45,19 +45,42 @@
                unless(isInTemplateInstantiation()))
           .bind("call-move");
 
-  Finder->addMatcher(MoveCallMatcher, this);
+  Finder->addMatcher(
+      returnStmt(
+          hasReturnValue(anyOf(
+              ignoringImplicit(ignoringParenCasts(MoveCallMatcher)),
+              cxxConstructExpr(hasDeclaration(cxxConstructorDecl(allOf(
+                                   isUserProvided(), isMoveConstructor()))),
+                               hasAnyArgument(ignoringImplicit(
+                                   ignoringParenCasts(MoveCallMatcher)))))))
+          .bind("return-call-move"),
+      this);
+
+  Finder->addMatcher(
+      declStmt(hasSingleDecl(varDecl(hasInitializer(
+                   ignoringImplicit(ignoringParenCasts(MoveCallMatcher))))))
+          .bind("decl-call-move"),
+      this);
 
   Finder->addMatcher(
       invocation(forEachArgumentWithParam(
                      MoveCallMatcher,
-                     parmVarDecl(hasType(references(isConstQualified())))))
+                     parmVarDecl(anyOf(hasType(references(isConstQualified())),
+                                       hasType(rValueReferenceType())))
+                         .bind("invocation-parm")))
           .bind("receiving-expr"),
       this);
 }
 
 void MoveConstArgCheck::check(const MatchFinder::MatchResult &Result) {
   const auto *CallMove = Result.Nodes.getNodeAs<CallExpr>("call-move");
+  const auto *ReturnCallMove =
+      Result.Nodes.getNodeAs<ReturnStmt>("return-call-move");
+  const auto *DeclCallMove = Result.Nodes.getNodeAs<DeclStmt>("decl-call-move");
   const auto *ReceivingExpr = Result.Nodes.getNodeAs<Expr>("receiving-expr");
+  const auto *InvocationParm =
+      Result.Nodes.getNodeAs<ParmVarDecl>("invocation-parm");
+
   const Expr *Arg = CallMove->getArg(0);
   SourceManager &SM = Result.Context->getSourceManager();
 
@@ -102,8 +125,18 @@
                 << (IsConstArg && IsVariable && !IsTriviallyCopyable) << Var
                 << Arg->getType();
 
+    if (ReceivingExpr &&
+        InvocationParm->getOriginalType()->isRValueReferenceType() &&
+        !ReceivingExpr->getType()->isRecordType() && Arg->isLValue())
+      return;
+
     replaceCallWithArg(CallMove, Diag, SM, getLangOpts());
-  } else if (ReceivingExpr) {
+  } else if (ReturnCallMove || DeclCallMove || ReceivingExpr) {
+    if (ReceivingExpr &&
+        InvocationParm->getOriginalType()->isRValueReferenceType() &&
+        Arg->isLValue())
+      return;
+
     auto Diag = diag(FileMoveRange.getBegin(),
                      "passing result of std::move() as a const reference "
                      "argument; no move will actually happen");
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to