[PATCH] D61487: [clang-tidy] Make the plugin honor NOLINT
Nikolai Kosjar via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Fri May 3 05:42:19 PDT 2019
nik added a comment.
Thanks for the fast comments!
In D61487#1489308 <https://reviews.llvm.org/D61487#1489308>, @alexfh wrote:
> I suspect that we're missing proper test coverage here.
True, ideally all the test scripts would also trigger the plugin case code path.
I've started with a modified copy of nolint.cpp as I wasn't able to have the two invocations (clang-tidy, c-index-test plugin) work with the same file. For example, the order of the diagnostics is different (seems solvable by appending using -DAG) and the c-index-test plugin case does not output "Suppressed 13 warnings (13 NOLINT)" - is there a way to exclude the check for this output for the c-index-test case and still having all in the same file?
I haven't tested the plugin case with nolintnextline.cpp yet, but at least this one does not seem to contain the unmute case as far as I see.
> Another issue is that compiler diagnostics don't pass ClangTidyContext::diag in the non-plugin use case.
Right, but how is this an issue?
> Do all the existing tests pass with your patch?
Yes.
> A better way to implement diagnostic filtering in the plugin would be to make ClangTidyDiagnosticConsumer able to forward diagnostics to the external diagnostics engine. It will still need some sort of a buffering though to handle diagnostics and notes attached to them together.
Ahh, you suggest that the plugin should have a separate diagnostic engine and instantiate also a ClangTidyDiagnosticConsumer object, that forwards to the external one? Will check how this could work.
Repository:
rCTE Clang Tools Extra
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D61487/new/
https://reviews.llvm.org/D61487
More information about the cfe-commits
mailing list