Author: Kadir Cetinkaya Date: 2020-04-01T13:02:47+02:00 New Revision: 43aa04eb7a617ee75dfcbbe2d395b8208e66c0e0
URL: https://github.com/llvm/llvm-project/commit/43aa04eb7a617ee75dfcbbe2d395b8208e66c0e0 DIFF: https://github.com/llvm/llvm-project/commit/43aa04eb7a617ee75dfcbbe2d395b8208e66c0e0.diff LOG: [clangd] Run semaCodeComplete only with a preamble 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. Reviewers: sammccall Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, usaxena95, cfe-commits Tags: #clang Differential Revision: https://reviews.llvm.org/D77204 Added: Modified: clang-tools-extra/clangd/ClangdServer.cpp clang-tools-extra/clangd/CodeComplete.cpp clang-tools-extra/clangd/CodeComplete.h clang-tools-extra/clangd/unittests/ClangdTests.cpp clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp Removed: ################################################################################ diff --git a/clang-tools-extra/clangd/ClangdServer.cpp b/clang-tools-extra/clangd/ClangdServer.cpp index 0882784f7031..c1773258554c 100644 --- a/clang-tools-extra/clangd/ClangdServer.cpp +++ b/clang-tools-extra/clangd/ClangdServer.cpp @@ -271,9 +271,13 @@ void ClangdServer::signatureHelp(PathRef File, Position Pos, 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. diff --git a/clang-tools-extra/clangd/CodeComplete.cpp b/clang-tools-extra/clangd/CodeComplete.cpp index 86ea5f26d397..b544510ecea1 100644 --- a/clang-tools-extra/clangd/CodeComplete.cpp +++ b/clang-tools-extra/clangd/CodeComplete.cpp @@ -1022,7 +1022,7 @@ class SignatureHelpCollector final : public CodeCompleteConsumer { 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; @@ -1054,8 +1054,8 @@ bool semaCodeComplete(std::unique_ptr<CodeCompleteConsumer> Consumer, 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; @@ -1099,9 +1099,7 @@ bool semaCodeComplete(std::unique_ptr<CodeCompleteConsumer> Consumer, // 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()); @@ -1118,8 +1116,7 @@ bool semaCodeComplete(std::unique_ptr<CodeCompleteConsumer> Consumer, // - 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)); @@ -1758,12 +1755,12 @@ codeComplete(PathRef FileName, const tooling::CompileCommand &Command, 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) { diff --git a/clang-tools-extra/clangd/CodeComplete.h b/clang-tools-extra/clangd/CodeComplete.h index df06c156049f..3adea47c89a1 100644 --- a/clang-tools-extra/clangd/CodeComplete.h +++ b/clang-tools-extra/clangd/CodeComplete.h @@ -276,7 +276,7 @@ CodeCompleteResult codeComplete(PathRef FileName, /// 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); diff --git a/clang-tools-extra/clangd/unittests/ClangdTests.cpp b/clang-tools-extra/clangd/unittests/ClangdTests.cpp index 1e5fcf3d97e1..d15eba80ae29 100644 --- a/clang-tools-extra/clangd/unittests/ClangdTests.cpp +++ b/clang-tools-extra/clangd/unittests/ClangdTests.cpp @@ -552,15 +552,13 @@ TEST_F(ClangdVFSTest, InvalidCompileCommand) { 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 {}; diff --git a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp index 3cb18f6893e9..1084b1550579 100644 --- a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp +++ b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp @@ -1049,8 +1049,12 @@ SignatureHelp signatures(llvm::StringRef Text, Position Point, 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, _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
