[clangd] Turn no-parse-completion on by when preamble isn't ready. Add flag to force it.

[sammccall]

code-completion-parse -> completion-parse

[kadircet]

clangd/CodeComplete.h

128 ↗

(On Diff #200249)

Do we really have any real use case for this option apart from testing ?

clangd/tool/ClangdMain.cpp

268 ↗

(On Diff #200249)

Maybe CodeCompletionOptions().RunParser to keep the defaults at one place

[sammccall]

deduplicate default

[sammccall]

clangd/CodeComplete.h

128 ↗

(On Diff #200249)

I don't think so. (If testing includes manual testing/quality tuning, rather than just automated tests)

clang-tools-extra/trunk/clangd/ClangdServer.cpp

auto Task = [Pos, FS, CodeCompleteOpts,
     }
     // SpecFuzzyFind is only destroyed after speculative fuzzy find finishes.
     // We don't want `codeComplete` to wait for the async call if it doesn't use
     // the result (e.g. non-index completion, speculation fails), so that `CB`
     // is called as soon as results are available.
   };
 
   // We use a potentially-stale preamble because latency is critical here.
-  WorkScheduler.runWithPreamble("CodeComplete", File,
-                                Opts.AllowFallback ? TUScheduler::StaleOrAbsent
-                                                   : TUScheduler::Stale,
-                                Bind(Task, File.str(), std::move(CB)));
+  WorkScheduler.runWithPreamble(
+      "CodeComplete", File,
+      (Opts.RunParser == CodeCompleteOptions::AlwaysParse)
+          ? TUScheduler::Stale
+          : TUScheduler::StaleOrAbsent,
 }
 
 void ClangdServer::signatureHelp(PathRef File, Position Pos,
                                  Callback<SignatureHelp> CB) {
 
   auto FS = FSProvider.getFileSystem();
   auto *Index = this->Index;
   auto Action = [Pos, FS, Index](Path File, Callback<SignatureHelp> CB,

clang-tools-extra/trunk/clangd/CodeComplete.h

struct CodeCompleteOptions {
 
   /// Whether to include index symbols that are not defined in the scopes
   /// visible from the code completion point. This applies in contexts without
   /// explicit scope qualifiers.
   ///
   /// 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;
+  /// Whether to use the clang parser, or fallback to text-based completion
+  /// (using identifiers in the current file and symbol indexes).
+  enum CodeCompletionParse {
+    /// Block until we can run the parser (e.g. preamble is built).
+    /// Return an error if this fails.
+    AlwaysParse,
+    /// Run the parser if inputs (preamble) are ready.
+    /// Otherwise, use text-based completion.
+    ParseIfReady,
+    /// Always use text-based completion.
+    NeverParse,
+  } RunParser = ParseIfReady;
 };
 
 // Semi-structured representation of a code-complete suggestion for our C++ API.
 // We don't use the LSP structures here (unlike most features) as we want
 // to expose more data to allow for more precise testing and evaluation.
 struct CodeCompletion {
   // The unqualified name of the symbol or other completion item.
   std::string Name;

clang-tools-extra/trunk/clangd/CodeComplete.cpp

codeComplete(PathRef FileName, const tooling::CompileCommand &Command,
   auto Offset = positionToOffset(Contents, Pos);
   if (!Offset) {
     elog("Code completion position was invalid {0}", Offset.takeError());
     return CodeCompleteResult();
   }
   auto Flow = CodeCompleteFlow(
       FileName, Preamble ? Preamble->Includes : IncludeStructure(),
       SpecFuzzyFind, Opts);
-  return Preamble ? std::move(Flow).run(
-                        {FileName, Command, Preamble, Contents, *Offset, VFS})
-                  : std::move(Flow).runWithoutSema(Contents, *Offset, VFS);
+  return (!Preamble || Opts.RunParser == CodeCompleteOptions::NeverParse)
+             ? std::move(Flow).runWithoutSema(Contents, *Offset, VFS)
+             : std::move(Flow).run(
+                   {FileName, Command, Preamble, Contents, *Offset, VFS});
 }
 
 SignatureHelp signatureHelp(PathRef FileName,
                             const tooling::CompileCommand &Command,
                             const PreambleData *Preamble,
                             llvm::StringRef Contents, Position Pos,
                             llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS,
                             const SymbolIndex *Index) {

clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp

static llvm::cl::opt<OffsetEncoding> ForceOffsetEncoding(
     llvm::cl::desc("Force the offsetEncoding used for character positions. "
                    "This bypasses negotiation via client capabilities."),
     llvm::cl::values(clEnumValN(OffsetEncoding::UTF8, "utf-8",
                                 "Offsets are in UTF-8 bytes"),
                      clEnumValN(OffsetEncoding::UTF16, "utf-16",
                                 "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));
+static llvm::cl::opt<CodeCompleteOptions::CodeCompletionParse>
+    CodeCompletionParse(
+        "completion-parse",
+        llvm::cl::desc("Whether the clang-parser is used for code-completion"),
+        llvm::cl::values(clEnumValN(CodeCompleteOptions::AlwaysParse, "always",
+                                    "Block until the parser can be used"),
+                         clEnumValN(CodeCompleteOptions::ParseIfReady, "auto",
+                                    "Use text-based completion if the parser "
+                                    "is not ready"),
+                         clEnumValN(CodeCompleteOptions::NeverParse, "never",
+                                    "Always used text-based completion")),
+        llvm::cl::init(CodeCompleteOptions().RunParser), llvm::cl::Hidden);
 
 namespace {
 
 /// \brief Supports a test URI scheme with relaxed constraints for lit tests.
 /// The path in a test URI will be combined with a platform-specific fake
 /// directory to form an absolute path. For example, test:///a.cpp is resolved
 /// C:\clangd-test\a.cpp on Windows and /clangd-test/a.cpp on Unix.
 class TestScheme : public URIScheme {
int main(int argc, char *argv[]) {
   CCOpts.InsertIncludes = HeaderInsertion;
   if (!HeaderInsertionDecorators) {
     CCOpts.IncludeIndicator.Insert.clear();
     CCOpts.IncludeIndicator.NoInsert.clear();
   }
   CCOpts.SpeculativeIndexRequest = Opts.StaticIndex;
   CCOpts.EnableFunctionArgSnippets = EnableFunctionArgSnippets;
   CCOpts.AllScopes = AllScopesCompletion;
-  CCOpts.AllowFallback = AllowFallbackCompletion;
+  CCOpts.RunParser = CodeCompletionParse;
 
   RealFileSystemProvider FSProvider;
   // Initialize and run ClangdLSPServer.
   // Change stdin to binary to not lose \r\n on windows.
   llvm::sys::ChangeStdinToBinary();
 
   std::unique_ptr<Transport> TransportLayer;
   if (getenv("CLANGD_AS_XPC_SERVICE")) {

clang-tools-extra/trunk/clangd/unittests/ClangdTests.cpp

 //===-- ClangdTests.cpp - Clangd unit tests ---------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "Annotations.h"
 #include "ClangdLSPServer.h"
 #include "ClangdServer.h"
+#include "CodeComplete.h"
 #include "GlobalCompilationDatabase.h"
 #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"
Annotations Code(R"cpp(
     namespace ns { int xyz; }
     using namespace ns;
     int main() {
        xy^
     })cpp");
   FS.Files[FooCpp] = FooCpp;
 
   auto Opts = clangd::CodeCompleteOptions();
-  Opts.AllowFallback = true;
+  Opts.RunParser = CodeCompleteOptions::ParseIfReady;
 
   // 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_EQ(Res.Context, CodeCompletionContext::CCC_Recovery);
   // Identifier-based fallback completion doesn't know about "symbol" scope.
   EXPECT_THAT(Res.Completions,
               ElementsAre(AllOf(Field(&CodeCompletion::Name, "xyz"),
                                 Field(&CodeCompletion::Scope, ""))));
 
   // 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(),
-                                       clangd::CodeCompleteOptions()))
-                  .Completions,
-              ElementsAre(AllOf(Field(&CodeCompletion::Name, "xyz"),
-                                Field(&CodeCompletion::Scope, "ns::"))));
+  EXPECT_THAT(
+      cantFail(runCodeComplete(Server, FooCpp, Code.point(), Opts)).Completions,
+
+  // Now force identifier-based completion.
+  Opts.RunParser = CodeCompleteOptions::NeverParse;
+  EXPECT_THAT(
+      cantFail(runCodeComplete(Server, FooCpp, Code.point(), Opts)).Completions,
+      ElementsAre(AllOf(Field(&CodeCompletion::Name, "xyz"),
+                        Field(&CodeCompletion::Scope, ""))));
 }
 
 TEST_F(ClangdVFSTest, FallbackWhenWaitingForCompileCommand) {
   MockFSProvider FS;
   ErrorCheckingDiagConsumer DiagConsumer;
   // Returns compile command only when notified.
   class DelayedCompilationDatabase : public GlobalCompilationDatabase {
   public:
Annotations Code(R"cpp(
     })cpp");
   FS.Files[FooCpp] = FooCpp;
   Server.addDocument(FooCpp, Code.code());
 
   // Sleep for some time to make sure code completion is not run because update
   // hasn't been scheduled.
   std::this_thread::sleep_for(std::chrono::milliseconds(10));
   auto Opts = clangd::CodeCompleteOptions();
-  Opts.AllowFallback = true;
+  Opts.RunParser = CodeCompleteOptions::ParseIfReady;
 
   auto Res = cantFail(runCodeComplete(Server, FooCpp, Code.point(), Opts));
   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(AllOf(Field(&CodeCompletion::Name, "xyz"),
                                 Field(&CodeCompletion::Scope, "ns::"))));
 }
 
 } // namespace
 } // namespace clangd
 } // namespace clang