[PATCH] D112648: [clang-tidy] Don't offer partial fix-its for `modernize-pass-by-value`

[Aaron Ballman via Phabricator via cfe-commits]

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