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" @@ -153,17 +156,21 @@ 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 // current implementation. - ParseInputs Inputs; - Inputs.CompileCommand = getCompileCommand(File); - Inputs.FS = FSProvider.getFileSystem(); - Inputs.Contents = Contents; - Inputs.Opts = std::move(Opts); - Inputs.Index = Index; - WorkScheduler.update(File, Inputs, WantDiags); + FileUpdateInputs UpdateInputs; + std::string FilePath = File; + UpdateInputs.GetCompileCommand = [this, FilePath]() { + return getCompileCommand(FilePath); + }; + UpdateInputs.InputSansCommand.FS = FSProvider.getFileSystem(); + UpdateInputs.InputSansCommand.Contents = Contents; + UpdateInputs.InputSansCommand.Opts = std::move(Opts); + UpdateInputs.InputSansCommand.Index = Index; + WorkScheduler.update(File, UpdateInputs, WantDiags); } void ClangdServer::removeDocument(PathRef File) { WorkScheduler.remove(File); } @@ -187,7 +194,21 @@ return CB(IP.takeError()); if (isCancelled()) return CB(llvm::make_error()); + 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)); + } + assert(IP->Command.hasValue() && + "Compile command must exist if preamble has been built"); llvm::Optional SpecFuzzyFind; if (CodeCompleteOpts.Index && CodeCompleteOpts.SpeculativeIndexRequest) { SpecFuzzyFind.emplace(); @@ -200,7 +221,7 @@ // FIXME(ibiryukov): even if Preamble is non-null, we may want to check // both the old and the new version in case only one of them matches. CodeCompleteResult Result = clangd::codeComplete( - File, IP->Command, IP->Preamble, IP->Contents, Pos, FS, PCHs, + File, *IP->Command, IP->Preamble, IP->Contents, Pos, FS, PCHs, CodeCompleteOpts, SpecFuzzyFind ? SpecFuzzyFind.getPointer() : nullptr); { clang::clangd::trace::Span Tracer("Completion results callback"); @@ -219,7 +240,8 @@ // We use a potentially-stale preamble because latency is critical here. WorkScheduler.runWithPreamble("CodeComplete", File, TUScheduler::Stale, - Bind(Task, File.str(), std::move(CB))); + Bind(Task, File.str(), std::move(CB)), + Opts.AllowFallbackMode); } void ClangdServer::signatureHelp(PathRef File, Position Pos, @@ -233,9 +255,9 @@ if (!IP) return CB(IP.takeError()); - auto PreambleData = IP->Preamble; - CB(clangd::signatureHelp(File, IP->Command, PreambleData, IP->Contents, Pos, - FS, PCHs, Index)); + assert(IP->Command.hasValue()); + CB(clangd::signatureHelp(File, *IP->Command, IP->Preamble, IP->Contents, + Pos, FS, PCHs, Index)); }; // Unlike code completion, we wait for an up-to-date preamble here. Index: clangd/CodeComplete.h =================================================================== --- clangd/CodeComplete.h +++ clangd/CodeComplete.h @@ -110,6 +110,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/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 namespace clang { @@ -24,6 +26,13 @@ /// synchronously). unsigned getDefaultAsyncThreadsCount(); +// Inputs for updating a file in TUScheduler. +struct FileUpdateInputs { + std::function GetCompileCommand = nullptr; + // Contains all other inputs except for compile command. + ParseInputs InputSansCommand; +}; + struct InputsAndAST { const ParseInputs &Inputs; ParsedAST * @@ -31,7 +40,9 @@ struct InputsAndPreamble { llvm::StringRef Contents; - const tooling::CompileCommand &Command; + // Can be None in fallback mode when neither Command nor Preamble is ready. + llvm::Optional Command; + // In fallback mode, this can be nullptr when preamble is not ready. const PreambleData *Preamble; }; @@ -142,7 +153,7 @@ /// If diagnostics are requested (Yes), and the context is cancelled before /// they are prepared, they may be skipped if eventual-consistency permits it /// (i.e. WantDiagnostics is downgraded to Auto). - void update(PathRef File, ParseInputs Inputs, WantDiagnostics WD); + void update(PathRef File, FileUpdateInputs Inputs, WantDiagnostics WD); /// Remove \p File from the list of tracked files and schedule removal of its /// resources. Pending diagnostics for closed files may not be delivered, even @@ -179,15 +190,18 @@ /// This is the fastest option, usually a preamble is available immediately. Stale, }; + /// 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 (when \p AllowFallback is true) or + /// 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). void runWithPreamble(llvm::StringRef Name, PathRef File, PreambleConsistency Consistency, - Callback Action); + Callback Action, + bool AllowFallback = false); /// Wait until there are no scheduled or running tasks. /// Mostly useful for synchronizing tests. Index: clangd/TUScheduler.cpp =================================================================== --- clangd/TUScheduler.cpp +++ clangd/TUScheduler.cpp @@ -48,6 +48,8 @@ #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" @@ -175,15 +177,20 @@ ParsingCallbacks &Callbacks); ~ASTWorker(); - void update(ParseInputs Inputs, WantDiagnostics); + void update(FileUpdateInputs Inputs, WantDiagnostics); void runWithAST(llvm::StringRef Name, llvm::unique_function)> Action); bool blockUntilIdle(Deadline Timeout) const; std::shared_ptr getPossiblyStalePreamble() const; + + llvm::Optional getCompileCommand() const; + /// Obtain a preamble reflecting all updates so far. Threadsafe. /// It may be delivered immediately, or later on the worker thread. + /// This might not be built from the latest compile command, if preamble build + /// with the latest compile command has not finished. void getCurrentPreamble( llvm::unique_function)>); /// Wait for the first build of preamble to finish. Preamble itself can be @@ -191,6 +198,9 @@ /// 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; + void waitForFirstCompileCommand() const; + bool isFirstCompileCommandSet() const; std::size_t getUsedBytes() const; bool isASTCached() const; @@ -247,12 +257,15 @@ /// 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 LastBuiltPreamble; /* GUARDED_BY(Mutex) */ /// Becomes ready when the first preamble build finishes. Notification PreambleWasBuilt; + llvm::Optional + LastCompileCommand; /* GUARDED_BY(Mutex) */ + /// Becomes ready when the first compile command is set. + Notification CompileCommandWasSet; /// Set to true to signal run() to finish processing. bool Done; /* GUARDED_BY(Mutex) */ std::deque Requests; /* GUARDED_BY(Mutex) */ @@ -349,9 +362,18 @@ #endif } -void ASTWorker::update(ParseInputs Inputs, WantDiagnostics WantDiags) { +void ASTWorker::update(FileUpdateInputs UpdateInputs, + WantDiagnostics WantDiags) { llvm::StringRef TaskName = "Update"; auto Task = [=]() mutable { + auto Inputs = std::move(UpdateInputs.InputSansCommand); + Inputs.CompileCommand = UpdateInputs.GetCompileCommand(); + { + std::lock_guard Lock(Mutex); + this->LastCompileCommand = Inputs.CompileCommand; + } + CompileCommandWasSet.notify(); + // Will be used to check if we can avoid rebuilding the AST. bool InputsAreTheSame = std::tie(FileInputs.CompileCommand, FileInputs.Contents) == @@ -512,6 +534,11 @@ return LastBuiltPreamble; } +llvm::Optional ASTWorker::getCompileCommand() const { + std::lock_guard Lock(Mutex); + return LastCompileCommand; +} + void ASTWorker::getCurrentPreamble( llvm::unique_function)> Callback) { // We could just call startTask() to throw the read on the queue, knowing @@ -542,6 +569,18 @@ void ASTWorker::waitForFirstPreamble() const { PreambleWasBuilt.wait(); } +bool ASTWorker::isFirstPreambleBuilt() const { + return PreambleWasBuilt.notified(); +} + +void ASTWorker::waitForFirstCompileCommand() const { + CompileCommandWasSet.wait(); +} + +bool ASTWorker::isFirstCompileCommandSet() const { + return CompileCommandWasSet.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 @@ -780,7 +819,6 @@ struct TUScheduler::FileData { /// Latest inputs, passed to TUScheduler::update(). std::string Contents; - tooling::CompileCommand Command; ASTWorkerHandle Worker; }; @@ -823,7 +861,7 @@ return true; } -void TUScheduler::update(PathRef File, ParseInputs Inputs, +void TUScheduler::update(PathRef File, FileUpdateInputs UpdateInputs, WantDiagnostics WantDiags) { std::unique_ptr &FD = Files[File]; if (!FD) { @@ -832,12 +870,11 @@ File, *IdleASTs, WorkerThreads ? WorkerThreads.getPointer() : nullptr, Barrier, UpdateDebounce, PCHOps, StorePreamblesInMemory, *Callbacks); FD = std::unique_ptr(new FileData{ - Inputs.Contents, Inputs.CompileCommand, std::move(Worker)}); + UpdateInputs.InputSansCommand.Contents, std::move(Worker)}); } else { - FD->Contents = Inputs.Contents; - FD->Command = Inputs.CompileCommand; + FD->Contents = UpdateInputs.InputSansCommand.Contents; } - FD->Worker->update(std::move(Inputs), WantDiags); + FD->Worker->update(std::move(UpdateInputs), WantDiags); } void TUScheduler::remove(PathRef File) { @@ -867,9 +904,10 @@ It->second->Worker->runWithAST(Name, std::move(Action)); } -void TUScheduler::runWithPreamble( - llvm::StringRef Name, PathRef File, PreambleConsistency Consistency, - llvm::unique_function)> Action) { +void TUScheduler::runWithPreamble(llvm::StringRef Name, PathRef File, + PreambleConsistency Consistency, + Callback Action, + bool AllowFallback) { auto It = Files.find(File); if (It == Files.end()) { Action(llvm::make_error( @@ -877,13 +915,44 @@ 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() && AllowFallback && + PreambleTasks) { + std::shared_ptr Worker = It->second->Worker.lock(); + auto FallbackTask = [Worker, this](std::string Name, std::string File, + std::string Contents, Context Ctx, + decltype(Action) Action) mutable { + std::lock_guard BarrierLock(Barrier); + WithContext Guard(std::move(Ctx)); + trace::Span Tracer(Name + "(fallback)"); + SPAN_ATTACH(Tracer, "file", File); + Action(InputsAndPreamble{Contents, Worker->getCompileCommand(), + /*Preamble=*/nullptr}); + }; + + PreambleTasks->runAsync( + "task(fallback):" + llvm::sys::path::filename(File), + Bind(FallbackTask, std::string(Name), std::string(File), + It->second->Contents, + Context::current().derive(kFileBeingProcessed, File), + std::move(Action))); + return; + } if (!PreambleTasks) { + auto CompileCommand = It->second->Worker->getCompileCommand(); + if (!CompileCommand) { + Action(llvm::make_error( + "trying to get preamble for document without compile command", + ErrorCode::InvalidParams)); + return; + } trace::Span Tracer(Name); SPAN_ATTACH(Tracer, "file", File); std::shared_ptr Preamble = It->second->Worker->getPossiblyStalePreamble(); - Action(InputsAndPreamble{It->second->Contents, It->second->Command, + Action(InputsAndPreamble{It->second->Contents, CompileCommand, Preamble.get()}); return; } @@ -903,10 +972,13 @@ std::shared_ptr Worker = It->second->Worker.lock(); auto Task = [Worker, this](std::string Name, std::string File, - std::string Contents, - tooling::CompileCommand Command, Context Ctx, + std::string Contents, Context Ctx, decltype(ConsistentPreamble) ConsistentPreamble, decltype(Action) Action) mutable { + if (!Worker->isFirstCompileCommandSet()) + Worker->waitForFirstCompileCommand(); + auto CompileCommand = Worker->getCompileCommand(); + assert(CompileCommand.hasValue()); std::shared_ptr Preamble; if (ConsistentPreamble.valid()) { Preamble = ConsistentPreamble.get(); @@ -922,13 +994,12 @@ WithContext Guard(std::move(Ctx)); trace::Span Tracer(Name); SPAN_ATTACH(Tracer, "file", File); - Action(InputsAndPreamble{Contents, Command, Preamble.get()}); + Action(InputsAndPreamble{Contents, CompileCommand, Preamble.get()}); }; PreambleTasks->runAsync( "task:" + llvm::sys::path::filename(File), Bind(Task, std::string(Name), std::string(File), It->second->Contents, - It->second->Command, Context::current().derive(kFileBeingProcessed, File), std::move(ConsistentPreamble), std::move(Action))); } 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/tool/ClangdMain.cpp =================================================================== --- clangd/tool/ClangdMain.cpp +++ clangd/tool/ClangdMain.cpp @@ -219,6 +219,14 @@ "includes using index."), llvm::cl::init(true)); +static llvm::cl::opt 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. @@ -425,6 +433,7 @@ CCOpts.SpeculativeIndexRequest = Opts.StaticIndex; CCOpts.EnableFunctionArgSnippets = EnableFunctionArgSnippets; CCOpts.AllScopes = AllScopesCompletion; + CCOpts.AllowFallbackMode = AllowFallbackCompletion; RealFileSystemProvider FSProvider; // Initialize and run ClangdLSPServer. 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,55 @@ EXPECT_NE(Result, ""); } +TEST_F(ClangdVFSTest, FallbackWhenPreambleIsNotReady) { + MockFSProvider FS; + ErrorCheckingDiagConsumer DiagConsumer; + // Returns compile command only when notified. + class DelayedCompilationDatabase : public GlobalCompilationDatabase { + public: + DelayedCompilationDatabase(Notification &CanReturnCommand) + : CanReturnCommand(CanReturnCommand) {} + + llvm::Optional + getCompileCommand(PathRef File, ProjectInfo * = nullptr) const override { + CanReturnCommand.wait(); + auto FileName = llvm::sys::path::filename(File); + std::vector CommandLine = {"clangd", "-ffreestanding", File}; + return {tooling::CompileCommand(llvm::sys::path::parent_path(File), + FileName, std::move(CommandLine), "")}; + } + + std::vector ExtraClangFlags; + + private: + Notification &CanReturnCommand; + }; + + Notification CanReturnCommand; + DelayedCompilationDatabase CDB(CanReturnCommand); + 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; + Server.addDocument(FooCpp, Code.code()); + auto Opts = clangd::CodeCompleteOptions(); + Opts.AllowFallbackMode = true; + auto Res = cantFail(runCodeComplete(Server, FooCpp, Code.point(), Opts)); + EXPECT_THAT(Res.Completions, IsEmpty()); + EXPECT_EQ(Res.Context, CodeCompletionContext::CCC_Recovery); + CanReturnCommand.notify(); + ASSERT_TRUE(Server.blockUntilIdleForTest()); + EXPECT_THAT(cantFail(runCodeComplete(Server, FooCpp, Code.point(), + clangd::CodeCompleteOptions())) + .Completions, + ElementsAre(Field(&CodeCompletion::Name, "xyz"))); +} + } // namespace } // namespace clangd } // namespace clang Index: unittests/clangd/TUSchedulerTests.cpp =================================================================== --- unittests/clangd/TUSchedulerTests.cpp +++ unittests/clangd/TUSchedulerTests.cpp @@ -15,14 +15,13 @@ #include "gmock/gmock.h" #include "gtest/gtest.h" #include +#include #include namespace clang { namespace clangd { namespace { -using ::testing::_; -using ::testing::AllOf; using ::testing::AnyOf; using ::testing::Each; using ::testing::ElementsAre; @@ -35,13 +34,18 @@ class TUSchedulerTests : public ::testing::Test { protected: - ParseInputs getInputs(PathRef File, std::string Contents) { + FileUpdateInputs getInputs(PathRef File, std::string Contents) { ParseInputs Inputs; - Inputs.CompileCommand = *CDB.getCompileCommand(File); Inputs.FS = buildTestFS(Files, Timestamps); Inputs.Contents = std::move(Contents); Inputs.Opts = ParseOptions(); - return Inputs; + FileUpdateInputs UpdateInputs; + UpdateInputs.InputSansCommand = std::move(Inputs); + std::string FilePath = File; + UpdateInputs.GetCompileCommand = [this, FilePath]() { + return *CDB.getCompileCommand(FilePath); + }; + return UpdateInputs; } void updateWithCallback(TUScheduler &S, PathRef File, @@ -72,7 +76,7 @@ /// Schedule an update and call \p CB with the diagnostics it produces, if /// any. The TUScheduler should be created with captureDiags as a /// DiagsCallback for this to work. - void updateWithDiags(TUScheduler &S, PathRef File, ParseInputs Inputs, + void updateWithDiags(TUScheduler &S, PathRef File, FileUpdateInputs Inputs, WantDiagnostics WD, llvm::unique_function)> CB) { Path OrigFile = File.str(); @@ -245,7 +249,7 @@ S.runWithPreamble("StaleRead", Path, TUScheduler::Stale, [&](Expected Pre) { - ASSERT_TRUE(bool(Pre)); + EXPECT_TRUE((bool)Pre); assert(bool(Pre)); EXPECT_THAT(includes(Pre->Preamble), ElementsAre("")); @@ -397,8 +401,9 @@ EXPECT_THAT(Context::current().get(NonceKey), Pointee(Nonce)); ASSERT_TRUE((bool)AST); - EXPECT_EQ(AST->Inputs.FS, Inputs.FS); - EXPECT_EQ(AST->Inputs.Contents, Inputs.Contents); + EXPECT_EQ(AST->Inputs.FS, Inputs.InputSansCommand.FS); + EXPECT_EQ(AST->Inputs.Contents, + Inputs.InputSansCommand.Contents); std::lock_guard Lock(Mut); ++TotalASTReads; @@ -415,7 +420,7 @@ EXPECT_THAT(Context::current().get(NonceKey), Pointee(Nonce)); ASSERT_TRUE((bool)Preamble); - EXPECT_EQ(Preamble->Contents, Inputs.Contents); + EXPECT_EQ(Preamble->Contents, Inputs.InputSansCommand.Contents); std::lock_guard Lock(Mut); ++TotalPreambleReads; @@ -504,6 +509,7 @@ )cpp"; auto WithEmptyPreamble = R"cpp(int main() {})cpp"; S.update(Foo, getInputs(Foo, WithPreamble), WantDiagnostics::Auto); + S.runWithPreamble( "getNonEmptyPreamble", Foo, TUScheduler::Stale, [&](Expected Preamble) {