[clang-tools-extra] 2772c3a - [clangd] Introduce pullDiags endpoint
Kadir Cetinkaya via cfe-commits
cfe-commits at lists.llvm.org
Tue Mar 16 04:57:33 PDT 2021
Author: Kadir Cetinkaya
Date: 2021-03-16T12:52:15+01:00
New Revision: 2772c3a9752289ffec473b62f84855262a22de0b
URL: https://github.com/llvm/llvm-project/commit/2772c3a9752289ffec473b62f84855262a22de0b
DIFF: https://github.com/llvm/llvm-project/commit/2772c3a9752289ffec473b62f84855262a22de0b.diff
LOG: [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
https://github.com/microsoft/vscode-languageserver-node/blob/d15eb0671e0947d14e3548d13754104ee13aa56d/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
Added:
Modified:
clang-tools-extra/clangd/ClangdServer.cpp
clang-tools-extra/clangd/ClangdServer.h
clang-tools-extra/clangd/ParsedAST.cpp
clang-tools-extra/clangd/ParsedAST.h
clang-tools-extra/clangd/Preamble.h
clang-tools-extra/clangd/tool/Check.cpp
clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
clang-tools-extra/clangd/unittests/ModulesTests.cpp
clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
clang-tools-extra/clangd/unittests/PreambleTests.cpp
clang-tools-extra/clangd/unittests/SelectionTests.cpp
clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
clang-tools-extra/clangd/unittests/TestTU.cpp
clang-tools-extra/clangd/unittests/TestTU.h
clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clangd/ClangdServer.cpp b/clang-tools-extra/clangd/ClangdServer.cpp
index e9724e7516aa..f9516a1537a0 100644
--- a/clang-tools-extra/clangd/ClangdServer.cpp
+++ b/clang-tools-extra/clangd/ClangdServer.cpp
@@ -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();
}
diff --git a/clang-tools-extra/clangd/ClangdServer.h b/clang-tools-extra/clangd/ClangdServer.h
index b633d3139683..37ac30f70cb4 100644
--- a/clang-tools-extra/clangd/ClangdServer.h
+++ b/clang-tools-extra/clangd/ClangdServer.h
@@ -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 @@ class ClangdServer {
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 @@ class ClangdServer {
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
diff --git a/clang-tools-extra/clangd/ParsedAST.cpp b/clang-tools-extra/clangd/ParsedAST.cpp
index 24038f0ec35f..119263f0a891 100644
--- a/clang-tools-extra/clangd/ParsedAST.cpp
+++ b/clang-tools-extra/clangd/ParsedAST.cpp
@@ -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)),
diff --git a/clang-tools-extra/clangd/ParsedAST.h b/clang-tools-extra/clangd/ParsedAST.h
index 263a097b14cb..c1ce6fce7029 100644
--- a/clang-tools-extra/clangd/ParsedAST.h
+++ b/clang-tools-extra/clangd/ParsedAST.h
@@ -88,7 +88,9 @@ class ParsedAST {
/// (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 @@ class ParsedAST {
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 @@ class ParsedAST {
/// 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;
diff --git a/clang-tools-extra/clangd/Preamble.h b/clang-tools-extra/clangd/Preamble.h
index 1de1d6cfe5fa..5b9d17840214 100644
--- a/clang-tools-extra/clangd/Preamble.h
+++ b/clang-tools-extra/clangd/Preamble.h
@@ -126,6 +126,9 @@ class PreamblePatch {
/// 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;
diff --git a/clang-tools-extra/clangd/tool/Check.cpp b/clang-tools-extra/clangd/tool/Check.cpp
index 00a4609e5134..20b86daff8af 100644
--- a/clang-tools-extra/clangd/tool/Check.cpp
+++ b/clang-tools-extra/clangd/tool/Check.cpp
@@ -181,7 +181,7 @@ class Checker {
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) {
diff --git a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
index aebb231f39f9..d5b4a08a4229 100644
--- a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -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
diff erent 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) {
diff --git a/clang-tools-extra/clangd/unittests/ModulesTests.cpp b/clang-tools-extra/clangd/unittests/ModulesTests.cpp
index 83d6b28d6dfc..b56b91836508 100644
--- a/clang-tools-extra/clangd/unittests/ModulesTests.cpp
+++ b/clang-tools-extra/clangd/unittests/ModulesTests.cpp
@@ -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) {
diff --git a/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp b/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
index b96d1243d243..5435648cd9be 100644
--- a/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
+++ b/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
@@ -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,
diff --git a/clang-tools-extra/clangd/unittests/PreambleTests.cpp b/clang-tools-extra/clangd/unittests/PreambleTests.cpp
index 4eee9effb824..70a14241a8ac 100644
--- a/clang-tools-extra/clangd/unittests/PreambleTests.cpp
+++ b/clang-tools-extra/clangd/unittests/PreambleTests.cpp
@@ -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
diff --git a/clang-tools-extra/clangd/unittests/SelectionTests.cpp b/clang-tools-extra/clangd/unittests/SelectionTests.cpp
index e9c689f329ab..a063c84a6a4c 100644
--- a/clang-tools-extra/clangd/unittests/SelectionTests.cpp
+++ b/clang-tools-extra/clangd/unittests/SelectionTests.cpp
@@ -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());
diff --git a/clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp b/clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
index 22b6ea2296d2..5f8faf78df3c 100644
--- a/clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
+++ b/clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
@@ -121,7 +121,7 @@ class TUSchedulerTests : public ::testing::Test {
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,
diff --git a/clang-tools-extra/clangd/unittests/TestTU.cpp b/clang-tools-extra/clangd/unittests/TestTU.cpp
index 8d336b3f4e19..a36f2508cd31 100644
--- a/clang-tools-extra/clangd/unittests/TestTU.cpp
+++ b/clang-tools-extra/clangd/unittests/TestTU.cpp
@@ -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"
diff --git a/clang-tools-extra/clangd/unittests/TestTU.h b/clang-tools-extra/clangd/unittests/TestTU.h
index 18b490332b1a..169cab045ea1 100644
--- a/clang-tools-extra/clangd/unittests/TestTU.h
+++ b/clang-tools-extra/clangd/unittests/TestTU.h
@@ -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;
diff --git a/clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp b/clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp
index 08f936ce8b55..09f90fd6e6b5 100644
--- a/clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp
+++ b/clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp
@@ -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.
More information about the cfe-commits
mailing list