maxjhandsome
/
llvm-project
forked from OSchip/llvm-project
1
0
Fork 0
Code Issues Pull Requests Packages Releases Wiki Activity

[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
This commit is contained in:
Kadir Cetinkaya 2020-07-05 21:12:11 +02:00
parent 44716856db
commit 66a2e3a525
No known key found for this signature in database
GPG Key ID: E39E36B8D2057ED6
2 changed files with 23 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
clang-tools-extra/clangd/ParsedAST.cpp
View File

@ -424,15 +424,15 @@ ParsedAST::build(llvm::StringRef Filename, const ParseInputs &Inputs,
CTFinder.matchAST(Clang->getASTContext()); 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 // UnitDiagsConsumer is local, we can not store it in CompilerInstance that
// has a longer lifetime. // has a longer lifetime.
Clang->getDiagnostics().setClient(new IgnoreDiagnostics); Clang->getDiagnostics().setClient(new IgnoreDiagnostics);
// CompilerInstance won't run this callback, do it directly. // CompilerInstance won't run this callback, do it directly.
ASTDiags.EndSourceFile(); 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; std::vector<Diag> Diags = CompilerInvocationDiags;
// Add diagnostics from the preamble, if any. // Add diagnostics from the preamble, if any.

19
clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
View File

@ -29,6 +29,8 @@ namespace clangd {
namespace { namespace {
using ::testing::_; using ::testing::_;
using ::testing::AllOf;
using ::testing::Contains;
using ::testing::ElementsAre; using ::testing::ElementsAre;
using ::testing::Field; using ::testing::Field;
using ::testing::IsEmpty; using ::testing::IsEmpty;
@ -278,6 +280,23 @@ TEST(DiagnosticsTest, ClangTidy) {
"use a trailing return type for this function"))))); "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) { TEST(DiagnosticTest, TemplatesInHeaders) {
// Diagnostics from templates defined in headers are placed at the expansion. // Diagnostics from templates defined in headers are placed at the expansion.
Annotations Main(R"cpp( Annotations Main(R"cpp(