Prazek added a subscriber: Prazek.
Prazek added a comment.
Very usefull check!
I don't have enough time right now to check everything, so better wait for
review of Alexfh or someone else. I just wanted to leave some thoughts.
================
Comment at: clang-tidy/misc/MoveForwardingReferenceCheck.cpp:34
@@ +33,3 @@
+ if (CallRange.isValid()) {
+ const std::string ForwardName =
+ "forward<" + TypeParmType->getDecl()->getName().str() + ">";
----------------
you could probably use llvm::StringRef here, but I am not sure about it - ask
Alex.
================
Comment at: clang-tidy/misc/MoveForwardingReferenceCheck.cpp:71
@@ +70,3 @@
+void MoveForwardingReferenceCheck::registerMatchers(MatchFinder *Finder) {
+ if (!getLangOpts().CPlusPlus)
+ return;
----------------
Use CPlusPlus11 here
================
Comment at: clang-tidy/misc/MoveForwardingReferenceCheck.cpp:92
@@ +91,3 @@
+ argumentCountIs(1),
+ hasArgument(0, ignoringParenCasts(declRefExpr(
+ to(ForwardingReferenceParmMatcher)))))
----------------
maybe use ignoringImplicit or if you won't need that use ignoringParenImpCasts
(or something similar).
I am not sure if it is needed it, but it might help in some cases.
================
Comment at: clang-tidy/misc/MoveForwardingReferenceCheck.h:19
@@ +18,3 @@
+
+class MoveForwardingReferenceCheck : public ClangTidyCheck {
+public:
----------------
Please add doc comment.
================
Comment at: docs/clang-tidy/checks/misc-move-forwarding-reference.rst:29
@@ +28,3 @@
+
+Background
+----------
----------------
Very nice section! good idea.
So I have a thoughts about multiple sections in documentation (which is not a
issue for you).
I think the check lists doc should not include sections - it doesn't look good
and it also prevents people from using sections in docs.
================
Comment at: test/clang-tidy/misc-move-forwarding-reference.cpp:68
@@ +67,2 @@
+ void f(U &&SomeU) { T SomeT(std::move(SomeU)); }
+};
----------------
Please add some tests that checks if fixes doesn't happen inside macro.
http://reviews.llvm.org/D22220
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
