ioeric updated this revision to Diff 194141.
ioeric marked 11 inline comments as done.
ioeric added a comment.
- address review comments
Repository:
rCTE Clang Tools Extra
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D59811/new/
https://reviews.llvm.org/D59811
Files:
clangd/ClangdServer.cpp
clangd/CodeComplete.h
clangd/TUScheduler.cpp
clangd/TUScheduler.h
clangd/tool/ClangdMain.cpp
unittests/clangd/ClangdTests.cpp
Index: unittests/clangd/ClangdTests.cpp
===================================================================
--- unittests/clangd/ClangdTests.cpp
+++ unittests/clangd/ClangdTests.cpp
@@ -13,8 +13,10 @@
#include "Matchers.h"
#include "SyncAPI.h"
#include "TestFS.h"
+#include "Threading.h"
#include "URI.h"
#include "clang/Config/config.h"
+#include "clang/Sema/CodeCompleteConsumer.h"
#include "llvm/ADT/SmallVector.h"
#include "llvm/ADT/StringMap.h"
#include "llvm/Support/Errc.h"
@@ -36,7 +38,6 @@
namespace {
using ::testing::ElementsAre;
-using ::testing::Eq;
using ::testing::Field;
using ::testing::Gt;
using ::testing::IsEmpty;
@@ -1058,6 +1059,41 @@
EXPECT_NE(Result, "<no-ast>");
}
+TEST_F(ClangdVFSTest, FallbackWhenPreambleIsNotReady) {
+ MockFSProvider FS;
+ ErrorCheckingDiagConsumer DiagConsumer;
+ MockCompilationDatabase CDB;
+ ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest());
+
+ auto FooCpp = testPath("foo.cpp");
+ Annotations Code(R"cpp(
+ int main() {
+ int xyz;
+ xy^
+ })cpp");
+ FS.Files[FooCpp] = FooCpp;
+
+ auto Opts = clangd::CodeCompleteOptions();
+ Opts.AllowFallback = true;
+
+ // This will make compile command broken and preamble absent.
+ CDB.ExtraClangFlags = {"yolo.cc"};
+ Server.addDocument(FooCpp, Code.code());
+ ASSERT_TRUE(Server.blockUntilIdleForTest());
+ auto Res = cantFail(runCodeComplete(Server, FooCpp, Code.point(), Opts));
+ EXPECT_THAT(Res.Completions, IsEmpty());
+ EXPECT_EQ(Res.Context, CodeCompletionContext::CCC_Recovery);
+
+ // Make the compile command work again.
+ CDB.ExtraClangFlags = {"-std=c++11"};
+ Server.addDocument(FooCpp, Code.code());
+ ASSERT_TRUE(Server.blockUntilIdleForTest());
+ EXPECT_THAT(cantFail(runCodeComplete(Server, FooCpp, Code.point(),
+ Opts))
+ .Completions,
+ ElementsAre(Field(&CodeCompletion::Name, "xyz")));
+}
+
} // namespace
} // namespace clangd
} // namespace clang
Index: clangd/tool/ClangdMain.cpp
===================================================================
--- clangd/tool/ClangdMain.cpp
+++ clangd/tool/ClangdMain.cpp
@@ -231,6 +231,14 @@
"Offsets are in UTF-16 code units")),
llvm::cl::init(OffsetEncoding::UnsupportedEncoding));
+static llvm::cl::opt<bool> AllowFallbackCompletion(
+ "allow-fallback-completion",
+ llvm::cl::desc(
+ "Allow falling back to code completion without compiling files (using "
+ "identifiers and symbol indexes), when file cannot be built or the "
+ "build is not ready."),
+ llvm::cl::init(false));
+
namespace {
/// \brief Supports a test URI scheme with relaxed constraints for lit tests.
@@ -437,6 +445,7 @@
CCOpts.SpeculativeIndexRequest = Opts.StaticIndex;
CCOpts.EnableFunctionArgSnippets = EnableFunctionArgSnippets;
CCOpts.AllScopes = AllScopesCompletion;
+ CCOpts.AllowFallback = AllowFallbackCompletion;
RealFileSystemProvider FSProvider;
// Initialize and run ClangdLSPServer.
Index: clangd/TUScheduler.h
===================================================================
--- clangd/TUScheduler.h
+++ clangd/TUScheduler.h
@@ -13,7 +13,9 @@
#include "Function.h"
#include "Threading.h"
#include "index/CanonicalIncludes.h"
+#include "llvm/ADT/Optional.h"
#include "llvm/ADT/StringMap.h"
+#include "llvm/ADT/StringRef.h"
#include <future>
namespace clang {
@@ -32,6 +34,7 @@
struct InputsAndPreamble {
llvm::StringRef Contents;
const tooling::CompileCommand &Command;
+ // This can be nullptr if no preamble is availble.
const PreambleData *Preamble;
};
@@ -178,10 +181,14 @@
/// reading source code from headers.
/// This is the fastest option, usually a preamble is available immediately.
Stale,
+ /// Besides accepting stale preamble, this also allow preamble to be absent
+ /// (not ready or failed to build).
+ StaleOrAbsent,
};
+
/// Schedule an async read of the preamble.
- /// If there's no preamble yet (because the file was just opened), we'll wait
- /// for it to build. The result may be null if it fails to build or is empty.
+ /// If there's no up-to-date preamble, we follow the PreambleConsistency
+ /// policy.
/// If an error occurs, it is forwarded to the \p Action callback.
/// Context cancellation is ignored and should be handled by the Action.
/// (In practice, the Action is almost always executed immediately).
Index: clangd/TUScheduler.cpp
===================================================================
--- clangd/TUScheduler.cpp
+++ clangd/TUScheduler.cpp
@@ -47,6 +47,7 @@
#include "Trace.h"
#include "index/CanonicalIncludes.h"
#include "clang/Frontend/CompilerInvocation.h"
+#include "llvm/ADT/Optional.h"
#include "llvm/ADT/ScopeExit.h"
#include "llvm/Support/Errc.h"
#include "llvm/Support/Path.h"
@@ -179,6 +180,7 @@
bool blockUntilIdle(Deadline Timeout) const;
std::shared_ptr<const PreambleData> getPossiblyStalePreamble() const;
+
/// Obtain a preamble reflecting all updates so far. Threadsafe.
/// It may be delivered immediately, or later on the worker thread.
void getCurrentPreamble(
@@ -242,7 +244,6 @@
/// Whether the diagnostics for the current FileInputs were reported to the
/// users before.
bool DiagsWereReported = false;
- /// Size of the last AST
/// Guards members used by both TUScheduler and the worker thread.
mutable std::mutex Mutex;
std::shared_ptr<const PreambleData> LastBuiltPreamble; /* GUARDED_BY(Mutex) */
@@ -858,9 +859,9 @@
It->second->Worker->runWithAST(Name, std::move(Action));
}
-void TUScheduler::runWithPreamble(
- llvm::StringRef Name, PathRef File, PreambleConsistency Consistency,
- llvm::unique_function<void(llvm::Expected<InputsAndPreamble>)> Action) {
+void TUScheduler::runWithPreamble(llvm::StringRef Name, PathRef File,
+ PreambleConsistency Consistency,
+ Callback<InputsAndPreamble> Action) {
auto It = Files.find(File);
if (It == Files.end()) {
Action(llvm::make_error<LSPError>(
@@ -893,19 +894,21 @@
}
std::shared_ptr<const ASTWorker> Worker = It->second->Worker.lock();
- auto Task = [Worker, this](std::string Name, std::string File,
- std::string Contents,
- tooling::CompileCommand Command, Context Ctx,
- decltype(ConsistentPreamble) ConsistentPreamble,
- decltype(Action) Action) mutable {
+ auto Task = [Worker, Consistency,
+ this](std::string Name, std::string File, std::string Contents,
+ tooling::CompileCommand Command, Context Ctx,
+ decltype(ConsistentPreamble) ConsistentPreamble,
+ decltype(Action) Action) mutable {
std::shared_ptr<const PreambleData> Preamble;
if (ConsistentPreamble.valid()) {
Preamble = ConsistentPreamble.get();
} else {
- // We don't want to be running preamble actions before the preamble was
- // built for the first time. This avoids extra work of processing the
- // preamble headers in parallel multiple times.
- Worker->waitForFirstPreamble();
+ if (Consistency != PreambleConsistency::StaleOrAbsent) {
+ // Wait until the preamble is built for the first time, if preamble is
+ // required. This avoids extra work of processing the preamble headers
+ // in parallel multiple times.
+ Worker->waitForFirstPreamble();
+ }
Preamble = Worker->getPossiblyStalePreamble();
}
Index: clangd/CodeComplete.h
===================================================================
--- clangd/CodeComplete.h
+++ clangd/CodeComplete.h
@@ -109,6 +109,12 @@
///
/// Such completions can insert scope qualifiers.
bool AllScopes = false;
+
+ /// Whether to allow falling back to code completion without compiling files
+ /// (using identifiers in the current file and symbol indexes), when file
+ /// cannot be built (e.g. missing compile command), or the build is not ready
+ /// (e.g. preamble is still being built).
+ bool AllowFallback = false;
};
// Semi-structured representation of a code-complete suggestion for our C++ API.
Index: clangd/ClangdServer.cpp
===================================================================
--- clangd/ClangdServer.cpp
+++ clangd/ClangdServer.cpp
@@ -11,7 +11,9 @@
#include "CodeComplete.h"
#include "FindSymbols.h"
#include "Headers.h"
+#include "Protocol.h"
#include "SourceCode.h"
+#include "TUScheduler.h"
#include "Trace.h"
#include "index/CanonicalIncludes.h"
#include "index/FileIndex.h"
@@ -21,6 +23,7 @@
#include "clang/Frontend/CompilerInstance.h"
#include "clang/Frontend/CompilerInvocation.h"
#include "clang/Lex/Preprocessor.h"
+#include "clang/Sema/CodeCompleteConsumer.h"
#include "clang/Tooling/CompilationDatabase.h"
#include "clang/Tooling/Core/Replacement.h"
#include "clang/Tooling/Refactoring/RefactoringResultConsumer.h"
@@ -152,6 +155,7 @@
if (ClangTidyOptProvider)
Opts.ClangTidyOpts = ClangTidyOptProvider->getOptions(File);
Opts.SuggestMissingIncludes = SuggestMissingIncludes;
+
// FIXME: some build systems like Bazel will take time to preparing
// environment to build the file, it would be nice if we could emit a
// "PreparingBuild" status to inform users, it is non-trivial given the
@@ -183,6 +187,18 @@
return CB(IP.takeError());
if (isCancelled())
return CB(llvm::make_error<CancelledError>());
+ if (!IP->Preamble) {
+ vlog("File {0} is not ready for code completion. Enter fallback mode.",
+ File);
+ CodeCompleteResult CCR;
+ CCR.Context = CodeCompletionContext::CCC_Recovery;
+
+ // FIXME: perform simple completion e.g. using identifiers in the current
+ // file and symbols in the index.
+ // FIXME: let clients know that we've entered fallback mode.
+
+ return CB(std::move(CCR));
+ }
llvm::Optional<SpeculativeFuzzyFind> SpecFuzzyFind;
if (CodeCompleteOpts.Index && CodeCompleteOpts.SpeculativeIndexRequest) {
@@ -214,7 +230,9 @@
};
// We use a potentially-stale preamble because latency is critical here.
- WorkScheduler.runWithPreamble("CodeComplete", File, TUScheduler::Stale,
+ WorkScheduler.runWithPreamble("CodeComplete", File,
+ Opts.AllowFallback ? TUScheduler::StaleOrAbsent
+ : TUScheduler::Stale,
Bind(Task, File.str(), std::move(CB)));
}
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
