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

[clangd] Introduce pullDiags endpoint 

Implement initial support for pull-based diagnostics in ClangdServer.
This is planned for LSP 3.17, and initial proposal is in
d15eb0671e/protocol/src/common/proposed.diagnostic.ts (L111).

We chose to serve the requests only when clangd has a fresh preamble
available. In case of a stale preamble we just drop the request on the
floor.

This patch doesn't plumb this to LSP layer yet, as pullDiags is still a
proposal with only an implementation in vscode.

Differential Revision: https://reviews.llvm.org/D98623
This commit is contained in:
Kadir Cetinkaya 2021-03-15 10:18:12 +01:00
parent 35368bbdbb
commit 2772c3a975
No known key found for this signature in database
GPG Key ID: E39E36B8D2057ED6
15 changed files with 141 additions and 82 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

20
clang-tools-extra/clangd/ClangdServer.cpp
View File

@ -9,6 +9,7 @@
#include "ClangdServer.h"
#include "CodeComplete.h"
#include "Config.h"
#include "Diagnostics.h"
#include "DumpAST.h"
#include "FindSymbols.h"
#include "Format.h"
@ -81,7 +82,9 @@ struct UpdateIndexCallbacks : public ParsingCallbacks {
if (FIndex)
FIndex->updateMain(Path, AST);
std::vector<Diag> Diagnostics = AST.getDiagnostics();
assert(AST.getDiagnostics().hasValue() &&
"We issue callback only with fresh preambles");
std::vector<Diag> Diagnostics = *AST.getDiagnostics();
if (ServerCallbacks)
Publish([&]() {
ServerCallbacks->onDiagnosticsReady(Path, AST.version(),
@ -902,6 +905,21 @@ void ClangdServer::customAction(PathRef File, llvm::StringRef Name,
WorkScheduler->runWithAST(Name, File, std::move(Action));
}
void ClangdServer::diagnostics(PathRef File, Callback<std::vector<Diag>> CB) {
auto Action =
[CB = std::move(CB)](llvm::Expected<InputsAndAST> InpAST) mutable {
if (!InpAST)
return CB(InpAST.takeError());
if (auto Diags = InpAST->AST.getDiagnostics())
return CB(*Diags);
// FIXME: Use ServerCancelled error once it is settled in LSP-3.17.
return CB(llvm::make_error<LSPError>("server is busy parsing includes",
ErrorCode::InternalError));
};
WorkScheduler->runWithAST("Diagnostics", File, std::move(Action));
}
llvm::StringMap<TUScheduler::FileStats> ClangdServer::fileStats() const {
return WorkScheduler->fileStats();
}

12
clang-tools-extra/clangd/ClangdServer.h
View File

@ -12,6 +12,7 @@
#include "../clang-tidy/ClangTidyOptions.h"
#include "CodeComplete.h"
#include "ConfigProvider.h"
#include "Diagnostics.h"
#include "DraftStore.h"
#include "FeatureModule.h"
#include "GlobalCompilationDatabase.h"
@ -40,6 +41,7 @@
#include <string>
#include <type_traits>
#include <utility>
#include <vector>
namespace clang {
namespace clangd {
@ -64,6 +66,8 @@ public:
virtual ~Callbacks() = default;
/// Called by ClangdServer when \p Diagnostics for \p File are ready.
/// These pushed diagnostics might correspond to an older version of the
/// file, they do not interfere with "pull-based" ClangdServer::diagnostics.
/// May be called concurrently for separate files, not for a single file.
virtual void onDiagnosticsReady(PathRef File, llvm::StringRef Version,
std::vector<Diag> Diagnostics) {}
@ -345,6 +349,14 @@ public:
void customAction(PathRef File, llvm::StringRef Name,
Callback<InputsAndAST> Action);
/// Fetches diagnostics for current version of the \p File. This might fail if
/// server is busy (building a preamble) and would require a long time to
/// prepare diagnostics. If it fails, clients should wait for
/// onSemanticsMaybeChanged and then retry.
/// These 'pulled' diagnostics do not interfere with the diagnostics 'pushed'
/// to Callbacks::onDiagnosticsReady, and clients may use either or both.
void diagnostics(PathRef File, Callback<std::vector<Diag>> CB);
/// Returns estimated memory usage and other statistics for each of the
/// currently open files.
/// Overall memory usage of clangd may be significantly more than reported

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

@ -264,9 +264,11 @@ ParsedAST::build(llvm::StringRef Filename, const ParseInputs &Inputs,
StoreDiags ASTDiags;
llvm::Optional<PreamblePatch> Patch;
bool PreserveDiags = true;
if (Preamble) {
Patch = PreamblePatch::create(Filename, Inputs, *Preamble);
Patch->apply(*CI);
PreserveDiags = Patch->preserveDiagnostics();
}
auto Clang = prepareCompilerInstance(
std::move(CI), PreamblePCH,
@ -441,14 +443,20 @@ ParsedAST::build(llvm::StringRef Filename, const ParseInputs &Inputs,
// CompilerInstance won't run this callback, do it directly.
ASTDiags.EndSourceFile();
std::vector<Diag> Diags = CompilerInvocationDiags;
// Add diagnostics from the preamble, if any.
if (Preamble)
Diags.insert(Diags.end(), Preamble->Diags.begin(), Preamble->Diags.end());
// Finally, add diagnostics coming from the AST.
{
std::vector<Diag> D = ASTDiags.take(CTContext.getPointer());
Diags.insert(Diags.end(), D.begin(), D.end());
llvm::Optional<std::vector<Diag>> Diags;
// FIXME: Also skip generation of diagnostics alltogether to speed up ast
// builds when we are patching a stale preamble.
if (PreserveDiags) {
Diags = CompilerInvocationDiags;
// Add diagnostics from the preamble, if any.
if (Preamble)
Diags->insert(Diags->end(), Preamble->Diags.begin(),
Preamble->Diags.end());
// Finally, add diagnostics coming from the AST.
{
std::vector<Diag> D = ASTDiags.take(CTContext.getPointer());
Diags->insert(Diags->end(), D.begin(), D.end());
}
}
return ParsedAST(Inputs.Version, std::move(Preamble), std::move(Clang),
std::move(Action), std::move(Tokens), std::move(Macros),
@ -493,14 +501,12 @@ llvm::ArrayRef<Decl *> ParsedAST::getLocalTopLevelDecls() {
const MainFileMacros &ParsedAST::getMacros() const { return Macros; }
const std::vector<Diag> &ParsedAST::getDiagnostics() const { return Diags; }
std::size_t ParsedAST::getUsedBytes() const {
auto &AST = getASTContext();
// FIXME(ibiryukov): we do not account for the dynamically allocated part of
// Message and Fixes inside each diagnostic.
std::size_t Total =
clangd::getUsedBytes(LocalTopLevelDecls) + clangd::getUsedBytes(Diags);
std::size_t Total = clangd::getUsedBytes(LocalTopLevelDecls) +
(Diags ? clangd::getUsedBytes(*Diags) : 0);
// FIXME: the rest of the function is almost a direct copy-paste from
// libclang's clang_getCXTUResourceUsage. We could share the implementation.
@ -541,8 +547,8 @@ ParsedAST::ParsedAST(llvm::StringRef Version,
std::unique_ptr<FrontendAction> Action,
syntax::TokenBuffer Tokens, MainFileMacros Macros,
std::vector<Decl *> LocalTopLevelDecls,
std::vector<Diag> Diags, IncludeStructure Includes,
CanonicalIncludes CanonIncludes)
llvm::Optional<std::vector<Diag>> Diags,
IncludeStructure Includes, CanonicalIncludes CanonIncludes)
: Version(Version), Preamble(std::move(Preamble)), Clang(std::move(Clang)),
Action(std::move(Action)), Tokens(std::move(Tokens)),
Macros(std::move(Macros)), Diags(std::move(Diags)),

10
clang-tools-extra/clangd/ParsedAST.h
View File

@ -88,7 +88,9 @@ public:
/// (These should be const, but RecursiveASTVisitor requires Decl*).
ArrayRef<Decl *> getLocalTopLevelDecls();
const std::vector<Diag> &getDiagnostics() const;
const llvm::Optional<std::vector<Diag>> &getDiagnostics() const {
return Diags;
}
/// Returns the estimated size of the AST and the accessory structures, in
/// bytes. Does not include the size of the preamble.
@ -120,7 +122,7 @@ private:
std::unique_ptr<CompilerInstance> Clang,
std::unique_ptr<FrontendAction> Action, syntax::TokenBuffer Tokens,
MainFileMacros Macros, std::vector<Decl *> LocalTopLevelDecls,
std::vector<Diag> Diags, IncludeStructure Includes,
llvm::Optional<std::vector<Diag>> Diags, IncludeStructure Includes,
CanonicalIncludes CanonIncludes);
std::string Version;
@ -142,8 +144,8 @@ private:
/// All macro definitions and expansions in the main file.
MainFileMacros Macros;
// Data, stored after parsing.
std::vector<Diag> Diags;
// Data, stored after parsing. None if AST was built with a stale preamble.
llvm::Optional<std::vector<Diag>> Diags;
// Top-level decls inside the current file. Not that this does not include
// top-level decls from the preamble.
std::vector<Decl *> LocalTopLevelDecls;

3
clang-tools-extra/clangd/Preamble.h
View File

@ -126,6 +126,9 @@ public:
/// Returns textual patch contents.
llvm::StringRef text() const { return PatchContents; }
/// Whether diagnostics generated using this patch are trustable.
bool preserveDiagnostics() const { return PatchContents.empty(); }
private:
PreamblePatch() = default;
std::string PatchContents;

2
clang-tools-extra/clangd/tool/Check.cpp
View File

@ -181,7 +181,7 @@ public:
elog("Failed to build AST");
return false;
}
ErrCount += showErrors(llvm::makeArrayRef(AST->getDiagnostics())
ErrCount += showErrors(llvm::makeArrayRef(*AST->getDiagnostics())
.drop_front(Preamble->Diags.size()));
if (Opts.BuildDynamicSymbolIndex) {

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

@ -135,7 +135,7 @@ o]]();
auto TU = TestTU::withCode(Test.code());
TU.ClangTidyProvider = addTidyChecks("google-explicit-constructor");
EXPECT_THAT(
TU.build().getDiagnostics(),
*TU.build().getDiagnostics(),
ElementsAre(
// This range spans lines.
AllOf(Diag(Test.range("typo"),
@ -173,14 +173,14 @@ o]]();
TEST(DiagnosticsTest, FlagsMatter) {
Annotations Test("[[void]] main() {} // error-ok");
auto TU = TestTU::withCode(Test.code());
EXPECT_THAT(TU.build().getDiagnostics(),
EXPECT_THAT(*TU.build().getDiagnostics(),
ElementsAre(AllOf(Diag(Test.range(), "'main' must return 'int'"),
WithFix(Fix(Test.range(), "int",
"change 'void' to 'int'")))));
// Same code built as C gets different diagnostics.
TU.Filename = "Plain.c";
EXPECT_THAT(
TU.build().getDiagnostics(),
*TU.build().getDiagnostics(),
ElementsAre(AllOf(
Diag(Test.range(), "return type of 'main' is not 'int'"),
WithFix(Fix(Test.range(), "int", "change return type to 'int'")))));
@ -192,7 +192,7 @@ TEST(DiagnosticsTest, DiagnosticPreamble) {
)cpp");
auto TU = TestTU::withCode(Test.code());
EXPECT_THAT(TU.build().getDiagnostics(),
EXPECT_THAT(*TU.build().getDiagnostics(),
ElementsAre(::testing::AllOf(
Diag(Test.range(), "'not-found.h' file not found"),
DiagSource(Diag::Clang), DiagName("pp_file_not_found"))));
@ -209,7 +209,7 @@ TEST(DiagnosticsTest, DeduplicatedClangTidyDiagnostics) {
"hicpp-uppercase-literal-suffix");
// Verify that we filter out the duplicated diagnostic message.
EXPECT_THAT(
TU.build().getDiagnostics(),
*TU.build().getDiagnostics(),
UnorderedElementsAre(::testing::AllOf(
Diag(Test.range(),
"floating point literal has suffix 'f', which is not uppercase"),
@ -229,7 +229,7 @@ TEST(DiagnosticsTest, DeduplicatedClangTidyDiagnostics) {
// The check doesn't handle template instantiations which ends up emitting
// duplicated messages, verify that we deduplicate them.
EXPECT_THAT(
TU.build().getDiagnostics(),
*TU.build().getDiagnostics(),
UnorderedElementsAre(::testing::AllOf(
Diag(Test.range(),
"floating point literal has suffix 'f', which is not uppercase"),
@ -254,7 +254,7 @@ TEST(DiagnosticsTest, ClangTidy) {
"modernize-deprecated-headers,"
"modernize-use-trailing-return-type");
EXPECT_THAT(
TU.build().getDiagnostics(),
*TU.build().getDiagnostics(),
UnorderedElementsAre(
AllOf(Diag(Test.range("deprecated"),
"inclusion of deprecated C++ header 'assert.h'; consider "
@ -296,7 +296,7 @@ TEST(DiagnosticsTest, ClangTidyEOF) {
TU.AdditionalFiles["a.h"] = TU.AdditionalFiles["b.h"] = "";
TU.ClangTidyProvider = addTidyChecks("llvm-include-order");
EXPECT_THAT(
TU.build().getDiagnostics(),
*TU.build().getDiagnostics(),
Contains(AllOf(Diag(Test.range(), "#includes are not sorted properly"),
DiagSource(Diag::ClangTidy),
DiagName("llvm-include-order"))));
@ -314,7 +314,7 @@ TEST(DiagnosticTest, TemplatesInHeaders) {
TestTU TU = TestTU::withCode(Main.code());
TU.HeaderCode = Header.code().str();
EXPECT_THAT(
TU.build().getDiagnostics(),
*TU.build().getDiagnostics(),
ElementsAre(AllOf(
Diag(Main.range(), "in template: base specifier must name a class"),
WithNote(Diag(Header.range(), "error occurred here"),
@ -340,7 +340,7 @@ TEST(DiagnosticTest, MakeUnique) {
}
}
)cpp";
EXPECT_THAT(TU.build().getDiagnostics(),
EXPECT_THAT(*TU.build().getDiagnostics(),
UnorderedElementsAre(
Diag(Main.range(),
"in template: "
@ -368,7 +368,7 @@ TEST(DiagnosticTest, NoMultipleDiagnosticInFlight) {
TestTU TU = TestTU::withCode(Main.code());
TU.ClangTidyProvider = addTidyChecks("modernize-loop-convert");
EXPECT_THAT(
TU.build().getDiagnostics(),
*TU.build().getDiagnostics(),
UnorderedElementsAre(::testing::AllOf(
Diag(Main.range(), "use range-based for loop instead"),
DiagSource(Diag::ClangTidy), DiagName("modernize-loop-convert"))));
@ -384,14 +384,14 @@ TEST(DiagnosticTest, RespectsDiagnosticConfig) {
)cpp");
auto TU = TestTU::withCode(Main.code());
EXPECT_THAT(
TU.build().getDiagnostics(),
*TU.build().getDiagnostics(),
ElementsAre(Diag(Main.range(), "use of undeclared identifier 'unknown'"),
Diag(Main.range("ret"),
"void function 'x' should not return a value")));
Config Cfg;
Cfg.Diagnostics.Suppress.insert("return-type");
WithContextValue WithCfg(Config::Key, std::move(Cfg));
EXPECT_THAT(TU.build().getDiagnostics(),
EXPECT_THAT(*TU.build().getDiagnostics(),
ElementsAre(Diag(Main.range(),
"use of undeclared identifier 'unknown'")));
}
@ -413,7 +413,7 @@ TEST(DiagnosticTest, ClangTidySuppressionComment) {
TestTU TU = TestTU::withCode(Main.code());
TU.ClangTidyProvider = addTidyChecks("bugprone-integer-division");
EXPECT_THAT(
TU.build().getDiagnostics(),
*TU.build().getDiagnostics(),
UnorderedElementsAre(::testing::AllOf(
Diag(Main.range(), "result of integer division used in a floating "
"point context; possible loss of precision"),
@ -431,7 +431,7 @@ TEST(DiagnosticTest, ClangTidyWarningAsError) {
TU.ClangTidyProvider =
addTidyChecks("bugprone-integer-division", "bugprone-integer-division");
EXPECT_THAT(
TU.build().getDiagnostics(),
*TU.build().getDiagnostics(),
UnorderedElementsAre(::testing::AllOf(
Diag(Main.range(), "result of integer division used in a floating "
"point context; possible loss of precision"),
@ -450,7 +450,7 @@ TEST(DiagnosticTest, LongFixMessages) {
)cpp");
TestTU TU = TestTU::withCode(Source.code());
EXPECT_THAT(
TU.build().getDiagnostics(),
*TU.build().getDiagnostics(),
ElementsAre(WithFix(Fix(
Source.range(),
"somereallyreallyreallyreallyreallyreallyreallyreallylongidentifier",
@ -466,7 +466,7 @@ n]] = 10; // error-ok
}
)cpp");
TU.Code = std::string(Source.code());
EXPECT_THAT(TU.build().getDiagnostics(),
EXPECT_THAT(*TU.build().getDiagnostics(),
ElementsAre(WithFix(
Fix(Source.range(), "ident", "change 'ide\\…' to 'ident'"))));
}
@ -481,7 +481,7 @@ TEST(DiagnosticTest, ClangTidySuppressionCommentTrumpsWarningAsError) {
TestTU TU = TestTU::withCode(Main.code());
TU.ClangTidyProvider =
addTidyChecks("bugprone-integer-division", "bugprone-integer-division");
EXPECT_THAT(TU.build().getDiagnostics(), UnorderedElementsAre());
EXPECT_THAT(*TU.build().getDiagnostics(), UnorderedElementsAre());
}
TEST(DiagnosticTest, ClangTidyNoLiteralDataInMacroToken) {
@ -496,7 +496,7 @@ TEST(DiagnosticTest, ClangTidyNoLiteralDataInMacroToken) {
)cpp");
TestTU TU = TestTU::withCode(Main.code());
TU.ClangTidyProvider = addTidyChecks("bugprone-bad-signal-to-kill-thread");
EXPECT_THAT(TU.build().getDiagnostics(), UnorderedElementsAre()); // no-crash
EXPECT_THAT(*TU.build().getDiagnostics(), UnorderedElementsAre()); // no-crash
}
TEST(DiagnosticTest, ElseAfterReturnRange) {
@ -513,7 +513,7 @@ TEST(DiagnosticTest, ElseAfterReturnRange) {
TestTU TU = TestTU::withCode(Main.code());
TU.ClangTidyProvider = addTidyChecks("llvm-else-after-return");
EXPECT_THAT(
TU.build().getDiagnostics(),
*TU.build().getDiagnostics(),
ElementsAre(Diag(Main.range(), "do not use 'else' after 'return'")));
}
@ -532,7 +532,7 @@ TEST(DiagnosticsTest, Preprocessor) {
#endif
)cpp");
EXPECT_THAT(
TestTU::withCode(Test.code()).build().getDiagnostics(),
*TestTU::withCode(Test.code()).build().getDiagnostics(),
ElementsAre(Diag(Test.range(), "use of undeclared identifier 'b'")));
}
@ -542,7 +542,7 @@ TEST(DiagnosticsTest, IgnoreVerify) {
)cpp");
TU.ExtraArgs.push_back("-Xclang");
TU.ExtraArgs.push_back("-verify");
EXPECT_THAT(TU.build().getDiagnostics(), IsEmpty());
EXPECT_THAT(*TU.build().getDiagnostics(), IsEmpty());
}
// Recursive main-file include is diagnosed, and doesn't crash.
@ -552,7 +552,7 @@ TEST(DiagnosticsTest, RecursivePreamble) {
int symbol;
)cpp");
TU.Filename = "foo.h";
EXPECT_THAT(TU.build().getDiagnostics(),
EXPECT_THAT(*TU.build().getDiagnostics(),
ElementsAre(DiagName("pp_including_mainfile_in_preamble")));
EXPECT_THAT(TU.build().getLocalTopLevelDecls(), SizeIs(1));
}
@ -565,7 +565,7 @@ TEST(DiagnosticsTest, RecursivePreamblePragmaOnce) {
int symbol;
)cpp");
TU.Filename = "foo.h";
EXPECT_THAT(TU.build().getDiagnostics(), IsEmpty());
EXPECT_THAT(*TU.build().getDiagnostics(), IsEmpty());
EXPECT_THAT(TU.build().getLocalTopLevelDecls(), SizeIs(1));
}
@ -581,7 +581,7 @@ TEST(DiagnosticsTest, RecursivePreambleIfndefGuard) {
)cpp");
TU.Filename = "foo.h";
// FIXME: should be no errors here.
EXPECT_THAT(TU.build().getDiagnostics(),
EXPECT_THAT(*TU.build().getDiagnostics(),
ElementsAre(DiagName("pp_including_mainfile_in_preamble")));
EXPECT_THAT(TU.build().getLocalTopLevelDecls(), SizeIs(1));
}
@ -598,7 +598,7 @@ TEST(DiagnosticsTest, InsideMacros) {
return $bar[[TEN]];
}
)cpp");
EXPECT_THAT(TestTU::withCode(Test.code()).build().getDiagnostics(),
EXPECT_THAT(*TestTU::withCode(Test.code()).build().getDiagnostics(),
ElementsAre(Diag(Test.range("foo"),
"cannot initialize return object of type "
"'int *' with an rvalue of type 'int'"),
@ -614,7 +614,7 @@ TEST(DiagnosticsTest, NoFixItInMacro) {
[[Define]](main) // error-ok
)cpp");
auto TU = TestTU::withCode(Test.code());
EXPECT_THAT(TU.build().getDiagnostics(),
EXPECT_THAT(*TU.build().getDiagnostics(),
ElementsAre(AllOf(Diag(Test.range(), "'main' must return 'int'"),
Not(WithFix(_)))));
}
@ -625,7 +625,7 @@ TEST(ClangdTest, MSAsm) {
llvm::InitializeAllTargetInfos(); // As in ClangdMain
auto TU = TestTU::withCode("void fn() { __asm { cmp cl,64 } }");
TU.ExtraArgs = {"-fms-extensions"};
EXPECT_THAT(TU.build().getDiagnostics(), IsEmpty());
EXPECT_THAT(*TU.build().getDiagnostics(), IsEmpty());
}
TEST(DiagnosticsTest, ToLSP) {
@ -783,7 +783,7 @@ class T {
TU.ExternalIndex = Index.get();
EXPECT_THAT(
TU.build().getDiagnostics(),
*TU.build().getDiagnostics(),
UnorderedElementsAreArray(
{AllOf(Diag(Test.range("nested"),
"incomplete type 'ns::X' named in nested name specifier"),
@ -868,7 +868,7 @@ int main() {
MemIndex::build(std::move(Slab).build(), RefSlab(), RelationSlab());
TU.ExternalIndex = Index.get();
EXPECT_THAT(TU.build().getDiagnostics(),
EXPECT_THAT(*TU.build().getDiagnostics(),
UnorderedElementsAre(
Diag(Test.range("base"), "base class has incomplete type"),
Diag(Test.range("access"),
@ -901,7 +901,7 @@ using Type = ns::$template[[Foo]]<int>;
TU.ExternalIndex = Index.get();
EXPECT_THAT(
TU.build().getDiagnostics(),
*TU.build().getDiagnostics(),
UnorderedElementsAre(
AllOf(Diag(Test.range("unqualified1"), "unknown type name 'X'"),
DiagName("unknown_typename"),
@ -946,7 +946,7 @@ void foo() {
SymbolWithHeader{"na::nb::X", "unittest:///b.h", "\"b.h\""}});
TU.ExternalIndex = Index.get();
EXPECT_THAT(TU.build().getDiagnostics(),
EXPECT_THAT(*TU.build().getDiagnostics(),
UnorderedElementsAre(AllOf(
Diag(Test.range("unqualified"), "unknown type name 'X'"),
DiagName("unknown_typename"),
@ -967,7 +967,7 @@ TEST(IncludeFixerTest, NoCrashMemebrAccess) {
TU.ExternalIndex = Index.get();
EXPECT_THAT(
TU.build().getDiagnostics(),
*TU.build().getDiagnostics(),
UnorderedElementsAre(Diag(Test.range(), "no member named 'xy' in 'X'")));
}
@ -1002,7 +1002,7 @@ void bar(X *x) {
TU.ExternalIndex = Index.get();
auto Parsed = TU.build();
for (const auto &D : Parsed.getDiagnostics()) {
for (const auto &D : *Parsed.getDiagnostics()) {
if (D.Fixes.size() != 1) {
ADD_FAILURE() << "D.Fixes.size() != 1";
continue;
@ -1027,7 +1027,7 @@ void g() { ns::$[[scope]]::X_Y(); }
TU.ExternalIndex = Index.get();
EXPECT_THAT(
TU.build().getDiagnostics(),
*TU.build().getDiagnostics(),
UnorderedElementsAre(AllOf(
Diag(Test.range(), "no member named 'scope' in namespace 'ns'"),
DiagName("no_member"),
@ -1055,7 +1055,7 @@ void f() {
TU.ExternalIndex = Index.get();
EXPECT_THAT(
TU.build().getDiagnostics(),
*TU.build().getDiagnostics(),
UnorderedElementsAre(
AllOf(
Diag(Test.range("q1"), "use of undeclared identifier 'clangd'; "
@ -1098,7 +1098,7 @@ namespace c {
SymbolWithHeader{"a::X", "unittest:///x.h", "\"x.h\""});
TU.ExternalIndex = Index.get();
EXPECT_THAT(TU.build().getDiagnostics(),
EXPECT_THAT(*TU.build().getDiagnostics(),
UnorderedElementsAre(AllOf(
Diag(Test.range(), "no type named 'X' in namespace 'a'"),
DiagName("typename_nested_not_found"),
@ -1124,7 +1124,7 @@ TEST(IncludeFixerTest, NoCrashOnTemplateInstantiations) {
TU.ExternalIndex = Index.get();
EXPECT_THAT(
TU.build().getDiagnostics(),
*TU.build().getDiagnostics(),
ElementsAre(Diag(Test.range(), "use of undeclared identifier 'a'")));
}
@ -1135,7 +1135,7 @@ TEST(DiagsInHeaders, DiagInsideHeader) {
Annotations Header("[[no_type_spec]]; // error-ok");
TestTU TU = TestTU::withCode(Main.code());
TU.AdditionalFiles = {{"a.h", std::string(Header.code())}};
EXPECT_THAT(TU.build().getDiagnostics(),
EXPECT_THAT(*TU.build().getDiagnostics(),
UnorderedElementsAre(AllOf(
Diag(Main.range(), "in included file: C++ requires a "
"type specifier for all declarations"),
@ -1149,7 +1149,7 @@ TEST(DiagsInHeaders, DiagInTransitiveInclude) {
TestTU TU = TestTU::withCode(Main.code());
TU.AdditionalFiles = {{"a.h", "#include \"b.h\""},
{"b.h", "no_type_spec; // error-ok"}};
EXPECT_THAT(TU.build().getDiagnostics(),
EXPECT_THAT(*TU.build().getDiagnostics(),
UnorderedElementsAre(
Diag(Main.range(), "in included file: C++ requires a "
"type specifier for all declarations")));
@ -1163,7 +1163,7 @@ TEST(DiagsInHeaders, DiagInMultipleHeaders) {
TestTU TU = TestTU::withCode(Main.code());
TU.AdditionalFiles = {{"a.h", "no_type_spec; // error-ok"},
{"b.h", "no_type_spec; // error-ok"}};
EXPECT_THAT(TU.build().getDiagnostics(),
EXPECT_THAT(*TU.build().getDiagnostics(),
UnorderedElementsAre(
Diag(Main.range("a"), "in included file: C++ requires a type "
"specifier for all declarations"),
@ -1180,7 +1180,7 @@ TEST(DiagsInHeaders, PreferExpansionLocation) {
TU.AdditionalFiles = {
{"a.h", "#include \"b.h\"\n"},
{"b.h", "#ifndef X\n#define X\nno_type_spec; // error-ok\n#endif"}};
EXPECT_THAT(TU.build().getDiagnostics(),
EXPECT_THAT(*TU.build().getDiagnostics(),
UnorderedElementsAre(Diag(Main.range(),
"in included file: C++ requires a type "
"specifier for all declarations")));
@ -1198,7 +1198,7 @@ TEST(DiagsInHeaders, PreferExpansionLocationMacros) {
{"a.h", "#include \"c.h\"\n"},
{"b.h", "#include \"c.h\"\n"},
{"c.h", "#ifndef X\n#define X\nno_type_spec; // error-ok\n#endif"}};
EXPECT_THAT(TU.build().getDiagnostics(),
EXPECT_THAT(*TU.build().getDiagnostics(),
UnorderedElementsAre(
Diag(Main.range(), "in included file: C++ requires a "
"type specifier for all declarations")));
@ -1227,7 +1227,7 @@ TEST(DiagsInHeaders, LimitDiagsOutsideMainFile) {
no_type_spec_9;
no_type_spec_10;
#endif)cpp"}};
EXPECT_THAT(TU.build().getDiagnostics(),
EXPECT_THAT(*TU.build().getDiagnostics(),
UnorderedElementsAre(
Diag(Main.range(), "in included file: C++ requires a "
"type specifier for all declarations")));
@ -1242,7 +1242,7 @@ TEST(DiagsInHeaders, OnlyErrorOrFatal) {
int x = 5/0;)cpp");
TestTU TU = TestTU::withCode(Main.code());
TU.AdditionalFiles = {{"a.h", std::string(Header.code())}};
EXPECT_THAT(TU.build().getDiagnostics(),
EXPECT_THAT(*TU.build().getDiagnostics(),
UnorderedElementsAre(AllOf(
Diag(Main.range(), "in included file: C++ requires "
"a type specifier for all declarations"),
@ -1260,7 +1260,7 @@ TEST(DiagsInHeaders, OnlyDefaultErrorOrFatal) {
TU.AdditionalFiles = {{"a.h", std::string(Header.code())}};
// promote warnings to errors.
TU.ExtraArgs = {"-Werror", "-Wunused"};
EXPECT_THAT(TU.build().getDiagnostics(), IsEmpty());
EXPECT_THAT(*TU.build().getDiagnostics(), IsEmpty());
}
TEST(DiagsInHeaders, FromNonWrittenSources) {
@ -1273,7 +1273,7 @@ TEST(DiagsInHeaders, FromNonWrittenSources) {
TestTU TU = TestTU::withCode(Main.code());
TU.AdditionalFiles = {{"a.h", std::string(Header.code())}};
TU.ExtraArgs = {"-DFOO=NOOO"};
EXPECT_THAT(TU.build().getDiagnostics(),
EXPECT_THAT(*TU.build().getDiagnostics(),
UnorderedElementsAre(AllOf(
Diag(Main.range(),
"in included file: use of undeclared identifier 'NOOO'"),
@ -1291,7 +1291,7 @@ TEST(DiagsInHeaders, ErrorFromMacroExpansion) {
X;)cpp");
TestTU TU = TestTU::withCode(Main.code());
TU.AdditionalFiles = {{"a.h", std::string(Header.code())}};
EXPECT_THAT(TU.build().getDiagnostics(),
EXPECT_THAT(*TU.build().getDiagnostics(),
UnorderedElementsAre(
Diag(Main.range(), "in included file: use of undeclared "
"identifier 'foo'; did you mean 'fo'?")));
@ -1308,7 +1308,7 @@ TEST(DiagsInHeaders, ErrorFromMacroArgument) {
X(foo);)cpp");
TestTU TU = TestTU::withCode(Main.code());
TU.AdditionalFiles = {{"a.h", std::string(Header.code())}};
EXPECT_THAT(TU.build().getDiagnostics(),
EXPECT_THAT(*TU.build().getDiagnostics(),
UnorderedElementsAre(
Diag(Main.range(), "in included file: use of undeclared "
"identifier 'foo'; did you mean 'fo'?")));
@ -1320,7 +1320,7 @@ TEST(IgnoreDiags, FromNonWrittenInclude) {
TU.AdditionalFiles = {{"a.h", "void main();"}};
// The diagnostic "main must return int" is from the header, we don't attempt
// to render it in the main file as there is no written location there.
EXPECT_THAT(TU.build().getDiagnostics(), UnorderedElementsAre());
EXPECT_THAT(*TU.build().getDiagnostics(), UnorderedElementsAre());
}
TEST(ToLSPDiag, RangeIsInMain) {

2
clang-tools-extra/clangd/unittests/ModulesTests.cpp
View File

@ -61,7 +61,7 @@ TEST(Modules, PreambleBuildVisibility) {
header "module.h"
}
)modulemap";
EXPECT_TRUE(TU.build().getDiagnostics().empty());
EXPECT_TRUE(TU.build().getDiagnostics()->empty());
}
TEST(Modules, Diagnostic) {

6
clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
View File

@ -511,7 +511,7 @@ TEST(ParsedASTTest, ReplayPreambleForTidyCheckers) {
auto PatchedAST = ParsedAST::build(testPath(TU.Filename), TU.inputs(FS),
std::move(CI), {}, BaselinePreamble);
ASSERT_TRUE(PatchedAST);
EXPECT_TRUE(PatchedAST->getDiagnostics().empty());
EXPECT_FALSE(PatchedAST->getDiagnostics());
}
// Then ensure correctness by making sure includes were seen only once.
@ -526,7 +526,7 @@ TEST(ParsedASTTest, ReplayPreambleForTidyCheckers) {
auto PatchedAST = ParsedAST::build(testPath(TU.Filename), TU.inputs(FS),
std::move(CI), {}, BaselinePreamble);
ASSERT_TRUE(PatchedAST);
EXPECT_TRUE(PatchedAST->getDiagnostics().empty());
EXPECT_FALSE(PatchedAST->getDiagnostics());
EXPECT_THAT(Includes,
ElementsAre(WithFileName(testPath("__preamble_patch__.h")),
WithFileName("b.h"), WithFileName("a.h")));
@ -569,7 +569,7 @@ TEST(ParsedASTTest, PatchesAdditionalIncludes) {
auto PatchedAST = ParsedAST::build(testPath("foo.cpp"), Inputs, std::move(CI),
{}, EmptyPreamble);
ASSERT_TRUE(PatchedAST);
ASSERT_TRUE(PatchedAST->getDiagnostics().empty());
ASSERT_FALSE(PatchedAST->getDiagnostics());
// Ensure source location information is correct, including resolved paths.
EXPECT_THAT(PatchedAST->getIncludeStructure().MainFileIncludes,

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

@ -274,8 +274,12 @@ TEST(PreamblePatchTest, Define) {
auto AST = createPatchedAST("", Modified.code());
ASSERT_TRUE(AST);
EXPECT_THAT(AST->getDiagnostics(),
Not(Contains(Field(&Diag::Range, Modified.range()))));
std::vector<Range> MacroRefRanges;
for (auto &M : AST->getMacros().MacroRefs) {
for (auto &O : M.getSecond())
MacroRefRanges.push_back(O.Rng);
}
EXPECT_THAT(MacroRefRanges, Contains(Modified.range()));
}
}
@ -298,8 +302,6 @@ TEST(PreamblePatchTest, OrderingPreserved) {
auto AST = createPatchedAST(Baseline, Modified.code());
ASSERT_TRUE(AST);
EXPECT_THAT(AST->getDiagnostics(),
Not(Contains(Field(&Diag::Range, Modified.range()))));
}
TEST(PreamblePatchTest, LocateMacroAtWorks) {
@ -535,6 +537,15 @@ TEST(PreamblePatch, ModifiedBounds) {
ExpectedBounds.PreambleEndsAtStartOfLine);
}
}
TEST(PreamblePatch, DropsDiagnostics) {
llvm::StringLiteral Code = "#define FOO\nx;/* error-ok */";
// First check that this code generates diagnostics.
EXPECT_THAT(*TestTU::withCode(Code).build().getDiagnostics(),
testing::Not(testing::IsEmpty()));
// Ensure they are dropeed when a patched preamble is used.
EXPECT_FALSE(createPatchedAST("", Code)->getDiagnostics());
}
} // namespace
} // namespace clangd
} // namespace clang

2
clang-tools-extra/clangd/unittests/SelectionTests.cpp
View File

@ -581,7 +581,7 @@ TEST(SelectionTest, PathologicalPreprocessor) {
auto TU = TestTU::withCode(Test.code());
TU.AdditionalFiles["Expand.inc"] = "MACRO\n";
auto AST = TU.build();
EXPECT_THAT(AST.getDiagnostics(), ::testing::IsEmpty());
EXPECT_THAT(*AST.getDiagnostics(), ::testing::IsEmpty());
auto T = makeSelectionTree(Case, AST);
EXPECT_EQ("BreakStmt", T.commonAncestor()->kind());

2
clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
View File

@ -121,7 +121,7 @@ protected:
class CaptureDiags : public ParsingCallbacks {
public:
void onMainAST(PathRef File, ParsedAST &AST, PublishFn Publish) override {
reportDiagnostics(File, AST.getDiagnostics(), Publish);
reportDiagnostics(File, *AST.getDiagnostics(), Publish);
}
void onFailedAST(PathRef File, llvm::StringRef Version,

8
clang-tools-extra/clangd/unittests/TestTU.cpp
View File

@ -113,6 +113,11 @@ ParsedAST TestTU::build() const {
ADD_FAILURE() << "Failed to build code:\n" << Code;
llvm_unreachable("Failed to build TestTU!");
}
if (!AST->getDiagnostics()) {
ADD_FAILURE() << "TestTU should always build an AST with a fresh Preamble"
<< Code;
return std::move(*AST);
}
// Check for error diagnostics and report gtest failures (unless expected).
// This guards against accidental syntax errors silently subverting tests.
// error-ok is awfully primitive - using clang -verify would be nicer.
@ -128,7 +133,8 @@ ParsedAST TestTU::build() const {
return false;
}();
if (!ErrorOk) {
for (const auto &D : AST->getDiagnostics())
// We always build AST with a fresh preamble in TestTU.
for (const auto &D : *AST->getDiagnostics())
if (D.Severity >= DiagnosticsEngine::Error) {
ADD_FAILURE()
<< "TestTU failed to build (suppress with /*error-ok*/): \n"

1
clang-tools-extra/clangd/unittests/TestTU.h
View File

@ -78,6 +78,7 @@ struct TestTU {
// By default, build() will report Error diagnostics as GTest errors.
// Suppress this behavior by adding an 'error-ok' comment to the code.
// The result will always have getDiagnostics() populated.
ParsedAST build() const;
std::shared_ptr<const PreambleData>
preamble(PreambleParsedCallback PreambleCallback = nullptr) const;

2
clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp
View File

@ -398,7 +398,7 @@ TEST(TypeHierarchy, RecursiveHierarchyUnbounded) {
// The compiler should produce a diagnostic for hitting the
// template instantiation depth.
ASSERT_TRUE(!AST.getDiagnostics().empty());
ASSERT_TRUE(!AST.getDiagnostics()->empty());
// Make sure getTypeHierarchy() doesn't get into an infinite recursion.
// The parent is reported as "S" because "S<0>" is an invalid instantiation.