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 @@ -190,24 +190,35 @@ auto Diag = diag(ParamDecl->getBeginLoc(), "pass by value and use std::move"); - // Iterate over all declarations of the constructor. - for (const ParmVarDecl *ParmDecl : collectParamDecls(Ctor, ParamDecl)) { - auto ParamTL = ParmDecl->getTypeSourceInfo()->getTypeLoc(); - auto RefTL = ParamTL.getAs(); + // If we received a `const&` type, we need to rewrite the function + // declarations + if (ParamDecl->getType()->isLValueReferenceType()) { + // Check if we can succesfully rewrite all declarations of the constructor. + for (const ParmVarDecl *ParmDecl : collectParamDecls(Ctor, ParamDecl)) { + TypeLoc ParamTL = ParmDecl->getTypeSourceInfo()->getTypeLoc(); + ReferenceTypeLoc RefTL = ParamTL.getAs(); + if (RefTL.isNull()) { + // We cannot rewrite this instance. The type is probably hidden behind + // some `typedef`. Do not offer a fix-it in this case. + return; + } + } + // Rewrite all declarations. + for (const ParmVarDecl *ParmDecl : collectParamDecls(Ctor, ParamDecl)) { + TypeLoc ParamTL = ParmDecl->getTypeSourceInfo()->getTypeLoc(); + ReferenceTypeLoc RefTL = ParamTL.getAs(); - // Do not replace if it is already a value, skip. - if (RefTL.isNull()) - continue; - - TypeLoc ValueTL = RefTL.getPointeeLoc(); - auto TypeRange = CharSourceRange::getTokenRange(ParmDecl->getBeginLoc(), - ParamTL.getEndLoc()); - std::string ValueStr = Lexer::getSourceText(CharSourceRange::getTokenRange( - ValueTL.getSourceRange()), - SM, getLangOpts()) - .str(); - ValueStr += ' '; - Diag << FixItHint::CreateReplacement(TypeRange, ValueStr); + TypeLoc ValueTL = RefTL.getPointeeLoc(); + CharSourceRange TypeRange = CharSourceRange::getTokenRange( + ParmDecl->getBeginLoc(), ParamTL.getEndLoc()); + std::string ValueStr = + Lexer::getSourceText( + CharSourceRange::getTokenRange(ValueTL.getSourceRange()), SM, + getLangOpts()) + .str(); + ValueStr += ' '; + Diag << FixItHint::CreateReplacement(TypeRange, ValueStr); + } } // Use std::move in the initialization list. 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 @@ -231,5 +231,18 @@ struct U { U(const POD &M) : M(M) {} + // CHECK-FIXES: U(const POD &M) : M(M) {} POD M; }; + +// The rewrite can't look through `typedefs` and `using`. +// Test that we don't partially rewrite one decl without rewriting the other. +using MovableConstRef = const Movable &; +struct V { + V(MovableConstRef M); + // CHECK-FIXES: V(MovableConstRef M); + Movable M; +}; +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) {}