Author: Kadir Cetinkaya Date: 2020-04-21T10:27:26+02:00 New Revision: 2214b9076f1d3a4784820c4479e2417685e5c980
URL: https://github.com/llvm/llvm-project/commit/2214b9076f1d3a4784820c4479e2417685e5c980 DIFF: https://github.com/llvm/llvm-project/commit/2214b9076f1d3a4784820c4479e2417685e5c980.diff LOG: [clangd] Make signatureHelp work with stale preambles Summary: This is achieved by calculating newly added includes and implicitly parsing them as if they were part of the main file. This also gets rid of the need for consistent preamble reads. Reviewers: sammccall Subscribers: ilya-biryukov, javed.absar, MaskRay, jkorous, mgrang, arphaman, jfb, usaxena95, cfe-commits Tags: #clang Differential Revision: https://reviews.llvm.org/D77392 Added: clang-tools-extra/clangd/unittests/PreambleTests.cpp Modified: 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/TUSchedulerTests.cpp clang/include/clang/Basic/TokenKinds.h clang/include/clang/Frontend/PrecompiledPreamble.h Removed: ################################################################################ diff --git a/clang-tools-extra/clangd/ClangdServer.cpp b/clang-tools-extra/clangd/ClangdServer.cpp index f82bfa4f2682..c5148f81f3df 100644 --- a/clang-tools-extra/clangd/ClangdServer.cpp +++ b/clang-tools-extra/clangd/ClangdServer.cpp @@ -280,11 +280,8 @@ void ClangdServer::signatureHelp(PathRef File, Position Pos, 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)); } diff --git a/clang-tools-extra/clangd/CodeComplete.cpp b/clang-tools-extra/clangd/CodeComplete.cpp index 7dbb4f5b78a3..3fa5b53ebaad 100644 --- a/clang-tools-extra/clangd/CodeComplete.cpp +++ b/clang-tools-extra/clangd/CodeComplete.cpp @@ -1023,6 +1023,7 @@ struct SemaCompleteInput { 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 @@ bool semaCodeComplete(std::unique_ptr<CodeCompleteConsumer> Consumer, 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 @@ bool semaCodeComplete(std::unique_ptr<CodeCompleteConsumer> Consumer, 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 @@ codeComplete(PathRef FileName, const tooling::CompileCommand &Command, 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 @@ SignatureHelp signatureHelp(PathRef FileName, 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; } diff --git a/clang-tools-extra/clangd/Preamble.cpp b/clang-tools-extra/clangd/Preamble.cpp index 97cd22a5d1fc..8392748227b4 100644 --- a/clang-tools-extra/clangd/Preamble.cpp +++ b/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,81 @@ class CppFilePreambleCallbacks : public PreambleCallbacks { 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. +llvm::Expected<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 llvm::createStringError(llvm::inconvertibleErrorCode(), + "failed to create compiler invocation"); + 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); + if (Clang->getFrontendOpts().Inputs.empty()) + return llvm::createStringError(llvm::inconvertibleErrorCode(), + "compiler instance had no inputs"); + // We are only interested in main file includes. + Clang->getPreprocessorOpts().SingleFileParseMode = true; + PreambleOnlyAction Action; + if (!Action.BeginSourceFile(*Clang, Clang->getFrontendOpts().Inputs[0])) + return llvm::createStringError(llvm::inconvertibleErrorCode(), + "failed BeginSourceFile"); + Preprocessor &PP = Clang->getPreprocessor(); + IncludeStructure Includes; + PP.addPPCallbacks( + collectIncludeStructureCallback(Clang->getSourceManager(), &Includes)); + if (llvm::Error Err = Action.Execute()) + return std::move(Err); + 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 +269,78 @@ bool isPreambleCompatible(const PreambleData &Preamble, Preamble.Preamble.CanReuse(CI, ContentsBuffer.get(), Bounds, Inputs.FS.get()); } + +PreamblePatch PreamblePatch::create(llvm::StringRef FileName, + const ParseInputs &Modified, + const PreambleData &Baseline) { + // First scan the include directives in Baseline and Modified. These will be + // used to figure out newly added directives in Modified. Scanning can fail, + // the code just bails out and creates an empty patch in such cases, as: + // - If scanning for Baseline fails, no knowledge of existing includes hence + // patch will contain all the includes in Modified. Leading to rebuild of + // whole preamble, which is terribly slow. + // - If scanning for Modified fails, cannot figure out newly added ones so + // there's nothing to do but generate an empty patch. + auto BaselineIncludes = scanPreambleIncludes( + // Contents needs to be null-terminated. + Baseline.Preamble.getContents().str(), + Baseline.StatCache->getConsumingFS(Modified.FS), Modified.CompileCommand); + if (!BaselineIncludes) { + elog("Failed to scan includes for baseline of {0}: {1}", FileName, + BaselineIncludes.takeError()); + return {}; + } + auto ModifiedIncludes = scanPreambleIncludes( + Modified.Contents, Baseline.StatCache->getConsumingFS(Modified.FS), + Modified.CompileCommand); + if (!ModifiedIncludes) { + elog("Failed to scan includes for modified contents of {0}: {1}", FileName, + ModifiedIncludes.takeError()); + return {}; + } + + 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, record the ones in Baseline + // for exclusion. + llvm::DenseSet<std::pair<tok::PPKeywordKind, llvm::StringRef>> + ExistingIncludes; + for (const auto &Inc : *BaselineIncludes) + 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 : *ModifiedIncludes) { + 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 diff --git a/clang-tools-extra/clangd/Preamble.h b/clang-tools-extra/clangd/Preamble.h index bd67610e0ad7..10c292a71f38 100644 --- a/clang-tools-extra/clangd/Preamble.h +++ b/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 @@ buildPreamble(PathRef FileName, CompilerInvocation CI, 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 diff --git a/clang-tools-extra/clangd/TUScheduler.cpp b/clang-tools-extra/clangd/TUScheduler.cpp index 26adcfd2b8f2..2f2abb59ab3c 100644 --- a/clang-tools-extra/clangd/TUScheduler.cpp +++ b/clang-tools-extra/clangd/TUScheduler.cpp @@ -866,36 +866,6 @@ ASTWorker::getPossiblyStalePreamble() const { 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 @@ void TUScheduler::runWithPreamble(llvm::StringRef Name, PathRef File, 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)); diff --git a/clang-tools-extra/clangd/TUScheduler.h b/clang-tools-extra/clangd/TUScheduler.h index a7f5d2f493fb..48ed2c76f546 100644 --- a/clang-tools-extra/clangd/TUScheduler.h +++ b/clang-tools-extra/clangd/TUScheduler.h @@ -256,11 +256,6 @@ class TUScheduler { /// 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: diff --git a/clang-tools-extra/clangd/unittests/CMakeLists.txt b/clang-tools-extra/clangd/unittests/CMakeLists.txt index 541367a1d96a..4119445b85a0 100644 --- a/clang-tools-extra/clangd/unittests/CMakeLists.txt +++ b/clang-tools-extra/clangd/unittests/CMakeLists.txt @@ -59,6 +59,7 @@ add_unittest(ClangdUnitTests ClangdTests LSPClient.cpp ParsedASTTests.cpp PathMappingTests.cpp + PreambleTests.cpp PrintASTTests.cpp QualityTests.cpp RenameTests.cpp diff --git a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp index 794b21ee61a5..47637024ab91 100644 --- a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp +++ b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp @@ -1186,6 +1186,31 @@ TEST(SignatureHelpTest, OpeningParen) { } } +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 diff --git a/clang-tools-extra/clangd/unittests/PreambleTests.cpp b/clang-tools-extra/clangd/unittests/PreambleTests.cpp new file mode 100644 index 000000000000..2815ca0a46f1 --- /dev/null +++ b/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 diff ing 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 diff --git a/clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp b/clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp index 7106e01a10e4..6f50a5acd4e3 100644 --- a/clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp +++ b/clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp @@ -246,66 +246,6 @@ TEST_F(TUSchedulerTests, Debounce) { 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 diff --git a/clang/include/clang/Basic/TokenKinds.h b/clang/include/clang/Basic/TokenKinds.h index c25181e6827c..4e66aa1c8c2d 100644 --- a/clang/include/clang/Basic/TokenKinds.h +++ b/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 @@ bool isAnnotation(TokenKind K); /// 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 diff --git a/clang/include/clang/Frontend/PrecompiledPreamble.h b/clang/include/clang/Frontend/PrecompiledPreamble.h index 0d95ee683eee..538f6c92ad55 100644 --- a/clang/include/clang/Frontend/PrecompiledPreamble.h +++ b/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 @@ class PrecompiledPreamble { /// 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, _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
