Skip to content

Commit dd02825

Browse files
author
Eric Liu
committedApr 8, 2019
[clangd] Add fallback mode for code completion when compile command or preamble is not ready.
Summary: When calling TUScehduler::runWithPreamble (e.g. in code compleiton), allow entering a fallback mode when compile command or preamble is not ready, instead of waiting. This allows clangd to perform naive code completion e.g. using identifiers in the current file or symbols in the index. This patch simply returns empty result for code completion in fallback mode. Identifier-based plus more advanced index-based completion will be added in followup patches. Reviewers: ilya-biryukov, sammccall Reviewed By: sammccall Subscribers: sammccall, javed.absar, MaskRay, jkorous, arphaman, kadircet, jdoerfert, cfe-commits Tags: #clang Differential Revision: https://reviews.llvm.org/D59811 llvm-svn: 357916
1 parent c70d38e commit dd02825

File tree

6 files changed

+96
-17
lines changed

6 files changed

+96
-17
lines changed
 

‎clang-tools-extra/clangd/ClangdServer.cpp

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,9 @@
1111
#include "CodeComplete.h"
1212
#include "FindSymbols.h"
1313
#include "Headers.h"
14+
#include "Protocol.h"
1415
#include "SourceCode.h"
16+
#include "TUScheduler.h"
1517
#include "Trace.h"
1618
#include "index/CanonicalIncludes.h"
1719
#include "index/FileIndex.h"
@@ -21,6 +23,7 @@
2123
#include "clang/Frontend/CompilerInstance.h"
2224
#include "clang/Frontend/CompilerInvocation.h"
2325
#include "clang/Lex/Preprocessor.h"
26+
#include "clang/Sema/CodeCompleteConsumer.h"
2427
#include "clang/Tooling/CompilationDatabase.h"
2528
#include "clang/Tooling/Core/Replacement.h"
2629
#include "clang/Tooling/Refactoring/RefactoringResultConsumer.h"
@@ -152,6 +155,7 @@ void ClangdServer::addDocument(PathRef File, llvm::StringRef Contents,
152155
if (ClangTidyOptProvider)
153156
Opts.ClangTidyOpts = ClangTidyOptProvider->getOptions(File);
154157
Opts.SuggestMissingIncludes = SuggestMissingIncludes;
158+
155159
// FIXME: some build systems like Bazel will take time to preparing
156160
// environment to build the file, it would be nice if we could emit a
157161
// "PreparingBuild" status to inform users, it is non-trivial given the
@@ -183,6 +187,18 @@ void ClangdServer::codeComplete(PathRef File, Position Pos,
183187
return CB(IP.takeError());
184188
if (isCancelled())
185189
return CB(llvm::make_error<CancelledError>());
190+
if (!IP->Preamble) {
191+
vlog("File {0} is not ready for code completion. Enter fallback mode.",
192+
File);
193+
CodeCompleteResult CCR;
194+
CCR.Context = CodeCompletionContext::CCC_Recovery;
195+
196+
// FIXME: perform simple completion e.g. using identifiers in the current
197+
// file and symbols in the index.
198+
// FIXME: let clients know that we've entered fallback mode.
199+
200+
return CB(std::move(CCR));
201+
}
186202

187203
llvm::Optional<SpeculativeFuzzyFind> SpecFuzzyFind;
188204
if (CodeCompleteOpts.Index && CodeCompleteOpts.SpeculativeIndexRequest) {
@@ -214,7 +230,9 @@ void ClangdServer::codeComplete(PathRef File, Position Pos,
214230
};
215231

216232
// We use a potentially-stale preamble because latency is critical here.
217-
WorkScheduler.runWithPreamble("CodeComplete", File, TUScheduler::Stale,
233+
WorkScheduler.runWithPreamble("CodeComplete", File,
234+
Opts.AllowFallback ? TUScheduler::StaleOrAbsent
235+
: TUScheduler::Stale,
218236
Bind(Task, File.str(), std::move(CB)));
219237
}
220238

‎clang-tools-extra/clangd/CodeComplete.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,12 @@ struct CodeCompleteOptions {
109109
///
110110
/// Such completions can insert scope qualifiers.
111111
bool AllScopes = false;
112+
113+
/// Whether to allow falling back to code completion without compiling files
114+
/// (using identifiers in the current file and symbol indexes), when file
115+
/// cannot be built (e.g. missing compile command), or the build is not ready
116+
/// (e.g. preamble is still being built).
117+
bool AllowFallback = false;
112118
};
113119

114120
// Semi-structured representation of a code-complete suggestion for our C++ API.

‎clang-tools-extra/clangd/TUScheduler.cpp

Lines changed: 16 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@
4747
#include "Trace.h"
4848
#include "index/CanonicalIncludes.h"
4949
#include "clang/Frontend/CompilerInvocation.h"
50+
#include "llvm/ADT/Optional.h"
5051
#include "llvm/ADT/ScopeExit.h"
5152
#include "llvm/Support/Errc.h"
5253
#include "llvm/Support/Path.h"
@@ -179,6 +180,7 @@ class ASTWorker {
179180
bool blockUntilIdle(Deadline Timeout) const;
180181

181182
std::shared_ptr<const PreambleData> getPossiblyStalePreamble() const;
183+
182184
/// Obtain a preamble reflecting all updates so far. Threadsafe.
183185
/// It may be delivered immediately, or later on the worker thread.
184186
void getCurrentPreamble(
@@ -242,7 +244,6 @@ class ASTWorker {
242244
/// Whether the diagnostics for the current FileInputs were reported to the
243245
/// users before.
244246
bool DiagsWereReported = false;
245-
/// Size of the last AST
246247
/// Guards members used by both TUScheduler and the worker thread.
247248
mutable std::mutex Mutex;
248249
std::shared_ptr<const PreambleData> LastBuiltPreamble; /* GUARDED_BY(Mutex) */
@@ -858,9 +859,9 @@ void TUScheduler::runWithAST(
858859
It->second->Worker->runWithAST(Name, std::move(Action));
859860
}
860861

861-
void TUScheduler::runWithPreamble(
862-
llvm::StringRef Name, PathRef File, PreambleConsistency Consistency,
863-
llvm::unique_function<void(llvm::Expected<InputsAndPreamble>)> Action) {
862+
void TUScheduler::runWithPreamble(llvm::StringRef Name, PathRef File,
863+
PreambleConsistency Consistency,
864+
Callback<InputsAndPreamble> Action) {
864865
auto It = Files.find(File);
865866
if (It == Files.end()) {
866867
Action(llvm::make_error<LSPError>(
@@ -893,19 +894,21 @@ void TUScheduler::runWithPreamble(
893894
}
894895

895896
std::shared_ptr<const ASTWorker> Worker = It->second->Worker.lock();
896-
auto Task = [Worker, this](std::string Name, std::string File,
897-
std::string Contents,
898-
tooling::CompileCommand Command, Context Ctx,
899-
decltype(ConsistentPreamble) ConsistentPreamble,
900-
decltype(Action) Action) mutable {
897+
auto Task = [Worker, Consistency,
898+
this](std::string Name, std::string File, std::string Contents,
899+
tooling::CompileCommand Command, Context Ctx,
900+
decltype(ConsistentPreamble) ConsistentPreamble,
901+
decltype(Action) Action) mutable {
901902
std::shared_ptr<const PreambleData> Preamble;
902903
if (ConsistentPreamble.valid()) {
903904
Preamble = ConsistentPreamble.get();
904905
} else {
905-
// We don't want to be running preamble actions before the preamble was
906-
// built for the first time. This avoids extra work of processing the
907-
// preamble headers in parallel multiple times.
908-
Worker->waitForFirstPreamble();
906+
if (Consistency != PreambleConsistency::StaleOrAbsent) {
907+
// Wait until the preamble is built for the first time, if preamble is
908+
// required. This avoids extra work of processing the preamble headers
909+
// in parallel multiple times.
910+
Worker->waitForFirstPreamble();
911+
}
909912
Preamble = Worker->getPossiblyStalePreamble();
910913
}
911914

‎clang-tools-extra/clangd/TUScheduler.h

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,9 @@
1313
#include "Function.h"
1414
#include "Threading.h"
1515
#include "index/CanonicalIncludes.h"
16+
#include "llvm/ADT/Optional.h"
1617
#include "llvm/ADT/StringMap.h"
18+
#include "llvm/ADT/StringRef.h"
1719
#include <future>
1820

1921
namespace clang {
@@ -32,6 +34,7 @@ struct InputsAndAST {
3234
struct InputsAndPreamble {
3335
llvm::StringRef Contents;
3436
const tooling::CompileCommand &Command;
37+
// This can be nullptr if no preamble is availble.
3538
const PreambleData *Preamble;
3639
};
3740

@@ -178,10 +181,14 @@ class TUScheduler {
178181
/// reading source code from headers.
179182
/// This is the fastest option, usually a preamble is available immediately.
180183
Stale,
184+
/// Besides accepting stale preamble, this also allow preamble to be absent
185+
/// (not ready or failed to build).
186+
StaleOrAbsent,
181187
};
188+
182189
/// Schedule an async read of the preamble.
183-
/// If there's no preamble yet (because the file was just opened), we'll wait
184-
/// for it to build. The result may be null if it fails to build or is empty.
190+
/// If there's no up-to-date preamble, we follow the PreambleConsistency
191+
/// policy.
185192
/// If an error occurs, it is forwarded to the \p Action callback.
186193
/// Context cancellation is ignored and should be handled by the Action.
187194
/// (In practice, the Action is almost always executed immediately).

‎clang-tools-extra/clangd/tool/ClangdMain.cpp

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -231,6 +231,14 @@ static llvm::cl::opt<OffsetEncoding> ForceOffsetEncoding(
231231
"Offsets are in UTF-16 code units")),
232232
llvm::cl::init(OffsetEncoding::UnsupportedEncoding));
233233

234+
static llvm::cl::opt<bool> AllowFallbackCompletion(
235+
"allow-fallback-completion",
236+
llvm::cl::desc(
237+
"Allow falling back to code completion without compiling files (using "
238+
"identifiers and symbol indexes), when file cannot be built or the "
239+
"build is not ready."),
240+
llvm::cl::init(false));
241+
234242
namespace {
235243

236244
/// \brief Supports a test URI scheme with relaxed constraints for lit tests.
@@ -437,6 +445,7 @@ int main(int argc, char *argv[]) {
437445
CCOpts.SpeculativeIndexRequest = Opts.StaticIndex;
438446
CCOpts.EnableFunctionArgSnippets = EnableFunctionArgSnippets;
439447
CCOpts.AllScopes = AllScopesCompletion;
448+
CCOpts.AllowFallback = AllowFallbackCompletion;
440449

441450
RealFileSystemProvider FSProvider;
442451
// Initialize and run ClangdLSPServer.

‎clang-tools-extra/unittests/clangd/ClangdTests.cpp

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,10 @@
1313
#include "Matchers.h"
1414
#include "SyncAPI.h"
1515
#include "TestFS.h"
16+
#include "Threading.h"
1617
#include "URI.h"
1718
#include "clang/Config/config.h"
19+
#include "clang/Sema/CodeCompleteConsumer.h"
1820
#include "llvm/ADT/SmallVector.h"
1921
#include "llvm/ADT/StringMap.h"
2022
#include "llvm/Support/Errc.h"
@@ -36,7 +38,6 @@ namespace clangd {
3638
namespace {
3739

3840
using ::testing::ElementsAre;
39-
using ::testing::Eq;
4041
using ::testing::Field;
4142
using ::testing::Gt;
4243
using ::testing::IsEmpty;
@@ -1058,6 +1059,41 @@ TEST_F(ClangdVFSTest, FlagsWithPlugins) {
10581059
EXPECT_NE(Result, "<no-ast>");
10591060
}
10601061

1062+
TEST_F(ClangdVFSTest, FallbackWhenPreambleIsNotReady) {
1063+
MockFSProvider FS;
1064+
ErrorCheckingDiagConsumer DiagConsumer;
1065+
MockCompilationDatabase CDB;
1066+
ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest());
1067+
1068+
auto FooCpp = testPath("foo.cpp");
1069+
Annotations Code(R"cpp(
1070+
int main() {
1071+
int xyz;
1072+
xy^
1073+
})cpp");
1074+
FS.Files[FooCpp] = FooCpp;
1075+
1076+
auto Opts = clangd::CodeCompleteOptions();
1077+
Opts.AllowFallback = true;
1078+
1079+
// This will make compile command broken and preamble absent.
1080+
CDB.ExtraClangFlags = {"yolo.cc"};
1081+
Server.addDocument(FooCpp, Code.code());
1082+
ASSERT_TRUE(Server.blockUntilIdleForTest());
1083+
auto Res = cantFail(runCodeComplete(Server, FooCpp, Code.point(), Opts));
1084+
EXPECT_THAT(Res.Completions, IsEmpty());
1085+
EXPECT_EQ(Res.Context, CodeCompletionContext::CCC_Recovery);
1086+
1087+
// Make the compile command work again.
1088+
CDB.ExtraClangFlags = {"-std=c++11"};
1089+
Server.addDocument(FooCpp, Code.code());
1090+
ASSERT_TRUE(Server.blockUntilIdleForTest());
1091+
EXPECT_THAT(cantFail(runCodeComplete(Server, FooCpp, Code.point(),
1092+
Opts))
1093+
.Completions,
1094+
ElementsAre(Field(&CodeCompletion::Name, "xyz")));
1095+
}
1096+
10611097
} // namespace
10621098
} // namespace clangd
10631099
} // namespace clang

0 commit comments

Comments
 (0)
Please sign in to comment.