[clangd] Run semaCodeComplete only with a preamble

[kadircet]

Summary

It is used by code completion and signature help. Code completion
already uses a special no-compile mode for missing preambles, so this change is
a no-op for that.

As for signature help, it already blocks for a preamble and missing it implies
clang has failed to parse the preamble and retrying it in signature help likely
will fail again. And even if it doesn't, request latency will be too high to be
useful as parsing preambles is expensive.

clang-tools-extra/clangd/ClangdServer.cpp

void ClangdServer::signatureHelp(PathRef File, Position Pos,
                                  Callback<SignatureHelp> CB) {
 
   auto Action = [Pos, FS = FSProvider.getFileSystem(), File = File.str(),
                  CB = std::move(CB),
                  this](llvm::Expected<InputsAndPreamble> IP) mutable {
     if (!IP)
       return CB(IP.takeError());
 
-    auto PreambleData = IP->Preamble;
-    CB(clangd::signatureHelp(File, IP->Command, PreambleData, IP->Contents, Pos,
-                             FS, Index));
+    const auto *PreambleData = IP->Preamble;
+    if (!PreambleData)
+      return CB(llvm::createStringError(llvm::inconvertibleErrorCode(),
+                                        "Failed to parse includes"));
+
+    CB(clangd::signatureHelp(File, IP->Command, *PreambleData, IP->Contents,
+                             Pos, FS, Index));
   };
 
   // Unlike code completion, we wait for an up-to-date preamble here.
   // Signature help is often triggered after code completion. If the code
   // completion inserted a header to make the symbol available, then using
   // the old preamble would yield useless results.
   WorkScheduler.runWithPreamble("SignatureHelp", File, TUScheduler::Consistent,
                                 std::move(Action));

clang-tools-extra/clangd/CodeComplete.h

CodeCompleteResult codeComplete(PathRef FileName,
                                 StringRef Contents, Position Pos,
                                 IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS,
                                 CodeCompleteOptions Opts,
                                 SpeculativeFuzzyFind *SpecFuzzyFind = nullptr);
 
 /// Get signature help at a specified \p Pos in \p FileName.
 SignatureHelp signatureHelp(PathRef FileName,
                             const tooling::CompileCommand &Command,
-                            const PreambleData *Preamble, StringRef Contents,
+                            const PreambleData &Preamble, StringRef Contents,
                             Position Pos,
                             IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS,
                             const SymbolIndex *Index);
 
 // For index-based completion, we only consider:
 //   * symbols in namespaces or translation unit scopes (e.g. no class
 //     members, no locals)
 //   * enum constants in unscoped enum decl (e.g. "red" in "enum {red};")

clang-tools-extra/clangd/CodeComplete.cpp

private:
   std::shared_ptr<clang::GlobalCodeCompletionAllocator> Allocator;
   CodeCompletionTUInfo CCTUInfo;
   const SymbolIndex *Index;
 }; // SignatureHelpCollector
 
 struct SemaCompleteInput {
   PathRef FileName;
   const tooling::CompileCommand &Command;
-  const PreambleData *Preamble;
+  const PreambleData &Preamble;
   llvm::StringRef Contents;
   size_t Offset;
   llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS;
 };
 
 void loadMainFilePreambleMacros(const Preprocessor &PP,
                                 const PreambleData &Preamble) {
   // The ExternalPreprocessorSource has our macros, if we know where to look.

 // Invokes Sema code completion on a file.
 // If \p Includes is set, it will be updated based on the compiler invocation.
 bool semaCodeComplete(std::unique_ptr<CodeCompleteConsumer> Consumer,
                       const clang::CodeCompleteOptions &Options,
                       const SemaCompleteInput &Input,
                       IncludeStructure *Includes = nullptr) {
   trace::Span Tracer("Sema completion");
   llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS = Input.VFS;
-  if (Input.Preamble && Input.Preamble->StatCache)
-    VFS = Input.Preamble->StatCache->getConsumingFS(std::move(VFS));
+  if (Input.Preamble.StatCache)
+    VFS = Input.Preamble.StatCache->getConsumingFS(std::move(VFS));
   ParseInputs ParseInput;
   ParseInput.CompileCommand = Input.Command;
   ParseInput.FS = VFS;
   ParseInput.Contents = std::string(Input.Contents);
   ParseInput.Opts = ParseOptions();
 
   IgnoreDiagnostics IgnoreDiags;
   auto CI = buildCompilerInvocation(ParseInput, IgnoreDiags);
bool semaCodeComplete(std::unique_ptr<CodeCompleteConsumer> Consumer,
   // overriding the preamble will break sema completion. Fortunately we can just
   // skip all includes in this case; these completions are really simple.
   PreambleBounds PreambleRegion =
       ComputePreambleBounds(*CI->getLangOpts(), ContentsBuffer.get(), 0);
   bool CompletingInPreamble = PreambleRegion.Size > Input.Offset;
   // NOTE: we must call BeginSourceFile after prepareCompilerInstance. Otherwise
   // the remapped buffers do not get freed.
   auto Clang = prepareCompilerInstance(
-      std::move(CI),
-      (Input.Preamble && !CompletingInPreamble) ? &Input.Preamble->Preamble
-                                                : nullptr,
+      std::move(CI), !CompletingInPreamble ? &Input.Preamble.Preamble : nullptr,
       std::move(ContentsBuffer), std::move(VFS), IgnoreDiags);
   Clang->getPreprocessorOpts().SingleFileParseMode = CompletingInPreamble;
   Clang->setCodeCompletionConsumer(Consumer.release());
 
   SyntaxOnlyAction Action;
   if (!Action.BeginSourceFile(*Clang, Clang->getFrontendOpts().Inputs[0])) {
     log("BeginSourceFile() failed when running codeComplete for {0}",
         Input.FileName);
     return false;
   }
   // Macros can be defined within the preamble region of the main file.
   // They don't fall nicely into our index/Sema dichotomy:
   //  - they're not indexed for completion (they're not available across files)
   //  - but Sema code complete won't see them: as part of the preamble, they're
   //    deserialized only when mentioned.
   // Force them to be deserialized so SemaCodeComplete sees them.
-  if (Input.Preamble)
-    loadMainFilePreambleMacros(Clang->getPreprocessor(), *Input.Preamble);
+  loadMainFilePreambleMacros(Clang->getPreprocessor(), Input.Preamble);
   if (Includes)
     Clang->getPreprocessor().addPPCallbacks(
         collectIncludeStructureCallback(Clang->getSourceManager(), Includes));
   if (llvm::Error Err = Action.Execute()) {
     log("Execute() failed when running codeComplete for {0}: {1}",
         Input.FileName, toString(std::move(Err)));
     return false;
   }
if (!Offset) {
     return CodeCompleteResult();
   }
   auto Flow = CodeCompleteFlow(
       FileName, Preamble ? Preamble->Includes : IncludeStructure(),
       SpecFuzzyFind, Opts);
   return (!Preamble || Opts.RunParser == CodeCompleteOptions::NeverParse)
              ? std::move(Flow).runWithoutSema(Contents, *Offset, VFS)
              : std::move(Flow).run(
-                   {FileName, Command, Preamble, Contents, *Offset, VFS});
+                   {FileName, Command, *Preamble, Contents, *Offset, VFS});
 }
 
 SignatureHelp signatureHelp(PathRef FileName,
                             const tooling::CompileCommand &Command,
-                            const PreambleData *Preamble,
+                            const PreambleData &Preamble,
                             llvm::StringRef Contents, Position Pos,
                             llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS,
                             const SymbolIndex *Index) {
   auto Offset = positionToOffset(Contents, Pos);
   if (!Offset) {
     elog("Code completion position was invalid {0}", Offset.takeError());
     return SignatureHelp();
   }

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

TEST_F(ClangdVFSTest, InvalidCompileCommand) {
   // Clang can't parse command args in that case, but we shouldn't crash.
   runAddDocument(Server, FooCpp, "int main() {}");
 
   EXPECT_EQ(runDumpAST(Server, FooCpp), "<no-ast>");
   EXPECT_ERROR(runLocateSymbolAt(Server, FooCpp, Position()));
   EXPECT_ERROR(runFindDocumentHighlights(Server, FooCpp, Position()));
   EXPECT_ERROR(runRename(Server, FooCpp, Position(), "new_name",
                          clangd::RenameOptions()));
+  EXPECT_ERROR(runSignatureHelp(Server, FooCpp, Position()));
   // Identifier-based fallback completion.
   EXPECT_THAT(cantFail(runCodeComplete(Server, FooCpp, Position(),
                                        clangd::CodeCompleteOptions()))
                   .Completions,
               ElementsAre(Field(&CodeCompletion::Name, "int"),
                           Field(&CodeCompletion::Name, "main")));
-  auto SigHelp = runSignatureHelp(Server, FooCpp, Position());
-  ASSERT_TRUE(bool(SigHelp)) << "signatureHelp returned an error";
-  EXPECT_THAT(SigHelp->signatures, IsEmpty());
 }
 
 class ClangdThreadingTest : public ClangdVFSTest {};
 
 TEST_F(ClangdThreadingTest, StressTest) {
   // Without 'static' clang gives an error for a usage inside TestDiagConsumer.
   static const unsigned FilesCount = 5;
   const unsigned RequestsCount = 500;

clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp

SignatureHelp signatures(llvm::StringRef Text, Position Point,
   auto CI = buildCompilerInvocation(Inputs, Diags);
   if (!CI) {
     ADD_FAILURE() << "Couldn't build CompilerInvocation";
     return {};
   }
   auto Preamble =
       buildPreamble(testPath(TU.Filename), *CI, /*OldPreamble=*/nullptr, Inputs,
                     /*InMemory=*/true, /*Callback=*/nullptr);
-  return signatureHelp(testPath(TU.Filename), Inputs.CompileCommand,
-                       Preamble.get(), Text, Point, Inputs.FS, Index.get());
+  if (!Preamble) {
+    ADD_FAILURE() << "Couldn't build Preamble";
+    return {};
+  }
+  return signatureHelp(testPath(TU.Filename), Inputs.CompileCommand, *Preamble,
+                       Text, Point, Inputs.FS, Index.get());
 }
 
 SignatureHelp signatures(llvm::StringRef Text,
                          std::vector<Symbol> IndexSymbols = {}) {
   Annotations Test(Text);
   return signatures(Test.code(), Test.point(), std::move(IndexSymbols));
 }