[PATCH] D92920: [clang-tidy] Add a diagnostic callback to parseConfiguration

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D92920#2443748 <https://reviews.llvm.org/D92920#2443748>, @njames93 wrote:

> In D92920#2443578 <https://reviews.llvm.org/D92920#2443578>, @aaron.ballman wrote:
>
>> Nothing is calling the new `parseConfigurationWithDiags()`, is that intentional? Also, is there a way to add test coverage for the change?
>
> Yes it is intentional, the idea is for programs embedding clang-tidy to use this interface if they wish to handle diagnostics from parsing themselves.

Ahh, I see, thank you for the explanation.

> Tests could be added

I think they should be added.



================
Comment at: clang-tools-extra/clang-tidy/ClangTidyOptions.cpp:400
+                            DiagCallback Handler) {
+  llvm::yaml::Input Input(Config, nullptr, Handler ? diagHandlerImpl : nullptr,
+                          &Handler);
----------------
njames93 wrote:
> aaron.ballman wrote:
> > Would it make sense to require `Handler` to be nonnull with an assertion?
> Wasn't sure which way to go with that one, happy to use an assert if you think it's a good idea
Now that I understand the use for this change better, I think the code is good as-is.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92920