[PATCH] D53953: [clang-tidy] Get ClangTidyContext out of the business of storing diagnostics. NFC

Sam McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Oct 31 15:15:41 PDT 2018


sammccall created this revision.
sammccall added a reviewer: hokein.
Herald added subscribers: cfe-commits, xazax.hun.

Currently ClangTidyContext::diag() sends the diagnostics to a
DiagnosticsEngine, which probably delegates to a ClangTidyDiagnosticsConsumer,
which is supposed to go back and populate ClangTidyContext::Errors.

After this patch, the diagnostics are stored in the ClangTidyDiagnosticsConsumer
itself and can be retrieved from there.

Why?

- the round-trip from context -> engine -> consumer -> context is confusing and makes it harder to establish layering between these things.
- context does too many things, and makes it hard to use clang-tidy as a library
- everyone who actually wants the diagnostics has access to the ClangTidyDiagnosticsConsumer

The most natural implementation (ClangTidyDiagnosticsConsumer::take()
finalizes diagnostics) causes a test failure: clang-tidy-run-with-database.cpp
asserts that clang-tidy exits successfully when trying to process a file
that doesn't exist.
In clang-tidy today, this happens because finish() is never called, so the
diagnostic is never flushed. This looks like a bug to me.
For now, this patch carefully preserves that behavior, but I'll ping the
authors to see whether it's deliberate and worth preserving.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53953

Files:
  clang-tidy/ClangTidy.cpp
  clang-tidy/ClangTidy.h
  clang-tidy/ClangTidyDiagnosticConsumer.cpp
  clang-tidy/ClangTidyDiagnosticConsumer.h
  clang-tidy/tool/ClangTidyMain.cpp
  unittests/clang-tidy/ClangTidyTest.h

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D53953.172032.patch
Type: text/x-patch
Size: 9706 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20181031/5e567dc0/attachment.bin>


More information about the cfe-commits mailing list