[PATCH] D71182: [NFC][llvm][MIRVRegNamerUtils] Making some stylistic changes to MIRVRegNamerUtils.cpp

Justin Bogner via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Dec 9 15:21:32 PST 2019


bogner accepted this revision.
bogner added a comment.
This revision is now accepted and ready to land.

In D71182#1776145 <https://reviews.llvm.org/D71182#1776145>, @plotfi wrote:

> In D71182#1775875 <https://reviews.llvm.org/D71182#1775875>, @compnerd wrote:
>
> > The `std::transform` better expresses the intent IMO.  I think that it is definitely more idiomatic C++, so I think that is an improvement.
>
>
> I agree with this. Was going through the changes in this util in my downtime and just saw some opportunities to spruce some things up. I find it really odd that you'd have a HashOperand lambda and not want to apply that to your collection of uses from a llvm::transform.


FWIW I'm not really convinced that `std::transform` is *ever* clearer or better any more since `C++11` gave us `auto` and range-for.

In any case, the rest of this all seems like a nice clean up to me.



================
Comment at: llvm/lib/CodeGen/MIRVRegNamerUtils.cpp:129
   std::string Temp(Name);
-  std::transform(Temp.begin(), Temp.end(), Temp.begin(), ::tolower);
+  llvm::transform(Temp, Temp.begin(), ::tolower);
   if (auto RC = MRI.getRegClassOrNull(VReg))
----------------
Slightly simpler to use a `StringRef` param and then this is just `std::string Temp(Name.lower())`


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71182/new/

https://reviews.llvm.org/D71182





More information about the llvm-commits mailing list