[clang-tidy] Fix false positive in modernize-pass-by-value

The check should not trigger on lvalue/rvalue overload pairs:

```
struct S {
  S(const A& a) : a(a) {}
  S(A&& a) : a(std::move(a)) {}

  A a;
}
```

Differential Revision: https://reviews.llvm.org/D116535
This commit is contained in:
Clement Courbet 2022-01-03 14:25:29 +01:00
parent 3a2393795f
commit ed8ff29aa6
2 changed files with 90 additions and 0 deletions

View File

@ -105,6 +105,75 @@ static bool paramReferredExactlyOnce(const CXXConstructorDecl *Ctor,
return ExactlyOneUsageVisitor(ParamDecl).hasExactlyOneUsageIn(Ctor);
}
/// Returns true if the given constructor is part of a lvalue/rvalue reference
/// pair, i.e. `Param` is of lvalue reference type, and there exists another
/// constructor such that:
/// - it has the same number of parameters as `Ctor`.
/// - the parameter at the same index as `Param` is an rvalue reference
/// of the same pointee type
/// - all other parameters have the same type as the corresponding parameter in
/// `Ctor` or are rvalue references with the same pointee type.
/// Examples:
/// A::A(const B& Param)
/// A::A(B&&)
///
/// A::A(const B& Param, const C&)
/// A::A(B&& Param, C&&)
///
/// A::A(const B&, const C& Param)
/// A::A(B&&, C&& Param)
///
/// A::A(const B&, const C& Param)
/// A::A(const B&, C&& Param)
///
/// A::A(const B& Param, int)
/// A::A(B&& Param, int)
static bool hasRValueOverload(const CXXConstructorDecl *Ctor,
const ParmVarDecl *Param) {
if (!Param->getType().getCanonicalType()->isLValueReferenceType()) {
// The parameter is passed by value.
return false;
}
const int ParamIdx = Param->getFunctionScopeIndex();
const CXXRecordDecl *Record = Ctor->getParent();
// Check whether a ctor `C` forms a pair with `Ctor` under the aforementionned
// rules.
const auto IsRValueOverload = [&Ctor, ParamIdx](const CXXConstructorDecl *C) {
if (C == Ctor || C->isDeleted() ||
C->getNumParams() != Ctor->getNumParams())
return false;
for (int I = 0, E = C->getNumParams(); I < E; ++I) {
const clang::QualType CandidateParamType =
C->parameters()[I]->getType().getCanonicalType();
const clang::QualType CtorParamType =
Ctor->parameters()[I]->getType().getCanonicalType();
const bool IsLValueRValuePair =
CtorParamType->isLValueReferenceType() &&
CandidateParamType->isRValueReferenceType() &&
CandidateParamType->getPointeeType()->getUnqualifiedDesugaredType() ==
CtorParamType->getPointeeType()->getUnqualifiedDesugaredType();
if (I == ParamIdx) {
// The parameter of interest must be paired.
if (!IsLValueRValuePair)
return false;
} else {
// All other parameters can be similar or paired.
if (!(CandidateParamType == CtorParamType || IsLValueRValuePair))
return false;
}
}
return true;
};
for (const auto *Candidate : Record->ctors()) {
if (IsRValueOverload(Candidate)) {
return true;
}
}
return false;
}
/// Find all references to \p ParamDecl across all of the
/// redeclarations of \p Ctor.
static SmallVector<const ParmVarDecl *, 2>
@ -188,6 +257,10 @@ void PassByValueCheck::check(const MatchFinder::MatchResult &Result) {
*Result.Context))
return;
// Do not trigger if we find a paired constructor with an rvalue.
if (hasRValueOverload(Ctor, ParamDecl))
return;
auto Diag = diag(ParamDecl->getBeginLoc(), "pass by value and use std::move");
// If we received a `const&` type, we need to rewrite the function

View File

@ -246,3 +246,20 @@ struct V {
V::V(const Movable &M) : M(M) {}
// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: pass by value and use std::move
// CHECK-FIXES: V::V(const Movable &M) : M(M) {}
// Test with paired lvalue/rvalue overloads.
struct W1 {
W1(const Movable &M) : M(M) {}
W1(Movable &&M);
Movable M;
};
struct W2 {
W2(const Movable &M, int) : M(M) {}
W2(Movable &&M, int);
Movable M;
};
struct W3 {
W3(const W1 &, const Movable &M) : M(M) {}
W3(W1 &&, Movable &&M);
Movable M;
};