diff --git a/clang-tools-extra/clang-tidy/modernize/PassByValueCheck.cpp b/clang-tools-extra/clang-tidy/modernize/PassByValueCheck.cpp --- a/clang-tools-extra/clang-tidy/modernize/PassByValueCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/PassByValueCheck.cpp @@ -105,6 +105,75 @@ 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 @@ -188,6 +257,10 @@ *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 diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize-pass-by-value.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize-pass-by-value.cpp --- a/clang-tools-extra/test/clang-tidy/checkers/modernize-pass-by-value.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize-pass-by-value.cpp @@ -246,3 +246,20 @@ 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; +};