Sockke updated this revision to Diff 367474.
Sockke marked 2 inline comments as done.
Sockke added a comment.
Herald added a subscriber: jfb.
Thanks for your reply @Quuxplusone. I have updated!
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/clang-tidy/performance/MoveConstArgCheck.h
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
@@ -246,3 +246,27 @@
};
f(MoveSemantics());
}
+
+void showInt(int &&) {}
+void testInt() {
+ int a = 10;
+ 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; consider changing showInt's parameter from 'int &&' to 'const int &'
+}
+template <class T>
+void forwardToShowInt(T &&t) {
+ showInt(static_cast<T &&>(t));
+}
+void testTemplate() {
+ int a = 10;
+ forwardToShowInt(std::move(a));
+ // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: std::move of the variable 'a' of the trivially-copyable type 'int' has no effect
+}
+
+struct Tmp {};
+void showTmp(Tmp &&) {}
+void testTmp() {
+ Tmp t;
+ 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; consider changing showTmp's parameter from 'Tmp &&' to 'const Tmp &'
+}
Index: clang-tools-extra/clang-tidy/performance/MoveConstArgCheck.h
===================================================================
--- clang-tools-extra/clang-tidy/performance/MoveConstArgCheck.h
+++ clang-tools-extra/clang-tidy/performance/MoveConstArgCheck.h
@@ -10,6 +10,7 @@
#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_MOVECONSTANTARGUMENTCHECK_H
#include "../ClangTidyCheck.h"
+#include "llvm/ADT/DenseSet.h"
namespace clang {
namespace tidy {
@@ -36,6 +37,7 @@
private:
const bool CheckTriviallyCopyableMove;
+ llvm::DenseSet<const CallExpr *> HasCheckedMoveSet;
};
} // namespace performance
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
@@ -50,7 +50,9 @@
Finder->addMatcher(
invocation(forEachArgumentWithParam(
MoveCallMatcher,
- parmVarDecl(hasType(references(isConstQualified())))))
+ parmVarDecl(anyOf(hasType(references(isConstQualified())),
+ hasType(rValueReferenceType())))
+ .bind("invocation-parm")))
.bind("receiving-expr"),
this);
}
@@ -58,6 +60,15 @@
void MoveConstArgCheck::check(const MatchFinder::MatchResult &Result) {
const auto *CallMove = Result.Nodes.getNodeAs<CallExpr>("call-move");
const auto *ReceivingExpr = Result.Nodes.getNodeAs<Expr>("receiving-expr");
+ const auto *InvocationParm =
+ Result.Nodes.getNodeAs<ParmVarDecl>("invocation-parm");
+
+ if (!ReceivingExpr && HasCheckedMoveSet.contains(CallMove))
+ return;
+
+ if (ReceivingExpr)
+ HasCheckedMoveSet.insert(CallMove);
+
const Expr *Arg = CallMove->getArg(0);
SourceManager &SM = Result.Context->getSourceManager();
@@ -90,20 +101,57 @@
return;
bool IsVariable = isa<DeclRefExpr>(Arg);
+ bool IsRValueReferenceArg = false;
+ bool IsTemplateInstantiation = false;
+ StringRef FuncName;
+ QualType ParmType;
+ std::string ExpectParmTypeName;
const auto *Var =
IsVariable ? dyn_cast<DeclRefExpr>(Arg)->getDecl() : nullptr;
+
+ if (ReceivingExpr &&
+ InvocationParm->getOriginalType()->isRValueReferenceType() &&
+ !ReceivingExpr->getType()->isRecordType() && Arg->isLValue()) {
+ IsRValueReferenceArg = true;
+ const auto *ReceivingCallExpr = dyn_cast<CallExpr>(ReceivingExpr);
+ if (!ReceivingCallExpr)
+ return;
+ IsTemplateInstantiation =
+ ReceivingCallExpr->getDirectCallee()->isTemplateInstantiation();
+ FuncName = ReceivingCallExpr->getDirectCallee()->getName();
+ ParmType = InvocationParm->getOriginalType();
+ if (Arg->getType()->isRecordType()) {
+ if (const CXXRecordDecl *R = Arg->getType()->getAsCXXRecordDecl())
+ ExpectParmTypeName = R->getNameAsString();
+ } else
+ ExpectParmTypeName =
+ Arg->getType().getAtomicUnqualifiedType().getAsString();
+ }
+
auto Diag = diag(FileMoveRange.getBegin(),
"std::move of the %select{|const }0"
- "%select{expression|variable %4}1 "
- "%select{|of the trivially-copyable type %5 }2"
- "has no effect; remove std::move()"
- "%select{| or make the variable non-const}3")
+ "%select{expression|variable %5}1 "
+ "%select{|of the trivially-copyable type %6 }2"
+ "has no effect%select{; remove std::move()||; consider "
+ "changing %7's parameter from %8 to 'const %9 &'}3"
+ "%select{| or make the variable non-const}4")
<< IsConstArg << IsVariable << IsTriviallyCopyable
- << (IsConstArg && IsVariable && !IsTriviallyCopyable) << Var
- << Arg->getType();
+ << (IsRValueReferenceArg + (IsRValueReferenceArg &&
+ IsTriviallyCopyable &&
+ !IsTemplateInstantiation))
+ << (IsConstArg && IsVariable && !IsTriviallyCopyable &&
+ !IsRValueReferenceArg)
+ << Var << Arg->getType() << FuncName << ParmType
+ << ExpectParmTypeName;
+
+ if (IsRValueReferenceArg)
+ return;
replaceCallWithArg(CallMove, Diag, SM, getLangOpts());
} else if (ReceivingExpr) {
+ if (InvocationParm->getOriginalType()->isRValueReferenceType())
+ 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
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
