[clang-tools-extra] 6ff0228 - [clang] Add `forceReload` clangd extension to 'textDocument/didChange'
David Goldman via cfe-commits
cfe-commits at lists.llvm.org
Mon Feb 10 11:02:46 PST 2020
Author: David Goldman
Date: 2020-02-10T14:02:02-05:00
New Revision: 6ff0228c6df37e052fa6e8e3927e83b289402cf6
URL: https://github.com/llvm/llvm-project/commit/6ff0228c6df37e052fa6e8e3927e83b289402cf6
DIFF: https://github.com/llvm/llvm-project/commit/6ff0228c6df37e052fa6e8e3927e83b289402cf6.diff
LOG: [clang] Add `forceReload` clangd extension to 'textDocument/didChange'
Summary:
- This option forces a preamble rebuild to handle the odd case
of a missing header file being added
Reviewers: sammccall
Subscribers: ilya-biryukov, javed.absar, MaskRay, jkorous, arphaman, jfb, kadircet, usaxena95, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D73916
Added:
Modified:
clang-tools-extra/clangd/ClangdLSPServer.cpp
clang-tools-extra/clangd/ClangdServer.cpp
clang-tools-extra/clangd/ClangdServer.h
clang-tools-extra/clangd/Compiler.h
clang-tools-extra/clangd/Protocol.cpp
clang-tools-extra/clangd/Protocol.h
clang-tools-extra/clangd/TUScheduler.cpp
clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clangd/ClangdLSPServer.cpp b/clang-tools-extra/clangd/ClangdLSPServer.cpp
index 880359f21dda..93609a8852db 100644
--- a/clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ b/clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -647,7 +647,7 @@ void ClangdLSPServer::onDocumentDidChange(
return;
}
- Server->addDocument(File, *Contents, WantDiags);
+ Server->addDocument(File, *Contents, WantDiags, Params.forceRebuild);
}
void ClangdLSPServer::onFileEvent(const DidChangeWatchedFilesParams &Params) {
diff --git a/clang-tools-extra/clangd/ClangdServer.cpp b/clang-tools-extra/clangd/ClangdServer.cpp
index 3e22a4dfe667..48cf921ff18c 100644
--- a/clang-tools-extra/clangd/ClangdServer.cpp
+++ b/clang-tools-extra/clangd/ClangdServer.cpp
@@ -170,7 +170,7 @@ ClangdServer::ClangdServer(const GlobalCompilationDatabase &CDB,
}
void ClangdServer::addDocument(PathRef File, llvm::StringRef Contents,
- WantDiagnostics WantDiags) {
+ WantDiagnostics WantDiags, bool ForceRebuild) {
auto FS = FSProvider.getFileSystem();
ParseOptions Opts;
@@ -184,6 +184,7 @@ void ClangdServer::addDocument(PathRef File, llvm::StringRef Contents,
ParseInputs Inputs;
Inputs.FS = FS;
Inputs.Contents = std::string(Contents);
+ Inputs.ForceRebuild = ForceRebuild;
Inputs.Opts = std::move(Opts);
Inputs.Index = Index;
bool NewFile = WorkScheduler.update(File, Inputs, WantDiags);
diff --git a/clang-tools-extra/clangd/ClangdServer.h b/clang-tools-extra/clangd/ClangdServer.h
index 3c3505295a75..5156520e2d07 100644
--- a/clang-tools-extra/clangd/ClangdServer.h
+++ b/clang-tools-extra/clangd/ClangdServer.h
@@ -172,7 +172,8 @@ class ClangdServer {
/// separate thread. When the parsing is complete, DiagConsumer passed in
/// constructor will receive onDiagnosticsReady callback.
void addDocument(PathRef File, StringRef Contents,
- WantDiagnostics WD = WantDiagnostics::Auto);
+ WantDiagnostics WD = WantDiagnostics::Auto,
+ bool ForceRebuild = false);
/// Get the contents of \p File, which should have been added.
llvm::StringRef getDocument(PathRef File) const;
diff --git a/clang-tools-extra/clangd/Compiler.h b/clang-tools-extra/clangd/Compiler.h
index 51414c37fc04..356293b158f8 100644
--- a/clang-tools-extra/clangd/Compiler.h
+++ b/clang-tools-extra/clangd/Compiler.h
@@ -45,6 +45,9 @@ struct ParseInputs {
tooling::CompileCommand CompileCommand;
IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS;
std::string Contents;
+ // Prevent reuse of the cached preamble/AST. Slow! Useful to workaround
+ // clangd's assumption that missing header files will stay missing.
+ bool ForceRebuild = false;
// Used to recover from diagnostics (e.g. find missing includes for symbol).
const SymbolIndex *Index = nullptr;
ParseOptions Opts;
diff --git a/clang-tools-extra/clangd/Protocol.cpp b/clang-tools-extra/clangd/Protocol.cpp
index aabf0fa11d45..d607ffbbc815 100644
--- a/clang-tools-extra/clangd/Protocol.cpp
+++ b/clang-tools-extra/clangd/Protocol.cpp
@@ -430,7 +430,10 @@ bool fromJSON(const llvm::json::Value &Params, DidCloseTextDocumentParams &R) {
bool fromJSON(const llvm::json::Value &Params, DidChangeTextDocumentParams &R) {
llvm::json::ObjectMapper O(Params);
- return O && O.map("textDocument", R.textDocument) &&
+ if (!O)
+ return false;
+ O.map("forceRebuild", R.forceRebuild); // Optional clangd extension.
+ return O.map("textDocument", R.textDocument) &&
O.map("contentChanges", R.contentChanges) &&
O.map("wantDiagnostics", R.wantDiagnostics);
}
diff --git a/clang-tools-extra/clangd/Protocol.h b/clang-tools-extra/clangd/Protocol.h
index 255629f0a0ec..3275cbbd17b1 100644
--- a/clang-tools-extra/clangd/Protocol.h
+++ b/clang-tools-extra/clangd/Protocol.h
@@ -652,6 +652,12 @@ struct DidChangeTextDocumentParams {
/// either they will be provided for this version or some subsequent one.
/// This is a clangd extension.
llvm::Optional<bool> wantDiagnostics;
+
+ /// Force a complete rebuild of the file, ignoring all cached state. Slow!
+ /// This is useful to defeat clangd's assumption that missing headers will
+ /// stay missing.
+ /// This is a clangd extension.
+ bool forceRebuild = false;
};
bool fromJSON(const llvm::json::Value &, DidChangeTextDocumentParams &);
diff --git a/clang-tools-extra/clangd/TUScheduler.cpp b/clang-tools-extra/clangd/TUScheduler.cpp
index 12a062378202..5a1caa964520 100644
--- a/clang-tools-extra/clangd/TUScheduler.cpp
+++ b/clang-tools-extra/clangd/TUScheduler.cpp
@@ -436,7 +436,8 @@ void ASTWorker::update(ParseInputs Inputs, WantDiagnostics WantDiags) {
}
std::shared_ptr<const PreambleData> OldPreamble =
- getPossiblyStalePreamble();
+ Inputs.ForceRebuild ? std::shared_ptr<const PreambleData>()
+ : getPossiblyStalePreamble();
std::shared_ptr<const PreambleData> NewPreamble = buildPreamble(
FileName, *Invocation, OldPreamble, OldCommand, Inputs,
StorePreambleInMemory,
diff --git a/clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp b/clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
index 6605ccf65860..214099767990 100644
--- a/clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
+++ b/clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
@@ -616,6 +616,53 @@ TEST_F(TUSchedulerTests, NoopOnEmptyChanges) {
ASSERT_FALSE(DoUpdate(OtherSourceContents));
}
+TEST_F(TUSchedulerTests, ForceRebuild) {
+ TUScheduler S(CDB, optsForTest(), captureDiags());
+
+ auto Source = testPath("foo.cpp");
+ auto Header = testPath("foo.h");
+
+ auto SourceContents = R"cpp(
+ #include "foo.h"
+ int b = a;
+ )cpp";
+
+ ParseInputs Inputs = getInputs(Source, SourceContents);
+
+ // Update the source contents, which should trigger an initial build with
+ // the header file missing.
+ updateWithDiags(S, Source, Inputs, WantDiagnostics::Yes,
+ [](std::vector<Diag> Diags) {
+ EXPECT_THAT(
+ Diags,
+ ElementsAre(
+ Field(&Diag::Message, "'foo.h' file not found"),
+ Field(&Diag::Message, "use of undeclared identifier 'a'")));
+ });
+
+ // Add the header file. We need to recreate the inputs since we changed a
+ // file from underneath the test FS.
+ Files[Header] = "int a;";
+ Timestamps[Header] = time_t(1);
+ Inputs = getInputs(Source, SourceContents);
+
+ // The addition of the missing header file shouldn't trigger a rebuild since
+ // we don't track missing files.
+ updateWithDiags(S, Source, Inputs, WantDiagnostics::Yes,
+ [](std::vector<Diag> Diags) {
+ ADD_FAILURE() << "Did not expect diagnostics for missing header update";
+ });
+
+ // Forcing the reload should should cause a rebuild which no longer has any
+ // errors.
+ Inputs.ForceRebuild = true;
+ updateWithDiags(S, Source, Inputs, WantDiagnostics::Yes,
+ [](std::vector<Diag> Diags) {
+ EXPECT_THAT(Diags, IsEmpty());
+ });
+
+ ASSERT_TRUE(S.blockUntilIdle(timeoutSeconds(10)));
+}
TEST_F(TUSchedulerTests, NoChangeDiags) {
TUScheduler S(CDB, optsForTest(), captureDiags());
@@ -722,7 +769,8 @@ TEST_F(TUSchedulerTests, CommandLineErrors) {
TUScheduler S(CDB, optsForTest(), captureDiags());
std::vector<Diag> Diagnostics;
updateWithDiags(S, testPath("foo.cpp"), "void test() {}",
- WantDiagnostics::Yes, [&](std::vector<Diag> D) {
+ WantDiagnostics::Yes,
+ [&](std::vector<Diag> D) {
Diagnostics = std::move(D);
Ready.notify();
});
@@ -746,7 +794,8 @@ TEST_F(TUSchedulerTests, CommandLineWarnings) {
TUScheduler S(CDB, optsForTest(), captureDiags());
std::vector<Diag> Diagnostics;
updateWithDiags(S, testPath("foo.cpp"), "void test() {}",
- WantDiagnostics::Yes, [&](std::vector<Diag> D) {
+ WantDiagnostics::Yes,
+ [&](std::vector<Diag> D) {
Diagnostics = std::move(D);
Ready.notify();
});
More information about the cfe-commits
mailing list