aaron.ballman added a subscriber: aaron.ballman.
================
Comment at: clang-tidy/misc/MoveConstantArgumentCheck.cpp:1
@@ +1,2 @@
+#include "MoveConstantArgumentCheck.h"
+
----------------
Missing new file legal text.
================
Comment at: clang-tidy/misc/MoveConstantArgumentCheck.cpp:9
@@ +8,3 @@
+
+void MoveConstantArgumentCheck::registerMatchers(MatchFinder* Finder) {
+ Finder->addMatcher(
----------------
Formatting -- the * should go with Finder. May want to run clang-format over
the entire patch; there's a lot of other formatting issues I will refrain from
commenting on.
================
Comment at: clang-tidy/misc/MoveConstantArgumentCheck.cpp:10
@@ +9,3 @@
+void MoveConstantArgumentCheck::registerMatchers(MatchFinder* Finder) {
+ Finder->addMatcher(
+ callExpr(callee(functionDecl(hasName("::std::move")))).bind("call-move"),
----------------
Please only register this matcher for C++ code.
================
Comment at: clang-tidy/misc/MoveConstantArgumentCheck.cpp:22
@@ +21,3 @@
+ bool IsTypeDependOnTemplateParameter =
+ false; // my first guess was type->getTypeClass () == 30 but it doesn't
+ // work in some cases. Could you please advice better solution.
----------------
Arg->getType()->isDependentType() should do what you want, if I understand you
properly.
================
Comment at: clang-tidy/misc/MoveConstantArgumentCheck.cpp:29
@@ +28,3 @@
+ if (IsConstArg || IsTriviallyCopyable) {
+ bool IsVariable = dyn_cast<DeclRefExpr>(Arg) != nullptr;
+ std::string message = "std::move of the ";
----------------
isa<DeclRefExpr> instead of dyn_cast and check against nullptr.
================
Comment at: clang-tidy/misc/MoveConstantArgumentCheck.cpp:34
@@ +33,3 @@
+ message += IsTriviallyCopyable ? "of trivially-copyable type " : "";
+ message += "doesn't have effect. ";
+ message += "Remove std::move()";
----------------
This line reads a bit strangely to me. Perhaps "has no effect" instead? Also,
our diagnostics are not full sentences, so you should remove the period, and
instead do: "; remove std::move()"
================
Comment at: clang-tidy/misc/MoveConstantArgumentCheck.h:1
@@ +1,2 @@
+#include "../ClangTidy.h"
+
----------------
Missing header include guards and header legal text.
http://reviews.llvm.org/D12031
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
