[clang-tools-extra] 43aa04e - [clangd] Run semaCodeComplete only with a preamble
Kadir Cetinkaya via cfe-commits
cfe-commits at lists.llvm.org
Wed Apr 1 04:03:45 PDT 2020
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,
More information about the cfe-commits
mailing list