PiotrZSL added inline comments.
================ Comment at: clang-tools-extra/clang-tidy/utils/FormatStringConverter.h:73 + // puts the width and preicision first. + std::vector<std::tuple<unsigned, unsigned>> ArgRotates; + ---------------- mikecrowe wrote: > PiotrZSL wrote: > > mikecrowe wrote: > > > PiotrZSL wrote: > > > > NOTE: You can use std::pair here. > > > True, but in my mind `std::pair` only exists because `std::tuple` > > > couldn't be implemented in the C++98. > > > > > > I was going to change it to use `std::pair` anyway, but I notice that I > > > already use `std::tuple` for `ArgFixes` just above. Should I change both > > > of them? > > It will be compiled to same thing anyway. So it's up to you. Keep it > > consistent. > > I personally use std::pair for 2 arguments, and std::tuple for more than 2. > > But to be honest probably better would be to introduce some structure so > > that those two `unsigned` could be named. And probably same for ArgFixes, > > but there we got different types, so it's not a big problem. > I think that you're probably right, but the names won't be used from the call > site since I use structured binding there. Do you mind if I do both together > in a separate commit? Not a problem, lets leave it for a next change. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154283/new/ https://reviews.llvm.org/D154283 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
