kadircet updated this revision to Diff 258833.
kadircet marked 2 inline comments as done.
kadircet added a comment.
- Address comments
- Don't store LexedInclude in PreambleData, expose the contents in
PrecompiledPreamble instead.
- Introduce DenseMapInfo for PPKeywordKind
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
clang/include/clang/Basic/TokenKinds.h
clang/include/clang/Frontend/PrecompiledPreamble.h
Index: clang/include/clang/Frontend/PrecompiledPreamble.h
===================================================================
--- clang/include/clang/Frontend/PrecompiledPreamble.h
+++ clang/include/clang/Frontend/PrecompiledPreamble.h
@@ -16,6 +16,7 @@
#include "clang/Lex/Lexer.h"
#include "clang/Lex/Preprocessor.h"
#include "llvm/ADT/IntrusiveRefCntPtr.h"
+#include "llvm/ADT/StringRef.h"
#include "llvm/Support/AlignOf.h"
#include "llvm/Support/MD5.h"
#include <cstddef>
@@ -94,6 +95,11 @@
/// be used for logging and debugging purposes only.
std::size_t getSize() const;
+ /// Returned string is not null-terminated.
+ llvm::StringRef getContents() const {
+ return {PreambleBytes.data(), PreambleBytes.size()};
+ }
+
/// Check whether PrecompiledPreamble can be reused for the new contents(\p
/// MainFileBuffer) of the main file.
bool CanReuse(const CompilerInvocation &Invocation,
Index: clang/include/clang/Basic/TokenKinds.h
===================================================================
--- clang/include/clang/Basic/TokenKinds.h
+++ clang/include/clang/Basic/TokenKinds.h
@@ -14,6 +14,7 @@
#ifndef LLVM_CLANG_BASIC_TOKENKINDS_H
#define LLVM_CLANG_BASIC_TOKENKINDS_H
+#include "llvm/ADT/DenseMapInfo.h"
#include "llvm/Support/Compiler.h"
namespace clang {
@@ -95,7 +96,25 @@
/// Return true if this is an annotation token representing a pragma.
bool isPragmaAnnotation(TokenKind K);
-} // end namespace tok
-} // end namespace clang
+} // end namespace tok
+} // end namespace clang
+
+namespace llvm {
+template <> struct DenseMapInfo<clang::tok::PPKeywordKind> {
+ static inline clang::tok::PPKeywordKind getEmptyKey() {
+ return clang::tok::PPKeywordKind::pp_not_keyword;
+ }
+ static inline clang::tok::PPKeywordKind getTombstoneKey() {
+ return clang::tok::PPKeywordKind::NUM_PP_KEYWORDS;
+ }
+ static unsigned getHashValue(const clang::tok::PPKeywordKind &Val) {
+ return static_cast<unsigned>(Val);
+ }
+ static bool isEqual(const clang::tok::PPKeywordKind &LHS,
+ const clang::tok::PPKeywordKind &RHS) {
+ return LHS == RHS;
+ }
+};
+} // namespace llvm
#endif
Index: clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
+++ clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
@@ -246,66 +246,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,123 @@
+//===--- 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, IncludeParsing) {
+ MockFSProvider FS;
+ MockCompilationDatabase CDB;
+ IgnoreDiagnostics Diags;
+ ParseInputs PI;
+ PI.FS = FS.getFileSystem();
+
+ // We expect any line with a point to show up in the patch.
+ 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
+ ^#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);
+ PI.Contents = Code.str();
+
+ std::string ExpectedBuffer;
+ const auto Points = Test.points();
+ for (const auto &P : Points) {
+ // Copy the whole line.
+ auto StartOffset = llvm::cantFail(positionToOffset(Code, P));
+ ExpectedBuffer.append(Code.substr(StartOffset)
+ .take_until([](char C) { return C == '\n'; })
+ .str());
+ ExpectedBuffer += '\n';
+ }
+
+ PreamblePatch::create(FileName, PI, *EmptyPreamble).apply(*CI);
+ EXPECT_THAT(CI->getPreprocessorOpts().RemappedFileBuffers,
+ Contains(Pair(_, HasContents(ExpectedBuffer))));
+ }
+}
+
+TEST(PreamblePatchTest, ContainsNewIncludes) {
+ MockFSProvider FS;
+ MockCompilationDatabase CDB;
+ IgnoreDiagnostics Diags;
+
+ const auto FileName = testPath("foo.cc");
+ ParseInputs PI;
+ PI.FS = FS.getFileSystem();
+ PI.CompileCommand = *CDB.getCompileCommand(FileName);
+ PI.Contents = "#include <existing.h>\n";
+
+ // Check diffing logic by adding a new header to the preamble and ensuring
+ // only it is patched.
+ const auto CI = buildCompilerInvocation(PI, Diags);
+ const auto FullPreamble = buildPreamble(FileName, *CI, PI, true, nullptr);
+
+ constexpr llvm::StringLiteral Patch =
+ "#include <test>\n#import <existing.h>\n";
+ // We provide the same includes twice, they shouldn't be included in the
+ // patch.
+ PI.Contents = (Patch + PI.Contents + PI.Contents).str();
+ PreamblePatch::create(FileName, PI, *FullPreamble).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
@@ -59,6 +59,7 @@
LSPClient.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
@@ -256,11 +256,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
@@ -866,36 +866,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 {
@@ -1307,41 +1277,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>
@@ -88,6 +89,31 @@
bool isPreambleCompatible(const PreambleData &Preamble,
const ParseInputs &Inputs, PathRef FileName,
const CompilerInvocation &CI);
+
+/// Stores information required to parse a TU using a (possibly stale) Baseline
+/// preamble. Updates compiler invocation to approximately reflect additions to
+/// the preamble section of Modified contents, e.g. new include directives.
+class PreamblePatch {
+public:
+ // With an empty patch, the preamble is used verbatim.
+ PreamblePatch() = default;
+ /// Builds a patch that contains new PP directives introduced to the preamble
+ /// section of \p Modified compared to \p Baseline.
+ /// FIXME: This only handles include directives, we should at least handle
+ /// define/undef.
+ static PreamblePatch create(llvm::StringRef FileName,
+ const ParseInputs &Modified,
+ const PreambleData &Baseline);
+ /// Adjusts CI (which compiles the modified inputs) to be used with the
+ /// baseline preamble. This is done by inserting an artifical include to the
+ /// \p CI that contains new directives calculated in create.
+ 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
@@ -8,11 +8,39 @@
#include "Preamble.h"
#include "Compiler.h"
+#include "Headers.h"
#include "Logger.h"
#include "Trace.h"
+#include "clang/Basic/Diagnostic.h"
+#include "clang/Basic/LangOptions.h"
#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/TokenKinds.h"
+#include "clang/Frontend/CompilerInvocation.h"
+#include "clang/Frontend/FrontendActions.h"
+#include "clang/Lex/Lexer.h"
#include "clang/Lex/PPCallbacks.h"
+#include "clang/Lex/Preprocessor.h"
#include "clang/Lex/PreprocessorOptions.h"
+#include "clang/Tooling/CompilationDatabase.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/IntrusiveRefCntPtr.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/SmallString.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/StringSet.h"
+#include "llvm/Support/Error.h"
+#include "llvm/Support/ErrorHandling.h"
+#include "llvm/Support/FormatVariadic.h"
+#include "llvm/Support/MemoryBuffer.h"
+#include "llvm/Support/Path.h"
+#include "llvm/Support/VirtualFileSystem.h"
+#include "llvm/Support/raw_ostream.h"
+#include <iterator>
+#include <memory>
+#include <string>
+#include <system_error>
+#include <utility>
+#include <vector>
namespace clang {
namespace clangd {
@@ -74,6 +102,80 @@
const SourceManager *SourceMgr = nullptr;
};
+// Runs preprocessor over preamble section.
+class PreambleOnlyAction : public PreprocessorFrontendAction {
+protected:
+ void ExecuteAction() override {
+ Preprocessor &PP = getCompilerInstance().getPreprocessor();
+ auto &SM = PP.getSourceManager();
+ PP.EnterMainSourceFile();
+ auto Bounds = ComputePreambleBounds(getCompilerInstance().getLangOpts(),
+ SM.getBuffer(SM.getMainFileID()), 0);
+ Token Tok;
+ do {
+ PP.Lex(Tok);
+ assert(SM.isInMainFile(Tok.getLocation()));
+ } while (Tok.isNot(tok::eof) &&
+ SM.getDecomposedLoc(Tok.getLocation()).second < Bounds.Size);
+ }
+};
+
+/// Gets the includes in the preamble section of the file by running
+/// preprocessor over \p Contents. Returned includes do not contain resolved
+/// paths. \p VFS and \p Cmd is used to build the compiler invocation, which
+/// might stat/read files.
+std::vector<Inclusion>
+scanPreambleIncludes(llvm::StringRef Contents,
+ llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS,
+ const tooling::CompileCommand &Cmd) {
+ // Build and run Preprocessor over the preamble.
+ ParseInputs PI;
+ PI.Contents = Contents.str();
+ PI.FS = std::move(VFS);
+ PI.CompileCommand = Cmd;
+ IgnoringDiagConsumer IgnoreDiags;
+ auto CI = buildCompilerInvocation(PI, IgnoreDiags);
+ if (!CI)
+ return {};
+ CI->getDiagnosticOpts().IgnoreWarnings = true;
+ auto ContentsBuffer = llvm::MemoryBuffer::getMemBuffer(Contents);
+ auto Clang = prepareCompilerInstance(
+ std::move(CI), nullptr, std::move(ContentsBuffer),
+ // Provide an empty FS to prevent preprocessor from performing IO. This
+ // also implies missing resolved paths for includes.
+ new llvm::vfs::InMemoryFileSystem, IgnoreDiags);
+ // We are only interested in main file includes.
+ Clang->getPreprocessorOpts().SingleFileParseMode = true;
+ PreambleOnlyAction Action;
+ if (Clang->getFrontendOpts().Inputs.empty())
+ return {};
+ if (!Action.BeginSourceFile(*Clang, Clang->getFrontendOpts().Inputs[0]))
+ return {};
+ Preprocessor &PP = Clang->getPreprocessor();
+ IncludeStructure Includes;
+ PP.addPPCallbacks(
+ collectIncludeStructureCallback(Clang->getSourceManager(), &Includes));
+ if (llvm::Error Err = Action.Execute()) {
+ elog("Failed to run include collection: {0}", toString(std::move(Err)));
+ return {};
+ }
+ Action.EndSourceFile();
+ return Includes.MainFileIncludes;
+}
+
+const char *spellingForIncDirective(tok::PPKeywordKind IncludeDirective) {
+ switch (IncludeDirective) {
+ case tok::pp_include:
+ return "include";
+ case tok::pp_import:
+ return "import";
+ case tok::pp_include_next:
+ return "include_next";
+ default:
+ break;
+ }
+ llvm_unreachable("not an include directive");
+}
} // namespace
PreambleData::PreambleData(const ParseInputs &Inputs,
@@ -166,5 +268,57 @@
Preamble.Preamble.CanReuse(CI, ContentsBuffer.get(), Bounds,
Inputs.FS.get());
}
+
+PreamblePatch PreamblePatch::create(llvm::StringRef FileName,
+ const ParseInputs &Modified,
+ const PreambleData &Baseline) {
+ PreamblePatch PP;
+ // This shouldn't coincide with any real file name.
+ llvm::SmallString<128> PatchName;
+ llvm::sys::path::append(PatchName, llvm::sys::path::parent_path(FileName),
+ "__preamble_patch__.h");
+ PP.PatchFileName = PatchName.str().str();
+
+ // We are only interested in newly added includes.
+ llvm::DenseSet<std::pair<tok::PPKeywordKind, llvm::StringRef>>
+ ExistingIncludes;
+ auto PreambleIncludes = scanPreambleIncludes(
+ // Contents needs to be null-terminated.
+ Baseline.Preamble.getContents().str(),
+ Baseline.StatCache->getConsumingFS(Modified.FS), Modified.CompileCommand);
+ for (const auto &Inc : PreambleIncludes)
+ ExistingIncludes.insert({Inc.Directive, Inc.Written});
+
+ // Calculate extra includes that needs to be inserted.
+ llvm::raw_string_ostream Patch(PP.PatchContents);
+ for (const auto &Inc : scanPreambleIncludes(
+ Modified.Contents, Baseline.StatCache->getConsumingFS(Modified.FS),
+ Modified.CompileCommand)) {
+ if (ExistingIncludes.count({Inc.Directive, Inc.Written}))
+ continue;
+ Patch << llvm::formatv("#{0} {1}\n", spellingForIncDirective(Inc.Directive),
+ Inc.Written);
+ }
+ Patch.flush();
+ // 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 =
+ // we copy here to ensure contents are still valid if CI outlives the
+ // PreamblePatch.
+ llvm::MemoryBuffer::getMemBufferCopy(PatchContents, PatchFileName);
+ // CI will take care of the lifetime of the buffer.
+ PPOpts.addRemappedFile(PatchFileName, PatchBuffer.release());
+ // The patch will be parsed after loading the preamble ast and before parsing
+ // the main file.
+ PPOpts.Includes.push_back(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;
@@ -1060,7 +1061,6 @@
ParseInput.CompileCommand = Input.Command;
ParseInput.FS = VFS;
ParseInput.Contents = std::string(Input.Contents);
- ParseInput.Opts = ParseOptions();
IgnoreDiagnostics IgnoreDiags;
auto CI = buildCompilerInvocation(ParseInput, IgnoreDiags);
@@ -1096,6 +1096,7 @@
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 +1755,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 +1778,15 @@
Options.IncludeMacros = false;
Options.IncludeCodePatterns = false;
Options.IncludeBriefComments = false;
- IncludeStructure PreambleInclusions; // Unused for signatureHelp
+
+ ParseInputs PI;
+ PI.CompileCommand = Command;
+ PI.Contents = Contents.str();
+ PI.FS = std::move(VFS);
+ auto PP = PreamblePatch::create(FileName, PI, Preamble);
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(PI.FS)});
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
