[PATCH] D92267: [clang-tidy][NFC] Use moves instead of copies when constructing OptionsProviders.

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone added inline comments.


================
Comment at: clang-tools-extra/clang-tidy/ClangTidyOptions.h:225
+                          ClangTidyOptions &&OverrideOptions,
+                          ConfigFileHandlers &&ConfigHandlers);
 
----------------
njames93 wrote:
> Quuxplusone wrote:
> > I'd strongly recommend doing these two signatures in exactly the same way as you've done the others: pass by value and `std::move` out of it. Pass-by-rvalue-reference should be reserved for arcane stuff like hand-written move-constructors. You don't need pass-by-rvalue-reference here.
> While I agree that rvalues in constructors is usually suspicious, in this case it does make sense.
> Firstly, FileOptionsBaseProviders constructors aren't exposed to the public interface, so we don't really need to worry on that front.
> Using r-values saves unneeded move constructor calls, which given that ClangTidyGlobalOptions and ClangTidyOptions have non-trivial most constructors, this is a slight win.
> Is it still better to just pass by value though??
Well, IMHO it's better to just pass by value (for clarity). But it doesn't matter much either way. IMHO the "performance" angle doesn't matter because optimizing compilers, so basically at this point we're trading off "code clarity" versus "amount of bikeshedding in this PR," and I'm certainly willing to stop my bikeshedding, either way. :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92267