dgoldman updated this revision to Diff 243251.
dgoldman marked 5 inline comments as done.
dgoldman added a comment.
- Refactor to use `forceRebuild` and `ParseOptions`
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D73916/new/
https://reviews.llvm.org/D73916
Files:
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
Index: clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
+++ clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
@@ -615,6 +615,46 @@
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";
+
+ // Return value indicates if the updated callback was received.
+ auto DoUpdate = [&](std::string Contents, bool ForceRebuild) -> bool {
+ std::atomic<bool> Updated(false);
+ Updated = false;
+ ParseInputs Inputs = getInputs(Source, std::string(Contents));
+ Inputs.Opts.ForceRebuild = ForceRebuild;
+ updateWithDiags(S, Source, Inputs, WantDiagnostics::Yes,
+ [&Updated](std::vector<Diag>) { Updated = true; });
+ bool UpdateFinished = S.blockUntilIdle(timeoutSeconds(10));
+ if (!UpdateFinished)
+ ADD_FAILURE() << "Updated has not finished in one second. Threading bug?";
+ return Updated;
+ };
+
+ // Update the source contents, which should trigger an initial build with
+ // the header file missing.
+ ASSERT_TRUE(DoUpdate(SourceContents, /*ForceRebuild*/false));
+
+ // Add the header file.
+ Files[Header] = "int a;";
+ Timestamps[Header] = time_t(1);
+
+ // The addition of the missing header file shouldn't trigger a rebuild since
+ // we don't track missing files.
+ ASSERT_FALSE(DoUpdate(SourceContents, /*ForceRebuild*/false));
+
+ // Forcing the reload should should cause a rebuild, though.
+ ASSERT_TRUE(DoUpdate(SourceContents, /*ForceRebuild*/true));
+}
TEST_F(TUSchedulerTests, NoChangeDiags) {
TUScheduler S(CDB, optsForTest(), captureDiags());
@@ -721,7 +761,8 @@
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();
});
@@ -745,7 +786,8 @@
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();
});
Index: clang-tools-extra/clangd/TUScheduler.cpp
===================================================================
--- clang-tools-extra/clangd/TUScheduler.cpp
+++ clang-tools-extra/clangd/TUScheduler.cpp
@@ -432,7 +432,8 @@
}
std::shared_ptr<const PreambleData> OldPreamble =
- getPossiblyStalePreamble();
+ Inputs.Opts.ForceRebuild ? std::shared_ptr<const PreambleData>()
+ : getPossiblyStalePreamble();
std::shared_ptr<const PreambleData> NewPreamble = buildPreamble(
FileName, *Invocation, OldPreamble, OldCommand, Inputs,
StorePreambleInMemory,
Index: clang-tools-extra/clangd/Protocol.h
===================================================================
--- clang-tools-extra/clangd/Protocol.h
+++ clang-tools-extra/clangd/Protocol.h
@@ -652,6 +652,12 @@
/// 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!
+ /// Disabled by default. This is useful to defeat clangd's assumption that
+ /// missing headers stay missing.
+ /// This is a clangd extension.
+ bool forceRebuild;
};
bool fromJSON(const llvm::json::Value &, DidChangeTextDocumentParams &);
Index: clang-tools-extra/clangd/Protocol.cpp
===================================================================
--- clang-tools-extra/clangd/Protocol.cpp
+++ clang-tools-extra/clangd/Protocol.cpp
@@ -430,7 +430,10 @@
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);
}
Index: clang-tools-extra/clangd/Compiler.h
===================================================================
--- clang-tools-extra/clangd/Compiler.h
+++ clang-tools-extra/clangd/Compiler.h
@@ -38,6 +38,7 @@
struct ParseOptions {
tidy::ClangTidyOptions ClangTidyOpts;
bool SuggestMissingIncludes = false;
+ bool ForceRebuild = false;
};
/// Information required to run clang, e.g. to parse AST or do code completion.
Index: clang-tools-extra/clangd/ClangdServer.h
===================================================================
--- clang-tools-extra/clangd/ClangdServer.h
+++ clang-tools-extra/clangd/ClangdServer.h
@@ -172,7 +172,8 @@
/// 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;
Index: clang-tools-extra/clangd/ClangdServer.cpp
===================================================================
--- clang-tools-extra/clangd/ClangdServer.cpp
+++ clang-tools-extra/clangd/ClangdServer.cpp
@@ -170,7 +170,7 @@
}
void ClangdServer::addDocument(PathRef File, llvm::StringRef Contents,
- WantDiagnostics WantDiags) {
+ WantDiagnostics WantDiags, bool ForceRebuild) {
auto FS = FSProvider.getFileSystem();
ParseOptions Opts;
@@ -179,6 +179,7 @@
if (GetClangTidyOptions)
Opts.ClangTidyOpts = GetClangTidyOptions(*FS, File);
Opts.SuggestMissingIncludes = SuggestMissingIncludes;
+ Opts.ForceRebuild = ForceRebuild;
// Compile command is set asynchronously during update, as it can be slow.
ParseInputs Inputs;
Index: clang-tools-extra/clangd/ClangdLSPServer.cpp
===================================================================
--- clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -647,7 +647,7 @@
return;
}
- Server->addDocument(File, *Contents, WantDiags);
+ Server->addDocument(File, *Contents, WantDiags, Params.forceRebuild);
}
void ClangdLSPServer::onFileEvent(const DidChangeWatchedFilesParams &Params) {
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
