Skip to content

Commit 6ff0228

Browse files
committed
[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
1 parent b50431d commit 6ff0228

File tree

8 files changed

+71
-7
lines changed

8 files changed

+71
-7
lines changed

clang-tools-extra/clangd/ClangdLSPServer.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -647,7 +647,7 @@ void ClangdLSPServer::onDocumentDidChange(
647647
return;
648648
}
649649

650-
Server->addDocument(File, *Contents, WantDiags);
650+
Server->addDocument(File, *Contents, WantDiags, Params.forceRebuild);
651651
}
652652

653653
void ClangdLSPServer::onFileEvent(const DidChangeWatchedFilesParams &Params) {

clang-tools-extra/clangd/ClangdServer.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -170,7 +170,7 @@ ClangdServer::ClangdServer(const GlobalCompilationDatabase &CDB,
170170
}
171171

172172
void ClangdServer::addDocument(PathRef File, llvm::StringRef Contents,
173-
WantDiagnostics WantDiags) {
173+
WantDiagnostics WantDiags, bool ForceRebuild) {
174174
auto FS = FSProvider.getFileSystem();
175175

176176
ParseOptions Opts;
@@ -184,6 +184,7 @@ void ClangdServer::addDocument(PathRef File, llvm::StringRef Contents,
184184
ParseInputs Inputs;
185185
Inputs.FS = FS;
186186
Inputs.Contents = std::string(Contents);
187+
Inputs.ForceRebuild = ForceRebuild;
187188
Inputs.Opts = std::move(Opts);
188189
Inputs.Index = Index;
189190
bool NewFile = WorkScheduler.update(File, Inputs, WantDiags);

clang-tools-extra/clangd/ClangdServer.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -172,7 +172,8 @@ class ClangdServer {
172172
/// separate thread. When the parsing is complete, DiagConsumer passed in
173173
/// constructor will receive onDiagnosticsReady callback.
174174
void addDocument(PathRef File, StringRef Contents,
175-
WantDiagnostics WD = WantDiagnostics::Auto);
175+
WantDiagnostics WD = WantDiagnostics::Auto,
176+
bool ForceRebuild = false);
176177

177178
/// Get the contents of \p File, which should have been added.
178179
llvm::StringRef getDocument(PathRef File) const;

clang-tools-extra/clangd/Compiler.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,9 @@ struct ParseInputs {
4545
tooling::CompileCommand CompileCommand;
4646
IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS;
4747
std::string Contents;
48+
// Prevent reuse of the cached preamble/AST. Slow! Useful to workaround
49+
// clangd's assumption that missing header files will stay missing.
50+
bool ForceRebuild = false;
4851
// Used to recover from diagnostics (e.g. find missing includes for symbol).
4952
const SymbolIndex *Index = nullptr;
5053
ParseOptions Opts;

clang-tools-extra/clangd/Protocol.cpp

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -430,7 +430,10 @@ bool fromJSON(const llvm::json::Value &Params, DidCloseTextDocumentParams &R) {
430430

431431
bool fromJSON(const llvm::json::Value &Params, DidChangeTextDocumentParams &R) {
432432
llvm::json::ObjectMapper O(Params);
433-
return O && O.map("textDocument", R.textDocument) &&
433+
if (!O)
434+
return false;
435+
O.map("forceRebuild", R.forceRebuild); // Optional clangd extension.
436+
return O.map("textDocument", R.textDocument) &&
434437
O.map("contentChanges", R.contentChanges) &&
435438
O.map("wantDiagnostics", R.wantDiagnostics);
436439
}

clang-tools-extra/clangd/Protocol.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -652,6 +652,12 @@ struct DidChangeTextDocumentParams {
652652
/// either they will be provided for this version or some subsequent one.
653653
/// This is a clangd extension.
654654
llvm::Optional<bool> wantDiagnostics;
655+
656+
/// Force a complete rebuild of the file, ignoring all cached state. Slow!
657+
/// This is useful to defeat clangd's assumption that missing headers will
658+
/// stay missing.
659+
/// This is a clangd extension.
660+
bool forceRebuild = false;
655661
};
656662
bool fromJSON(const llvm::json::Value &, DidChangeTextDocumentParams &);
657663

clang-tools-extra/clangd/TUScheduler.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -436,7 +436,8 @@ void ASTWorker::update(ParseInputs Inputs, WantDiagnostics WantDiags) {
436436
}
437437

438438
std::shared_ptr<const PreambleData> OldPreamble =
439-
getPossiblyStalePreamble();
439+
Inputs.ForceRebuild ? std::shared_ptr<const PreambleData>()
440+
: getPossiblyStalePreamble();
440441
std::shared_ptr<const PreambleData> NewPreamble = buildPreamble(
441442
FileName, *Invocation, OldPreamble, OldCommand, Inputs,
442443
StorePreambleInMemory,

clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp

Lines changed: 51 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -616,6 +616,53 @@ TEST_F(TUSchedulerTests, NoopOnEmptyChanges) {
616616
ASSERT_FALSE(DoUpdate(OtherSourceContents));
617617
}
618618

619+
TEST_F(TUSchedulerTests, ForceRebuild) {
620+
TUScheduler S(CDB, optsForTest(), captureDiags());
621+
622+
auto Source = testPath("foo.cpp");
623+
auto Header = testPath("foo.h");
624+
625+
auto SourceContents = R"cpp(
626+
#include "foo.h"
627+
int b = a;
628+
)cpp";
629+
630+
ParseInputs Inputs = getInputs(Source, SourceContents);
631+
632+
// Update the source contents, which should trigger an initial build with
633+
// the header file missing.
634+
updateWithDiags(S, Source, Inputs, WantDiagnostics::Yes,
635+
[](std::vector<Diag> Diags) {
636+
EXPECT_THAT(
637+
Diags,
638+
ElementsAre(
639+
Field(&Diag::Message, "'foo.h' file not found"),
640+
Field(&Diag::Message, "use of undeclared identifier 'a'")));
641+
});
642+
643+
// Add the header file. We need to recreate the inputs since we changed a
644+
// file from underneath the test FS.
645+
Files[Header] = "int a;";
646+
Timestamps[Header] = time_t(1);
647+
Inputs = getInputs(Source, SourceContents);
648+
649+
// The addition of the missing header file shouldn't trigger a rebuild since
650+
// we don't track missing files.
651+
updateWithDiags(S, Source, Inputs, WantDiagnostics::Yes,
652+
[](std::vector<Diag> Diags) {
653+
ADD_FAILURE() << "Did not expect diagnostics for missing header update";
654+
});
655+
656+
// Forcing the reload should should cause a rebuild which no longer has any
657+
// errors.
658+
Inputs.ForceRebuild = true;
659+
updateWithDiags(S, Source, Inputs, WantDiagnostics::Yes,
660+
[](std::vector<Diag> Diags) {
661+
EXPECT_THAT(Diags, IsEmpty());
662+
});
663+
664+
ASSERT_TRUE(S.blockUntilIdle(timeoutSeconds(10)));
665+
}
619666
TEST_F(TUSchedulerTests, NoChangeDiags) {
620667
TUScheduler S(CDB, optsForTest(), captureDiags());
621668

@@ -722,7 +769,8 @@ TEST_F(TUSchedulerTests, CommandLineErrors) {
722769
TUScheduler S(CDB, optsForTest(), captureDiags());
723770
std::vector<Diag> Diagnostics;
724771
updateWithDiags(S, testPath("foo.cpp"), "void test() {}",
725-
WantDiagnostics::Yes, [&](std::vector<Diag> D) {
772+
WantDiagnostics::Yes,
773+
[&](std::vector<Diag> D) {
726774
Diagnostics = std::move(D);
727775
Ready.notify();
728776
});
@@ -746,7 +794,8 @@ TEST_F(TUSchedulerTests, CommandLineWarnings) {
746794
TUScheduler S(CDB, optsForTest(), captureDiags());
747795
std::vector<Diag> Diagnostics;
748796
updateWithDiags(S, testPath("foo.cpp"), "void test() {}",
749-
WantDiagnostics::Yes, [&](std::vector<Diag> D) {
797+
WantDiagnostics::Yes,
798+
[&](std::vector<Diag> D) {
750799
Diagnostics = std::move(D);
751800
Ready.notify();
752801
});

0 commit comments

Comments
 (0)