[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