PiotrZSL 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
----------------
njames93 wrote:
> 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.
>
No, I need both Errors and Names. Not one of them.
And I think that some usages ignore Errors even if they exist.
================
Comment at: clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp:528
+
+static llvm::StringSet<> findSourcesForDiagnostic(
+ const std::vector<ClangTidyOptionsProvider::OptionsSource> &RawOptions,
----------------
njames93 wrote:
> 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.
--verify-config checks all config files, and prints diagnostic for all of them
with a path of file.
In this way if I will have same invalid value in 2 different files, even that
one override second, it will still provide warning for both.
No need to limit only to last entry...
================
Comment at: clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp:529
+static llvm::StringSet<> findSourcesForDiagnostic(
+ const std::vector<ClangTidyOptionsProvider::OptionsSource> &RawOptions,
+ llvm::StringRef Diagnostic) {
----------------
njames93 wrote:
>
Ack.
================
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) {
----------------
njames93 wrote:
> `CheckOptions` is a `StringMap`, so key lookup should be used instead of
> iterating through all the entries
Ack.
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
