[PATCH] D20277: [clang-tidy] UnnecessaryValueParamCheck - suggest std::move() if non-const value parameter can be moved.
Samuel Benzaquen via cfe-commits
cfe-commits at lists.llvm.org
Mon May 16 12:50:11 PDT 2016
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
More information about the cfe-commits
mailing list