[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