njames93 added inline comments.
================
Comment at: clang-tools-extra/clang-tidy/ClangTidy.h:61-69
struct NamesAndOptions {
llvm::StringSet<> Names;
llvm::StringSet<> Options;
+ std::vector<ClangTidyError> Errors;
};
NamesAndOptions
----------------
I don't think this is the correct approach here
`getAllChecksAndOptions` should instead return an
`llvm::Expected<NamesAndOptions>`.
You can create an error class that wraps a `std::vector<ClangTidyError>` and
then still get the same behaviour on the error path.
================
Comment at: clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp:528
+
+static llvm::StringSet<> findSourcesForDiagnostic(
+ const std::vector<ClangTidyOptionsProvider::OptionsSource> &RawOptions,
----------------
Given how options are read, we only care about the last option source that
defined the option
Therefore this should be changed to return a `std::optional<StringRef>` and the
for loop below can just search the RawOptions backwards for the first match,
then return that.
================
Comment at: clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp:529
+static llvm::StringSet<> findSourcesForDiagnostic(
+ const std::vector<ClangTidyOptionsProvider::OptionsSource> &RawOptions,
+ llvm::StringRef Diagnostic) {
----------------
================
Comment at: clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp:542
+ for (const auto &[Opts, Source] : RawOptions) {
+ for (const auto &[ItOption, ItValue] : Opts.CheckOptions) {
+ if (ItOption == Option && ItValue.Value == Value) {
----------------
`CheckOptions` is a `StringMap`, so key lookup should be used instead of
iterating through all the entries
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D146842/new/
https://reviews.llvm.org/D146842
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
