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

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added a comment.

LG in general.

> 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.

This looks like a bug indeed. Looking at https://reviews.llvm.org/D17335 where the test was added, I can't see any evidence of the opposite. Let's try fixing this (and the test) unless Haojian knows reasons to leave the current behavior.



================
Comment at: unittests/clang-tidy/ClangTidyTest.h:129
   tooling::Replacements Fixes;
-  for (const ClangTidyError &Error : Context.getErrors()) {
+  auto Diags = DiagConsumer.take();
+  for (const ClangTidyError &Error : Diags) {
----------------
Please specify the type explicitly, since it's not obvious from the context. http://llvm.org/docs/CodingStandards.html#use-auto-type-deduction-to-make-code-more-readable


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53953