dvadym updated the summary for this revision.
dvadym updated this revision to Diff 39077.
dvadym marked 6 inline comments as done.
dvadym added a comment.
1.Most comments addressed
2.Taking into consideration applying move to trivially copyable objects
3.Different message if move argument variable or expression.
It's not addressed yet case when move argument is depend in some way on
template argument. There is comment in code about this.
http://reviews.llvm.org/D12031
Files:
clang-tidy/misc/CMakeLists.txt
clang-tidy/misc/MiscTidyModule.cpp
clang-tidy/misc/MoveConstantArgumentCheck.cpp
clang-tidy/misc/MoveConstantArgumentCheck.h
test/clang-tidy/move-const-arg.cpp
Index: test/clang-tidy/move-const-arg.cpp
===================================================================
--- test/clang-tidy/move-const-arg.cpp
+++ test/clang-tidy/move-const-arg.cpp
@@ -0,0 +1,72 @@
+// RUN: $(dirname %s)/check_clang_tidy.sh %s misc-move-const-arg %t
+// REQUIRES: shell
+
+namespace std {
+// Directly copied from the stl.
+template<typename>
+struct remove_reference;
+
+template<typename _Tp>
+ struct remove_reference
+ { typedef _Tp type; };
+
+template<typename _Tp>
+ struct remove_reference<_Tp&>
+ { typedef _Tp type; };
+
+template<typename _Tp>
+ struct remove_reference<_Tp&&>
+ { typedef _Tp type; };
+
+template<typename _Tp>
+ constexpr typename std::remove_reference<_Tp>::type&&
+ move(_Tp&& __t);
+
+} // namespace std
+
+class A
+{
+ public:
+ A() {}
+ A(const A& rhs) {}
+ A(A&& rhs) {}
+};
+
+int f1()
+{
+ return std::move(42);
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: std::move of the expression of trivially-copyable type doesn't have effect. Remove std::move().
+ // CHECK-FIXES: return 42;
+}
+
+int f2(int x2)
+{
+ return std::move(x2);
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: std::move of the variable of trivially-copyable type doesn't have effect. Remove std::move().
+ // CHECK-FIXES: return x2;
+}
+
+int* f3(int* x3)
+{
+ return std::move(x3);
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: std::move of the expression of trivially-copyable type doesn't have effect. Remove std::move().
+ // CHECK-FIXES: return x3;
+}
+
+A f4(A x4)
+{
+ return std::move(x4);
+}
+
+A f5(const A x5)
+{
+ return std::move(x5);
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: std::move of the const variable doesn't have effect. Remove std::move() or make the variable non-const.
+ // CHECK-FIXES: return x5;
+}
+
+template<typename T>
+T f6(const T a)
+{
+ return std::move(a);
+}
Index: clang-tidy/misc/MoveConstantArgumentCheck.h
===================================================================
--- clang-tidy/misc/MoveConstantArgumentCheck.h
+++ clang-tidy/misc/MoveConstantArgumentCheck.h
@@ -0,0 +1,17 @@
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+class MoveConstantArgumentCheck : public ClangTidyCheck {
+public:
+ MoveConstantArgumentCheck(StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context) {}
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace misc
+} // namespace tidy
+} // namespace clang
Index: clang-tidy/misc/MoveConstantArgumentCheck.cpp
===================================================================
--- clang-tidy/misc/MoveConstantArgumentCheck.cpp
+++ clang-tidy/misc/MoveConstantArgumentCheck.cpp
@@ -0,0 +1,51 @@
+#include "MoveConstantArgumentCheck.h"
+
+#include <iostream>
+using namespace std;
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+using namespace ast_matchers;
+
+void MoveConstantArgumentCheck::registerMatchers(MatchFinder* Finder) {
+ Finder->addMatcher(
+ callExpr(callee(functionDecl(hasName("::std::move")))).bind("call-move"),
+ this);
+}
+
+void MoveConstantArgumentCheck::check(const MatchFinder::MatchResult& Result) {
+ const auto* CallMove = Result.Nodes.getNodeAs<CallExpr>("call-move");
+ if (CallMove->getNumArgs() != 1) return;
+ const Expr* Arg = CallMove->getArg(0);
+ auto* Context = Result.Context;
+
+ bool IsTypeDependOnTemplateParameter =
+ false; // my first guess was type->getTypeClass () == 30 but it doesn't
+ // work in some cases. Could you please advice better solution.
+ if (IsTypeDependOnTemplateParameter) return;
+ bool IsConstArg = Arg->getType().isConstQualified();
+ bool IsTriviallyCopyable = Arg->getType().isTriviallyCopyableType(*Context);
+
+ if (IsConstArg || IsTriviallyCopyable) {
+ bool IsVariable = (dyn_cast<DeclRefExpr>(Arg) != nullptr);
+ string message = "std::move of the ";
+ message += IsConstArg ? "const " : "";
+ message += IsVariable ? "variable " : "expression ";
+ message += IsTriviallyCopyable ? "of trivially-copyable type " : "";
+ message += "doesn't have effect. ";
+ message += "Remove std::move()";
+ message += IsConstArg && IsVariable ? " or make the variable non-const." : ".";
+
+ SourceRange RemoveRange1(CallMove->getLocStart(), Arg->getLocStart());
+ SourceRange RemoveRange2(CallMove->getLocEnd(), CallMove->getLocEnd());
+ diag(CallMove->getLocStart(), message)
+ << FixItHint::CreateRemoval(RemoveRange1)
+ << FixItHint::CreateRemoval(RemoveRange2);
+ }
+}
+
+} // namespace misc
+} // namespace tidy
+} // namespace clang
Index: clang-tidy/misc/MiscTidyModule.cpp
===================================================================
--- clang-tidy/misc/MiscTidyModule.cpp
+++ clang-tidy/misc/MiscTidyModule.cpp
@@ -18,6 +18,7 @@
#include "InefficientAlgorithmCheck.h"
#include "MacroParenthesesCheck.h"
#include "MacroRepeatedSideEffectsCheck.h"
+#include "MoveConstantArgumentCheck.h"
#include "NoexceptMoveConstructorCheck.h"
#include "StaticAssertCheck.h"
#include "SwappedArgumentsCheck.h"
@@ -50,6 +51,8 @@
"misc-macro-parentheses");
CheckFactories.registerCheck<MacroRepeatedSideEffectsCheck>(
"misc-macro-repeated-side-effects");
+ CheckFactories.registerCheck<MoveConstantArgumentCheck>(
+ "misc-move-const-arg");
CheckFactories.registerCheck<NoexceptMoveConstructorCheck>(
"misc-noexcept-move-constructor");
CheckFactories.registerCheck<StaticAssertCheck>(
Index: clang-tidy/misc/CMakeLists.txt
===================================================================
--- clang-tidy/misc/CMakeLists.txt
+++ clang-tidy/misc/CMakeLists.txt
@@ -10,6 +10,7 @@
MacroParenthesesCheck.cpp
MacroRepeatedSideEffectsCheck.cpp
MiscTidyModule.cpp
+ MoveConstantArgumentCheck.cpp
NoexceptMoveConstructorCheck.cpp
StaticAssertCheck.cpp
SwappedArgumentsCheck.cpp
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
