aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land.
LGTM aside from a small commenting nit. ================ Comment at: clang-tools-extra/clang-tidy/modernize/PassByValueCheck.cpp:194 + // If we received a `const&` type, we need to rewrite the function + // declarations + if (ParamDecl->getType()->isLValueReferenceType()) { ---------------- ================ Comment at: clang-tools-extra/clang-tidy/modernize/PassByValueCheck.cpp:198 + for (const ParmVarDecl *ParmDecl : collectParamDecls(Ctor, ParamDecl)) { + auto ParamTL = ParmDecl->getTypeSourceInfo()->getTypeLoc(); + auto RefTL = ParamTL.getAs<ReferenceTypeLoc>(); ---------------- avogelsgesang wrote: > aaron.ballman wrote: > > Please spell out this type (per our usual coding conventions). The use on > > the next line is fine because the type name is spelled out in the > > initialization. > Thanks for the hint. I wasn't aware of that convention. I fixed it here. > > Should I also fix the other violations in this file? > > E.g., a couple of lines above, we have > > ``` > auto Diag = diag(ParamDecl->getBeginLoc(), "pass by value and use std::move"); > ``` > > which I guess should be > > ``` > clang::DiagnosticBuilder Diag = diag(ParamDecl->getBeginLoc(), "pass by value > and use std::move"); > ``` > Thanks for the hint. I wasn't aware of that convention. No problem! In case you haven't seen it, we document our coding style here: https://llvm.org/docs/CodingStandards.html > Should I also fix the other violations in this file? If you want to, you can do it as an NFC change separate from these changes, but I certainly don't insist. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112648/new/ https://reviews.llvm.org/D112648 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits