[PATCH] D20277: [clang-tidy] UnnecessaryValueParamCheck - suggest std::move() if non-const value parameter can be moved.

[Samuel Benzaquen via cfe-commits]

sbenza added inline comments.

================
Comment at: clang-tidy/performance/UnnecessaryValueParamCheck.cpp:83
@@ +82,3 @@
+  if (!IsConstQualified) {
+    auto Matches = utils::decl_ref_expr::allDeclRefExprs(
+        *Param, *Function->getBody(), *Result.Context);
----------------
We should not call this more than once as it is a potentially expensive operation.
That is, isOnlyUsedAsConst will do it too.
Also, in this case we don't care about _all_. We only care about != 1.

================
Comment at: clang-tidy/performance/UnnecessaryValueParamCheck.cpp:144
@@ +143,3 @@
+  Diag << FixItHint::CreateReplacement(
+      CopyArgument.getSourceRange(),
+      (Twine("std::move(") + Var.getName() + ")").str());
----------------
Prefer two additions over a replacement.
They usually behave better in some conditions.

================
Comment at: clang-tidy/utils/DeclRefExprUtils.cpp:102
@@ +101,3 @@
+  auto Matches =
+      match(findAll(cxxConstructExpr(
+                        UsedAsConstRefArg,
----------------
why findAll() ?
You only care about one.

same as below.

================
Comment at: clang-tidy/utils/TypeTraits.cpp:131
@@ +130,3 @@
+  for (const auto *Constructor : Record->ctors()) {
+    if (Constructor->isMoveConstructor() && !Constructor->isDeleted())
+      return true;
----------------
maybe also check that it is not trivial?

below too.


Repository:
  rL LLVM

http://reviews.llvm.org/D20277