ioeric updated this revision to Diff 194132.
ioeric added a comment.
- Fix unit test
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/Threading.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.AllowFallbackMode = 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.AllowFallbackMode = AllowFallbackCompletion;
RealFileSystemProvider FSProvider;
// Initialize and run ClangdLSPServer.
Index: clangd/Threading.h
===================================================================
--- clangd/Threading.h
+++ clangd/Threading.h
@@ -27,6 +27,7 @@
public:
// Sets the flag. No-op if already set.
void notify();
+ bool notified() const { return Notified; }
// Blocks until flag is set.
void wait() const;
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,13 +181,19 @@
/// 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 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).
+ /// If there's no preamble yet (because the file was just opened), we'll
+ /// either run \p Action without preamble (if \p Consistency is
+ /// `StaleOrAbsent`) or wait for it to build. The result may be null, if it
+ /// fails to build, or empty if there's no include. 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).
void runWithPreamble(llvm::StringRef Name, PathRef File,
PreambleConsistency Consistency,
Callback<InputsAndPreamble> Action);
Index: clangd/TUScheduler.cpp
===================================================================
--- clangd/TUScheduler.cpp
+++ clangd/TUScheduler.cpp
@@ -47,6 +47,9 @@
#include "Trace.h"
#include "index/CanonicalIncludes.h"
#include "clang/Frontend/CompilerInvocation.h"
+#include "clang/Frontend/PCHContainerOperations.h"
+#include "clang/Tooling/CompilationDatabase.h"
+#include "llvm/ADT/Optional.h"
#include "llvm/ADT/ScopeExit.h"
#include "llvm/Support/Errc.h"
#include "llvm/Support/Path.h"
@@ -179,6 +182,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(
@@ -188,6 +192,7 @@
/// return after an unsuccessful build of the preamble too, i.e. result of
/// getPossiblyStalePreamble() can be null even after this function returns.
void waitForFirstPreamble() const;
+ bool isFirstPreambleBuilt() const;
std::size_t getUsedBytes() const;
bool isASTCached() const;
@@ -242,7 +247,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) */
@@ -534,6 +538,10 @@
void ASTWorker::waitForFirstPreamble() const { PreambleWasBuilt.wait(); }
+bool ASTWorker::isFirstPreambleBuilt() const {
+ return PreambleWasBuilt.notified();
+}
+
std::size_t ASTWorker::getUsedBytes() const {
// Note that we don't report the size of ASTs currently used for processing
// the in-flight requests. We used this information for debugging purposes
@@ -858,9 +866,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>(
@@ -868,6 +876,32 @@
ErrorCode::InvalidParams));
return;
}
+ // Enter fallback mode if preamble is not ready. We only do this in
+ // asynchronous mode, as TU update should finish before this is run.
+ if (!It->second->Worker->isFirstPreambleBuilt() &&
+ Consistency == StaleOrAbsent && PreambleTasks) {
+ std::shared_ptr<const ASTWorker> Worker = It->second->Worker.lock();
+ auto FallbackTask = [Worker, this](std::string Name, std::string File,
+ std::string Contents,
+ tooling::CompileCommand Command,
+ Context Ctx,
+ decltype(Action) Action) mutable {
+ std::lock_guard<Semaphore> BarrierLock(Barrier);
+ WithContext Guard(std::move(Ctx));
+ trace::Span Tracer(Name + "(fallback)");
+ SPAN_ATTACH(Tracer, "file", File);
+ Action(InputsAndPreamble{Contents, Command,
+ /*Preamble=*/nullptr});
+ };
+
+ PreambleTasks->runAsync(
+ "task(fallback):" + llvm::sys::path::filename(File),
+ Bind(FallbackTask, std::string(Name), std::string(File),
+ It->second->Contents, It->second->Command,
+ Context::current().derive(kFileBeingProcessed, File),
+ std::move(Action)));
+ return;
+ }
if (!PreambleTasks) {
trace::Span Tracer(Name);
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 AllowFallbackMode = 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,8 +230,10 @@
};
// We use a potentially-stale preamble because latency is critical here.
- WorkScheduler.runWithPreamble("CodeComplete", File, TUScheduler::Stale,
- Bind(Task, File.str(), std::move(CB)));
+ WorkScheduler.runWithPreamble(
+ "CodeComplete", File,
+ Opts.AllowFallbackMode ? TUScheduler::StaleOrAbsent : TUScheduler::Stale,
+ Bind(Task, File.str(), std::move(CB)));
}
void ClangdServer::signatureHelp(PathRef File, Position Pos,
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
