kadircet updated this revision to Diff 256359.
kadircet added a comment.
Herald added a subscriber: mgorny.
- Encapsulate logic into `PreamblePatch`
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D77392/new/
https://reviews.llvm.org/D77392
Files:
clang-tools-extra/clangd/ClangdServer.cpp
clang-tools-extra/clangd/CodeComplete.cpp
clang-tools-extra/clangd/Preamble.cpp
clang-tools-extra/clangd/Preamble.h
clang-tools-extra/clangd/TUScheduler.cpp
clang-tools-extra/clangd/TUScheduler.h
clang-tools-extra/clangd/unittests/CMakeLists.txt
clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
clang-tools-extra/clangd/unittests/PreambleTests.cpp
clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
Index: clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
+++ clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
@@ -245,66 +245,6 @@
EXPECT_EQ(2, CallbackCount);
}
-static std::vector<std::string> includes(const PreambleData *Preamble) {
- std::vector<std::string> Result;
- if (Preamble)
- for (const auto &Inclusion : Preamble->Includes.MainFileIncludes)
- Result.push_back(Inclusion.Written);
- return Result;
-}
-
-TEST_F(TUSchedulerTests, PreambleConsistency) {
- std::atomic<int> CallbackCount(0);
- {
- Notification InconsistentReadDone; // Must live longest.
- TUScheduler S(CDB, optsForTest());
- auto Path = testPath("foo.cpp");
- // Schedule two updates (A, B) and two preamble reads (stale, consistent).
- // The stale read should see A, and the consistent read should see B.
- // (We recognize the preambles by their included files).
- auto Inputs = getInputs(Path, "#include <A>");
- Inputs.Version = "A";
- updateWithCallback(S, Path, Inputs, WantDiagnostics::Yes, [&]() {
- // This callback runs in between the two preamble updates.
-
- // This blocks update B, preventing it from winning the race
- // against the stale read.
- // If the first read was instead consistent, this would deadlock.
- InconsistentReadDone.wait();
- // This delays update B, preventing it from winning a race
- // against the consistent read. The consistent read sees B
- // only because it waits for it.
- // If the second read was stale, it would usually see A.
- std::this_thread::sleep_for(std::chrono::milliseconds(100));
- });
- Inputs.Contents = "#include <B>";
- Inputs.Version = "B";
- S.update(Path, Inputs, WantDiagnostics::Yes);
-
- S.runWithPreamble("StaleRead", Path, TUScheduler::Stale,
- [&](Expected<InputsAndPreamble> Pre) {
- ASSERT_TRUE(bool(Pre));
- ASSERT_TRUE(Pre->Preamble);
- EXPECT_EQ(Pre->Preamble->Version, "A");
- EXPECT_THAT(includes(Pre->Preamble),
- ElementsAre("<A>"));
- InconsistentReadDone.notify();
- ++CallbackCount;
- });
- S.runWithPreamble("ConsistentRead", Path, TUScheduler::Consistent,
- [&](Expected<InputsAndPreamble> Pre) {
- ASSERT_TRUE(bool(Pre));
- ASSERT_TRUE(Pre->Preamble);
- EXPECT_EQ(Pre->Preamble->Version, "B");
- EXPECT_THAT(includes(Pre->Preamble),
- ElementsAre("<B>"));
- ++CallbackCount;
- });
- S.blockUntilIdle(timeoutSeconds(10));
- }
- EXPECT_EQ(2, CallbackCount);
-}
-
TEST_F(TUSchedulerTests, Cancellation) {
// We have the following update/read sequence
// U0
Index: clang-tools-extra/clangd/unittests/PreambleTests.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/clangd/unittests/PreambleTests.cpp
@@ -0,0 +1,127 @@
+//===--- PreambleTests.cpp --------------------------------------*- 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 "Compiler.h"
+#include "Preamble.h"
+#include "TestFS.h"
+#include "clang/Lex/PreprocessorOptions.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/StringRef.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+#include <string>
+#include <vector>
+
+namespace clang {
+namespace clangd {
+namespace {
+
+using testing::_;
+using testing::Contains;
+using testing::Pair;
+
+MATCHER_P(HasContents, Contents, "") { return arg->getBuffer() == Contents; }
+
+TEST(PreamblePatchTest, All) {
+ MockFSProvider FS;
+ MockCompilationDatabase CDB;
+ IgnoreDiagnostics Diags;
+ ParseInputs PI;
+ PI.FS = FS.getFileSystem();
+
+ llvm::StringRef Cases[] = {
+ // Only preamble
+ R"cpp(^#include [["a.h"]])cpp",
+ // Both preamble and mainfile
+ R"cpp(
+ ^#include [["a.h"]]
+ garbage, finishes preamble
+ #include "a.h")cpp",
+ // Mixed directives
+ R"cpp(
+ ^#include [["a.h"]]
+ #pragma directive
+ // some comments
+ ^#include_next [[<a.h>]]
+ #ifdef skipped
+ // FIXME: we shouldn't add the following include as it is in a disabled
+ // region.
+ ^#import [["a.h"]]
+ #endif)cpp",
+ // Broken directives
+ R"cpp(
+ #include "a
+ ^#include [["a.h"]]
+ #include <b
+ ^#include [[<b.h>]])cpp",
+ };
+
+ const auto FileName = testPath("foo.cc");
+ for (const auto Case : Cases) {
+ Annotations Test(Case);
+ const auto Code = Test.code();
+ PI.CompileCommand = *CDB.getCompileCommand(FileName);
+
+ SCOPED_TRACE(Code);
+ // Check preamble lexing logic by building an empty preamble and patching it
+ // with all the contents.
+ {
+ PI.Contents = "";
+ const auto CI = buildCompilerInvocation(PI, Diags);
+ const auto EmptyPreamble =
+ buildPreamble(FileName, *CI, PI, true, nullptr);
+ const auto PP = PreamblePatch::create(FileName, Code, *EmptyPreamble,
+ *CI->getLangOpts());
+
+ std::vector<std::string> Headers;
+ const auto Points = Test.points();
+ const auto Ranges = Test.ranges();
+ for (size_t I = 0, E = Points.size(); I != E; ++I) {
+ auto StartOffset =
+ llvm::cantFail(positionToOffset(Code, Ranges[I].start));
+ auto EndOffset = llvm::cantFail(positionToOffset(Code, Ranges[I].end));
+ Headers.push_back(
+ Code.substr(StartOffset, EndOffset - StartOffset).str());
+ }
+ llvm::sort(Headers);
+
+ std::string ExpectedBuffer;
+ for (llvm::StringRef Header : Headers) {
+ ExpectedBuffer.append("#include ");
+ ExpectedBuffer.append(Header.str());
+ ExpectedBuffer += '\n';
+ }
+
+ PP.apply(*CI);
+
+ EXPECT_THAT(CI->getPreprocessorOpts().RemappedFileBuffers,
+ Contains(Pair(_, HasContents(ExpectedBuffer))));
+ }
+
+ // Check diffing logic by adding a new header to the preamble and ensuring
+ // only it is patched.
+ {
+ PI.Contents = Code.str();
+ const auto CI = buildCompilerInvocation(PI, Diags);
+ const auto FullPreamble = buildPreamble(FileName, *CI, PI, true, nullptr);
+
+ constexpr llvm::StringLiteral Patch = "#include <test>\n";
+ const auto PP = PreamblePatch::create(FileName, (Patch + Code).str(),
+ *FullPreamble, *CI->getLangOpts());
+
+ PP.apply(*CI);
+ EXPECT_THAT(CI->getPreprocessorOpts().RemappedFileBuffers,
+ Contains(Pair(_, HasContents(Patch))));
+ }
+ }
+}
+
+} // namespace
+} // namespace clangd
+} // namespace clang
Index: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -1186,6 +1186,31 @@
}
}
+TEST(SignatureHelpTest, StalePreamble) {
+ TestTU TU;
+ TU.Code = "";
+ IgnoreDiagnostics Diags;
+ auto Inputs = TU.inputs();
+ auto CI = buildCompilerInvocation(Inputs, Diags);
+ ASSERT_TRUE(CI);
+ auto EmptyPreamble = buildPreamble(testPath(TU.Filename), *CI, Inputs,
+ /*InMemory=*/true, /*Callback=*/nullptr);
+ ASSERT_TRUE(EmptyPreamble);
+
+ TU.AdditionalFiles["a.h"] = "int foo(int x);";
+ const Annotations Test(R"cpp(
+ #include "a.h"
+ void bar() { foo(^2); })cpp");
+ TU.Code = Test.code().str();
+ Inputs = TU.inputs();
+ auto Results =
+ signatureHelp(testPath(TU.Filename), Inputs.CompileCommand,
+ *EmptyPreamble, TU.Code, Test.point(), Inputs.FS, nullptr);
+ EXPECT_THAT(Results.signatures, ElementsAre(Sig("foo([[int x]]) -> int")));
+ EXPECT_EQ(0, Results.activeSignature);
+ EXPECT_EQ(0, Results.activeParameter);
+}
+
class IndexRequestCollector : public SymbolIndex {
public:
bool
Index: clang-tools-extra/clangd/unittests/CMakeLists.txt
===================================================================
--- clang-tools-extra/clangd/unittests/CMakeLists.txt
+++ clang-tools-extra/clangd/unittests/CMakeLists.txt
@@ -57,6 +57,7 @@
JSONTransportTests.cpp
ParsedASTTests.cpp
PathMappingTests.cpp
+ PreambleTests.cpp
PrintASTTests.cpp
QualityTests.cpp
RenameTests.cpp
Index: clang-tools-extra/clangd/TUScheduler.h
===================================================================
--- clang-tools-extra/clangd/TUScheduler.h
+++ clang-tools-extra/clangd/TUScheduler.h
@@ -251,11 +251,6 @@
/// Controls whether preamble reads wait for the preamble to be up-to-date.
enum PreambleConsistency {
- /// The preamble is generated from the current version of the file.
- /// If the content was recently updated, we will wait until we have a
- /// preamble that reflects that update.
- /// This is the slowest option, and may be delayed by other tasks.
- Consistent,
/// The preamble may be generated from an older version of the file.
/// Reading from locations in the preamble may cause files to be re-read.
/// This gives callers two options:
Index: clang-tools-extra/clangd/TUScheduler.cpp
===================================================================
--- clang-tools-extra/clangd/TUScheduler.cpp
+++ clang-tools-extra/clangd/TUScheduler.cpp
@@ -860,36 +860,6 @@
return LatestPreamble;
}
-void ASTWorker::getCurrentPreamble(
- llvm::unique_function<void(std::shared_ptr<const PreambleData>)> Callback) {
- // We could just call startTask() to throw the read on the queue, knowing
- // it will run after any updates. But we know this task is cheap, so to
- // improve latency we cheat: insert it on the queue after the last update.
- std::unique_lock<std::mutex> Lock(Mutex);
- auto LastUpdate =
- std::find_if(Requests.rbegin(), Requests.rend(),
- [](const Request &R) { return R.UpdateType.hasValue(); });
- // If there were no writes in the queue, and CurrentRequest is not a write,
- // the preamble is ready now.
- if (LastUpdate == Requests.rend() &&
- (!CurrentRequest || CurrentRequest->UpdateType.hasValue())) {
- Lock.unlock();
- return Callback(getPossiblyStalePreamble());
- }
- assert(!RunSync && "Running synchronously, but queue is non-empty!");
- Requests.insert(LastUpdate.base(),
- Request{[Callback = std::move(Callback), this]() mutable {
- Callback(getPossiblyStalePreamble());
- },
- "GetPreamble", steady_clock::now(),
- Context::current().clone(),
- /*UpdateType=*/None,
- /*InvalidationPolicy=*/TUScheduler::NoInvalidation,
- /*Invalidate=*/nullptr});
- Lock.unlock();
- RequestsCV.notify_all();
-}
-
void ASTWorker::waitForFirstPreamble() const { BuiltFirstPreamble.wait(); }
tooling::CompileCommand ASTWorker::getCurrentCompileCommand() const {
@@ -1297,41 +1267,21 @@
return;
}
- // Future is populated if the task needs a specific preamble.
- std::future<std::shared_ptr<const PreambleData>> ConsistentPreamble;
- // FIXME: Currently this only holds because ASTWorker blocks after issuing a
- // preamble build. Get rid of consistent reads or make them build on the
- // calling thread instead.
- if (Consistency == Consistent) {
- std::promise<std::shared_ptr<const PreambleData>> Promise;
- ConsistentPreamble = Promise.get_future();
- It->second->Worker->getCurrentPreamble(
- [Promise = std::move(Promise)](
- std::shared_ptr<const PreambleData> Preamble) mutable {
- Promise.set_value(std::move(Preamble));
- });
- }
-
std::shared_ptr<const ASTWorker> Worker = It->second->Worker.lock();
auto Task =
[Worker, Consistency, Name = Name.str(), File = File.str(),
Contents = It->second->Contents,
Command = Worker->getCurrentCompileCommand(),
Ctx = Context::current().derive(kFileBeingProcessed, std::string(File)),
- ConsistentPreamble = std::move(ConsistentPreamble),
Action = std::move(Action), this]() mutable {
std::shared_ptr<const PreambleData> Preamble;
- if (ConsistentPreamble.valid()) {
- Preamble = ConsistentPreamble.get();
- } else {
- 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();
+ if (Consistency == PreambleConsistency::Stale) {
+ // 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();
std::lock_guard<Semaphore> BarrierLock(Barrier);
WithContext Guard(std::move(Ctx));
Index: clang-tools-extra/clangd/Preamble.h
===================================================================
--- clang-tools-extra/clangd/Preamble.h
+++ clang-tools-extra/clangd/Preamble.h
@@ -32,6 +32,7 @@
#include "clang/Frontend/CompilerInvocation.h"
#include "clang/Frontend/PrecompiledPreamble.h"
#include "clang/Tooling/CompilationDatabase.h"
+#include "llvm/ADT/StringRef.h"
#include <memory>
#include <string>
@@ -49,7 +50,8 @@
std::vector<Diag> Diags, IncludeStructure Includes,
MainFileMacros Macros,
std::unique_ptr<PreambleFileStatusCache> StatCache,
- CanonicalIncludes CanonIncludes);
+ CanonicalIncludes CanonIncludes,
+ std::vector<Inclusion> LexedIncludes);
// Version of the ParseInputs this preamble was built from.
std::string Version;
@@ -67,6 +69,9 @@
// When reusing a preamble, this cache can be consumed to save IO.
std::unique_ptr<PreambleFileStatusCache> StatCache;
CanonicalIncludes CanonIncludes;
+ /// Contains all of the includes spelled in the preamble section, even the
+ /// ones in disabled regions.
+ std::vector<Inclusion> LexedIncludes;
};
using PreambleParsedCallback =
@@ -88,6 +93,21 @@
bool isPreambleCompatible(const PreambleData &Preamble,
const ParseInputs &Inputs, PathRef FileName,
const CompilerInvocation &CI);
+
+class PreamblePatch {
+public:
+ PreamblePatch() = default;
+ static PreamblePatch create(llvm::StringRef FileName,
+ llvm::StringRef Contents,
+ const PreambleData &Preamble,
+ const LangOptions &LO);
+ void apply(CompilerInvocation &CI) const;
+
+private:
+ std::string PatchContents;
+ std::string PatchFileName;
+};
+
} // namespace clangd
} // namespace clang
Index: clang-tools-extra/clangd/Preamble.cpp
===================================================================
--- clang-tools-extra/clangd/Preamble.cpp
+++ clang-tools-extra/clangd/Preamble.cpp
@@ -10,9 +10,20 @@
#include "Compiler.h"
#include "Logger.h"
#include "Trace.h"
+#include "clang/Basic/LangOptions.h"
#include "clang/Basic/SourceLocation.h"
+#include "clang/Frontend/CompilerInvocation.h"
+#include "clang/Lex/Lexer.h"
#include "clang/Lex/PPCallbacks.h"
+#include "clang/Lex/Preprocessor.h"
#include "clang/Lex/PreprocessorOptions.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/FormatVariadic.h"
+#include "llvm/Support/MemoryBuffer.h"
+#include "llvm/Support/Path.h"
+#include "llvm/Support/raw_ostream.h"
+#include <string>
+#include <vector>
namespace clang {
namespace clangd {
@@ -74,6 +85,89 @@
const SourceManager *SourceMgr = nullptr;
};
+/// Gets the includes in the preamble section of the file by running lexer over
+/// \p Contents. Returned inclusions are sorted according to written filename.
+std::vector<Inclusion> getPreambleIncludes(llvm::StringRef Contents,
+ const PreambleBounds &Bounds,
+ const LangOptions &LangOpts) {
+ auto ContentsBuffer = llvm::MemoryBuffer::getMemBuffer(Contents, "dummy");
+ SourceManagerForFile SMFF("dummy", ContentsBuffer->getBuffer());
+ auto &SM = SMFF.get();
+ auto FID = SM.getMainFileID();
+
+ // We might want to run Preprocessor here instead of raw lexer to get more
+ // accurate results, e.g. skipping of includes in disabled PP regions.
+ // Using a PP might result in extra IO though, as it tries to resolve include
+ // directives into absolute file paths, which causes stats and realpath calls.
+ // We can get rid of that expensive IO in couple of different ways:
+ // - Use the StatCache build with a preamble, this won't change number of
+ // stats but will get rid of IO.
+ // - Use resolved paths coming from a preamble to build an alias map in
+ // HeaderSearchInfo. This will reduce number of stats for known files to
+ // one.
+ // - Map everything to an absolute path and provide a null/dummy VFS that will
+ // fail all stats. This is OK as we don't care about what the include really
+ // resolves to.
+ Lexer TheLexer(FID, SM.getBuffer(FID), SM, LangOpts);
+ Token TheTok;
+ std::vector<Inclusion> Incs;
+ Inclusion Inc;
+
+ // Lex until end of preamble bounds.
+ while (!TheLexer.LexFromRawLexer(TheTok) &&
+ TheLexer.getCurrentBufferOffset() < Bounds.Size) {
+ // Skip unless at the start of a PP directive.
+ if (!TheTok.isAtStartOfLine() || !TheTok.is(tok::hash))
+ continue;
+ Inc.HashOffset = TheLexer.getCurrentBufferOffset() - TheTok.getLength();
+
+ // Skip if not an include directive.
+ TheLexer.LexFromRawLexer(TheTok);
+ if (!TheTok.is(tok::raw_identifier))
+ continue;
+ llvm::StringRef Directive = TheTok.getRawIdentifier();
+ if (Directive != "include" && Directive != "import" &&
+ Directive != "include_next")
+ continue;
+
+ // Skip if on a broken include directive
+ TheLexer.LexIncludeFilename(TheTok);
+ if (!TheTok.isLiteral())
+ continue;
+ Inc.Written =
+ llvm::StringRef(TheTok.getLiteralData(), TheTok.getLength()).str();
+ Inc.R = halfOpenToRange(SM, CharSourceRange::getCharRange(
+ TheTok.getLocation(), TheTok.getEndLoc()));
+ Incs.push_back(std::move(Inc));
+ }
+ llvm::sort(Incs, [](const Inclusion &LHS, const Inclusion &RHS) {
+ return LHS.Written < RHS.Written;
+ });
+
+ return Incs;
+}
+
+/// Returns set of includes existing in \p New but missing in \p Old. Assumes
+/// inputs are sorted w.r.t Written field.
+std::vector<Inclusion> newIncludes(llvm::ArrayRef<Inclusion> Old,
+ llvm::ArrayRef<Inclusion> New) {
+ std::vector<Inclusion> Added;
+ while (!Old.empty() && !New.empty()) {
+ // Old.front() doesn't exists in New. Ignored as we are only looking for
+ // additions.
+ if (Old.front().Written < New.front().Written)
+ Old = Old.drop_front();
+ else if (Old.front().Written > New.front().Written) {
+ Added.push_back(New.front());
+ New = New.drop_front();
+ } else {
+ Old = Old.drop_front();
+ New = New.drop_front();
+ }
+ }
+ Added.insert(Added.end(), New.begin(), New.end());
+ return Added;
+}
} // namespace
PreambleData::PreambleData(const ParseInputs &Inputs,
@@ -81,12 +175,13 @@
std::vector<Diag> Diags, IncludeStructure Includes,
MainFileMacros Macros,
std::unique_ptr<PreambleFileStatusCache> StatCache,
- CanonicalIncludes CanonIncludes)
+ CanonicalIncludes CanonIncludes,
+ std::vector<Inclusion> LexedIncludes)
: Version(Inputs.Version), CompileCommand(Inputs.CompileCommand),
Preamble(std::move(Preamble)), Diags(std::move(Diags)),
Includes(std::move(Includes)), Macros(std::move(Macros)),
- StatCache(std::move(StatCache)), CanonIncludes(std::move(CanonIncludes)) {
-}
+ StatCache(std::move(StatCache)), CanonIncludes(std::move(CanonIncludes)),
+ LexedIncludes(std::move(LexedIncludes)) {}
std::shared_ptr<const PreambleData>
buildPreamble(PathRef FileName, CompilerInvocation CI,
@@ -146,7 +241,8 @@
Inputs, std::move(*BuiltPreamble), std::move(Diags),
SerializedDeclsCollector.takeIncludes(),
SerializedDeclsCollector.takeMacros(), std::move(StatCache),
- SerializedDeclsCollector.takeCanonicalIncludes());
+ SerializedDeclsCollector.takeCanonicalIncludes(),
+ getPreambleIncludes(Inputs.Contents, Bounds, *CI.getLangOpts()));
} else {
elog("Could not build a preamble for file {0} version {1}", FileName,
Inputs.Version);
@@ -166,5 +262,41 @@
Preamble.Preamble.CanReuse(CI, ContentsBuffer.get(), Bounds,
Inputs.FS.get());
}
+
+PreamblePatch PreamblePatch::create(llvm::StringRef FileName,
+ llvm::StringRef Contents,
+ const PreambleData &Preamble,
+ const LangOptions &LO) {
+ PreamblePatch PP;
+ auto ContentsBuffer = llvm::MemoryBuffer::getMemBuffer(Contents, FileName);
+ auto Bounds = ComputePreambleBounds(LO, ContentsBuffer.get(), 0);
+ // This shouldn't coincide with any real file name.
+ PP.PatchFileName = llvm::formatv("{0}_preamble_patch.h", FileName);
+
+ // Calculate extra includes that needs to be inserted.
+ auto CurrentIncludes = getPreambleIncludes(Contents, Bounds, LO);
+ auto NewIncludes = newIncludes(Preamble.LexedIncludes, CurrentIncludes);
+ llvm::raw_string_ostream Patch(PP.PatchContents);
+ for (const auto &Inc : NewIncludes) {
+ // FIXME: Preserve directive kind in Inclusion and use that instead of
+ // always using include.
+ Patch << "#include " << Inc.Written << '\n';
+ }
+ // FIXME: Handle more directives, e.g. define/undef.
+ return PP;
+}
+
+void PreamblePatch::apply(CompilerInvocation &CI) const {
+ // No need to map an empty file.
+ if (PatchContents.empty())
+ return;
+ auto &PPOpts = CI.getPreprocessorOpts();
+ auto PatchBuffer =
+ llvm::MemoryBuffer::getMemBuffer(PatchContents, PatchFileName);
+ // CI will take care of the lifetime of the buffer.
+ PPOpts.addRemappedFile(PatchFileName, PatchBuffer.release());
+ PPOpts.Includes.push_back(std::move(PatchFileName));
+}
+
} // namespace clangd
} // namespace clang
Index: clang-tools-extra/clangd/CodeComplete.cpp
===================================================================
--- clang-tools-extra/clangd/CodeComplete.cpp
+++ clang-tools-extra/clangd/CodeComplete.cpp
@@ -1023,6 +1023,7 @@
PathRef FileName;
const tooling::CompileCommand &Command;
const PreambleData &Preamble;
+ const PreamblePatch &PreamblePatch;
llvm::StringRef Contents;
size_t Offset;
llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS;
@@ -1048,6 +1049,9 @@
// Invokes Sema code completion on a file.
// If \p Includes is set, it will be updated based on the compiler invocation.
+// If \p PatchAdditionalIncludes is set, calculates newly introduced includes to
+// current file contents since \p Input.Preamble was built and parses those
+// includes as part of the mainfile.
bool semaCodeComplete(std::unique_ptr<CodeCompleteConsumer> Consumer,
const clang::CodeCompleteOptions &Options,
const SemaCompleteInput &Input,
@@ -1096,6 +1100,8 @@
PreambleBounds PreambleRegion =
ComputePreambleBounds(*CI->getLangOpts(), ContentsBuffer.get(), 0);
bool CompletingInPreamble = PreambleRegion.Size > Input.Offset;
+
+ Input.PreamblePatch.apply(*CI);
// NOTE: we must call BeginSourceFile after prepareCompilerInstance. Otherwise
// the remapped buffers do not get freed.
auto Clang = prepareCompilerInstance(
@@ -1754,8 +1760,10 @@
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});
+ : std::move(Flow).run({FileName, Command, *Preamble,
+ // We want to serve code completions with
+ // low latency, so don't bother patching.
+ PreamblePatch(), Contents, *Offset, VFS});
}
SignatureHelp signatureHelp(PathRef FileName,
@@ -1775,10 +1783,13 @@
Options.IncludeMacros = false;
Options.IncludeCodePatterns = false;
Options.IncludeBriefComments = false;
- IncludeStructure PreambleInclusions; // Unused for signatureHelp
+
+ // FIXME: Preserve LangOpts in PreambleData or build one out of Command.
+ LangOptions LO;
+ auto PP = PreamblePatch::create(FileName, Contents, Preamble, LO);
semaCodeComplete(
std::make_unique<SignatureHelpCollector>(Options, Index, Result), Options,
- {FileName, Command, Preamble, Contents, *Offset, std::move(VFS)});
+ {FileName, Command, Preamble, PP, Contents, *Offset, std::move(VFS)});
return Result;
}
Index: clang-tools-extra/clangd/ClangdServer.cpp
===================================================================
--- clang-tools-extra/clangd/ClangdServer.cpp
+++ clang-tools-extra/clangd/ClangdServer.cpp
@@ -280,11 +280,8 @@
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,
+ // Unlike code completion, we wait for a preamble here.
+ WorkScheduler.runWithPreamble("SignatureHelp", File, TUScheduler::Stale,
std::move(Action));
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
