[PATCH] D22220: [clang-tidy] Add check 'misc-move-forwarding-reference'

[Piotr Padlewski via cfe-commits]

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