[PATCH] D146520: [clang-tidy] Fix checks filter with warnings-as-errors

Mon Mar 27 07:44:16 PDT 2023

aaron.ballman added a comment.

In D146520#4221314 <https://reviews.llvm.org/D146520#4221314>, @carlosgalvezp wrote:

> Reading through Github I found the associated ticket (it would be good if you could mention it in the commit message):
> https://github.com/llvm/llvm-project/issues/61520
>
> So if I understand correctly, what you are trying to do is continue to let the clang-analyzer-core checks run, and simply ignore the warnings coming from them when enabling `warnings_as_errors`. I'm not entirely convinced that's the best approach, it's hiding the real problem - that as a user one cannot disable clang-analyzer-core checks (which is being fixed in another patch by PiotrZSL). I also find strange to move error-handling logic away from the DiagnosticConsumer class out to the main ClangTidy class.
>
> Would be good if @njames93  or @aaron.ballman  could comment on this, given their experience.

Ideally, users should be able to disable the core checks from running at all (the reason users want to disable checks is both because of the diagnostics they produce and because of the cost of running the check itself). However, this is solving the issue accidentally introduced which is incremental forward progress.

I'd say this is tentatively okay, but definitely needs test coverage for the change. I'd hope that the fix to no longer run the core checks at all will obviate the need for this code, though, so it might be appropriate to add a FIXME comment about that.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146520