[PATCH] D79426: [clangd] Change PreambleOnlyAction with content truncation

[Kadir Cetinkaya via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG34e39eb2adc2: [clangd] Change PreambleOnlyAction with content truncation (authored by kadircet).

Changed prior to commit:
  https://reviews.llvm.org/D79426?vs=262132&id=266107#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D79426/new/

https://reviews.llvm.org/D79426

Files:
  clang-tools-extra/clangd/Preamble.cpp
  clang-tools-extra/clangd/unittests/PreambleTests.cpp


Index: clang-tools-extra/clangd/unittests/PreambleTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/PreambleTests.cpp
+++ clang-tools-extra/clangd/unittests/PreambleTests.cpp
@@ -118,6 +118,11 @@
         ^#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) {
Index: clang-tools-extra/clangd/Preamble.cpp
===================================================================
--- clang-tools-extra/clangd/Preamble.cpp
+++ clang-tools-extra/clangd/Preamble.cpp
@@ -104,24 +104,6 @@
   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 @@
                                    "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 @@
                                    "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");


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D79426.266107.patch
Type: text/x-patch
Size: 3155 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20200526/e270e9ca/attachment-0001.bin>