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

Arthur O'Dwyer via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Sat Nov 28 07:00:39 PST 2020


Quuxplusone added inline comments.


================
Comment at: clang-tools-extra/clang-tidy/ClangTidyOptions.h:176
 public:
-  DefaultOptionsProvider(const ClangTidyGlobalOptions &GlobalOptions,
-                         const ClangTidyOptions &Options)
-      : GlobalOptions(GlobalOptions), DefaultOptions(Options) {}
+  DefaultOptionsProvider(ClangTidyGlobalOptions GlobalOptions,
+                         ClangTidyOptions Options)
----------------
If you're refactoring ctors anyway, perhaps add `explicit` to all of them? (The only reason to have a non-`explicit` ctor is if you want to enable callers to write `DefaultOptionsProvider d = {gopt, opt};` instead of `auto d = DefaultOptionsProvider(gopt, opt);`.) But this would be scope creep, of course.


================
Comment at: clang-tools-extra/clang-tidy/ClangTidyOptions.h:225
+                          ClangTidyOptions &&OverrideOptions,
+                          ConfigFileHandlers &&ConfigHandlers);
 
----------------
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92267



More information about the cfe-commits mailing list