From 34e39eb2adc2b3f16c2c2c0607a904ee55705c01 Mon Sep 17 00:00:00 2001 From: Kadir Cetinkaya Date: Tue, 5 May 2020 17:55:11 +0200 Subject: [PATCH] [clangd] Change PreambleOnlyAction with content truncation Summary: Lexing until the token location is past preamble bound could be wrong in some cases as preprocessor lexer can lex multiple tokens in a single call. Reviewers: sammccall Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, usaxena95, cfe-commits Tags: #clang Differential Revision: https://reviews.llvm.org/D79426 --- clang-tools-extra/clangd/Preamble.cpp | 29 +++++++--------------- .../clangd/unittests/PreambleTests.cpp | 5 ++++ 2 files changed, 14 insertions(+), 20 deletions(-) diff --git a/clang-tools-extra/clangd/Preamble.cpp b/clang-tools-extra/clangd/Preamble.cpp index 9d9c5ef..d3eaa92d 100644 --- a/clang-tools-extra/clangd/Preamble.cpp +++ b/clang-tools-extra/clangd/Preamble.cpp @@ -104,24 +104,6 @@ private: 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 @@ -142,8 +124,15 @@ scanPreambleIncludes(llvm::StringRef Contents, "failed to create compiler invocation"); CI->getDiagnosticOpts().IgnoreWarnings = true; auto ContentsBuffer = llvm::MemoryBuffer::getMemBuffer(Contents); + // This means we're scanning (though not preprocessing) the preamble section + // twice. However, it's important to precisely follow the preamble bounds used + // elsewhere. + auto Bounds = + ComputePreambleBounds(*CI->getLangOpts(), ContentsBuffer.get(), 0); + auto PreambleContents = + llvm::MemoryBuffer::getMemBufferCopy(Contents.substr(0, Bounds.Size)); auto Clang = prepareCompilerInstance( - std::move(CI), nullptr, std::move(ContentsBuffer), + std::move(CI), nullptr, std::move(PreambleContents), // Provide an empty FS to prevent preprocessor from performing IO. This // also implies missing resolved paths for includes. new llvm::vfs::InMemoryFileSystem, IgnoreDiags); @@ -152,7 +141,7 @@ scanPreambleIncludes(llvm::StringRef Contents, "compiler instance had no inputs"); // We are only interested in main file includes. Clang->getPreprocessorOpts().SingleFileParseMode = true; - PreambleOnlyAction Action; + PreprocessOnlyAction Action; if (!Action.BeginSourceFile(*Clang, Clang->getFrontendOpts().Inputs[0])) return llvm::createStringError(llvm::inconvertibleErrorCode(), "failed BeginSourceFile"); diff --git a/clang-tools-extra/clangd/unittests/PreambleTests.cpp b/clang-tools-extra/clangd/unittests/PreambleTests.cpp index c180198..db615e6 100644 --- a/clang-tools-extra/clangd/unittests/PreambleTests.cpp +++ b/clang-tools-extra/clangd/unittests/PreambleTests.cpp @@ -118,6 +118,11 @@ TEST(PreamblePatchTest, IncludeParsing) { ^#include "a.h" #include )cpp", + // Directive is not part of preamble if it is not the token immediately + // followed by the hash (#). + R"cpp( + ^#include "a.h" + #/**/include )cpp", }; for (const auto Case : Cases) { -- 2.7.4