Author: Kadir Cetinkaya Date: 2020-05-26T07:37:03+02:00 New Revision: 34e39eb2adc2b3f16c2c2c0607a904ee55705c01
URL: https://github.com/llvm/llvm-project/commit/34e39eb2adc2b3f16c2c2c0607a904ee55705c01 DIFF: https://github.com/llvm/llvm-project/commit/34e39eb2adc2b3f16c2c2c0607a904ee55705c01.diff LOG: [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 Added: Modified: clang-tools-extra/clangd/Preamble.cpp clang-tools-extra/clangd/unittests/PreambleTests.cpp Removed: ################################################################################ diff --git a/clang-tools-extra/clangd/Preamble.cpp b/clang-tools-extra/clangd/Preamble.cpp index 9d9c5eff8c68..d3eaa92d4c1a 100644 --- a/clang-tools-extra/clangd/Preamble.cpp +++ b/clang-tools-extra/clangd/Preamble.cpp @@ -104,24 +104,6 @@ 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 @@ -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 c1801980b1d5..db615e6e66e1 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 <b ^#include <b.h>)cpp", + // Directive is not part of preamble if it is not the token immediately + // followed by the hash (#). + R"cpp( + ^#include "a.h" + #/**/include <b.h>)cpp", }; for (const auto Case : Cases) { _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
