[clang-tools-extra] r357916 - [clangd] Add fallback mode for code completion when compile command or preamble is not ready.

Eric Liu via cfe-commits cfe-commits at lists.llvm.org
Mon Apr 8 07:53:17 PDT 2019


Author: ioeric
Date: Mon Apr  8 07:53:16 2019
New Revision: 357916

URL: http://llvm.org/viewvc/llvm-project?rev=357916&view=rev
Log:
[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

Modified:
    clang-tools-extra/trunk/clangd/ClangdServer.cpp
    clang-tools-extra/trunk/clangd/CodeComplete.h
    clang-tools-extra/trunk/clangd/TUScheduler.cpp
    clang-tools-extra/trunk/clangd/TUScheduler.h
    clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
    clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp

Modified: clang-tools-extra/trunk/clangd/ClangdServer.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.cpp?rev=357916&r1=357915&r2=357916&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/ClangdServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdServer.cpp Mon Apr  8 07:53:16 2019
@@ -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 @@ void ClangdServer::addDocument(PathRef F
   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 @@ void ClangdServer::codeComplete(PathRef
       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 @@ void ClangdServer::codeComplete(PathRef
   };
 
   // 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)));
 }
 

Modified: clang-tools-extra/trunk/clangd/CodeComplete.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CodeComplete.h?rev=357916&r1=357915&r2=357916&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/CodeComplete.h (original)
+++ clang-tools-extra/trunk/clangd/CodeComplete.h Mon Apr  8 07:53:16 2019
@@ -109,6 +109,12 @@ struct CodeCompleteOptions {
   ///
   /// 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.

Modified: clang-tools-extra/trunk/clangd/TUScheduler.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/TUScheduler.cpp?rev=357916&r1=357915&r2=357916&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/TUScheduler.cpp (original)
+++ clang-tools-extra/trunk/clangd/TUScheduler.cpp Mon Apr  8 07:53:16 2019
@@ -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 @@ public:
   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 @@ private:
   /// 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 @@ void TUScheduler::runWithAST(
   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 @@ void TUScheduler::runWithPreamble(
   }
 
   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();
     }
 

Modified: clang-tools-extra/trunk/clangd/TUScheduler.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/TUScheduler.h?rev=357916&r1=357915&r2=357916&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/TUScheduler.h (original)
+++ clang-tools-extra/trunk/clangd/TUScheduler.h Mon Apr  8 07:53:16 2019
@@ -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 InputsAndAST {
 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 @@ public:
     ///   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).

Modified: clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp?rev=357916&r1=357915&r2=357916&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp (original)
+++ clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp Mon Apr  8 07:53:16 2019
@@ -231,6 +231,14 @@ static llvm::cl::opt<OffsetEncoding> For
                                 "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 @@ int main(int argc, char *argv[]) {
   CCOpts.SpeculativeIndexRequest = Opts.StaticIndex;
   CCOpts.EnableFunctionArgSnippets = EnableFunctionArgSnippets;
   CCOpts.AllScopes = AllScopesCompletion;
+  CCOpts.AllowFallback = AllowFallbackCompletion;
 
   RealFileSystemProvider FSProvider;
   // Initialize and run ClangdLSPServer.

Modified: clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp?rev=357916&r1=357915&r2=357916&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp Mon Apr  8 07:53:16 2019
@@ -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 clangd {
 namespace {
 
 using ::testing::ElementsAre;
-using ::testing::Eq;
 using ::testing::Field;
 using ::testing::Gt;
 using ::testing::IsEmpty;
@@ -1058,6 +1059,41 @@ TEST_F(ClangdVFSTest, FlagsWithPlugins)
   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




More information about the cfe-commits mailing list