This revision was automatically updated to reflect the committed changes.
Closed by commit rL357916: [clangd] Add fallback mode for code completion when 
compile command or preamble… (authored by ioeric, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59811/new/

https://reviews.llvm.org/D59811

Files:
  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

Index: clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp
===================================================================
--- clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp
+++ clang-tools-extra/trunk/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: clang-tools-extra/trunk/clangd/ClangdServer.cpp
===================================================================
--- clang-tools-extra/trunk/clangd/ClangdServer.cpp
+++ clang-tools-extra/trunk/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)));
 }
 
Index: clang-tools-extra/trunk/clangd/TUScheduler.cpp
===================================================================
--- clang-tools-extra/trunk/clangd/TUScheduler.cpp
+++ clang-tools-extra/trunk/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: clang-tools-extra/trunk/clangd/CodeComplete.h
===================================================================
--- clang-tools-extra/trunk/clangd/CodeComplete.h
+++ clang-tools-extra/trunk/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: clang-tools-extra/trunk/clangd/TUScheduler.h
===================================================================
--- clang-tools-extra/trunk/clangd/TUScheduler.h
+++ clang-tools-extra/trunk/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: clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
===================================================================
--- clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/trunk/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.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to