Author: Kadir Cetinkaya Date: 2020-08-13T18:32:59+02:00 New Revision: 66a2e3a525645ad8d356ef4f5b752bfcae3c27b7
URL: https://github.com/llvm/llvm-project/commit/66a2e3a525645ad8d356ef4f5b752bfcae3c27b7 DIFF: https://github.com/llvm/llvm-project/commit/66a2e3a525645ad8d356ef4f5b752bfcae3c27b7.diff LOG: [clangd] Send EOF before resetting diagnostics consumer Summary: Some clang-tidy checkers, e.g. llvm-include-order can emit diagnostics at this callback (as mentioned in the comments). Clangd was resetting diag consumer to IgnoreDiags before sending EOF, hence we were unable to emit diagnostics for such checkers. This patch changes the order of that reset and preprocosser event to make sure we emit that diag. Fixes https://github.com/clangd/clangd/issues/314. Reviewers: sammccall Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, usaxena95, cfe-commits Tags: #clang Differential Revision: https://reviews.llvm.org/D83178 Added: Modified: clang-tools-extra/clangd/ParsedAST.cpp clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp Removed: ################################################################################ diff --git a/clang-tools-extra/clangd/ParsedAST.cpp b/clang-tools-extra/clangd/ParsedAST.cpp index 35e7fd72c3c5..23d86221e74f 100644 --- a/clang-tools-extra/clangd/ParsedAST.cpp +++ b/clang-tools-extra/clangd/ParsedAST.cpp @@ -424,15 +424,15 @@ ParsedAST::build(llvm::StringRef Filename, const ParseInputs &Inputs, CTFinder.matchAST(Clang->getASTContext()); } + // XXX: This is messy: clang-tidy checks flush some diagnostics at EOF. + // However Action->EndSourceFile() would destroy the ASTContext! + // So just inform the preprocessor of EOF, while keeping everything alive. + Clang->getPreprocessor().EndSourceFile(); // UnitDiagsConsumer is local, we can not store it in CompilerInstance that // has a longer lifetime. Clang->getDiagnostics().setClient(new IgnoreDiagnostics); // CompilerInstance won't run this callback, do it directly. ASTDiags.EndSourceFile(); - // XXX: This is messy: clang-tidy checks flush some diagnostics at EOF. - // However Action->EndSourceFile() would destroy the ASTContext! - // So just inform the preprocessor of EOF, while keeping everything alive. - Clang->getPreprocessor().EndSourceFile(); std::vector<Diag> Diags = CompilerInvocationDiags; // Add diagnostics from the preamble, if any. diff --git a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp index e8757079c675..6974131562b3 100644 --- a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp +++ b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp @@ -29,6 +29,8 @@ namespace clangd { namespace { using ::testing::_; +using ::testing::AllOf; +using ::testing::Contains; using ::testing::ElementsAre; using ::testing::Field; using ::testing::IsEmpty; @@ -278,6 +280,23 @@ TEST(DiagnosticsTest, ClangTidy) { "use a trailing return type for this function"))))); } +TEST(DiagnosticsTest, ClangTidyEOF) { + // clang-format off + Annotations Test(R"cpp( + [[#]]include <b.h> + #include "a.h")cpp"); + // clang-format on + auto TU = TestTU::withCode(Test.code()); + TU.ExtraArgs = {"-isystem."}; + TU.AdditionalFiles["a.h"] = TU.AdditionalFiles["b.h"] = ""; + TU.ClangTidyChecks = "-*, llvm-include-order"; + EXPECT_THAT( + TU.build().getDiagnostics(), + Contains(AllOf(Diag(Test.range(), "#includes are not sorted properly"), + DiagSource(Diag::ClangTidy), + DiagName("llvm-include-order")))); +} + TEST(DiagnosticTest, TemplatesInHeaders) { // Diagnostics from templates defined in headers are placed at the expansion. Annotations Main(R"cpp( _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
