[clangd] Change PreambleOnlyAction with content truncation

[kadircet]

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.

[kadircet]

• Add test

[sammccall]

clang-tools-extra/clangd/Preamble.cpp

127

Maybe add a comment here:

// This means we're scanning (though not preprocessing) the preamble section twice.
// However, it's important to precisely follow the preamble bounds used elsewhere.

This guards somewhat against someone sending the reverse patch. Chesterton's fence etc.

clang-tools-extra/clangd/Preamble.cpp

private:
   IncludeStructure Includes;
   CanonicalIncludes CanonIncludes;
   MainFileMacros Macros;
   std::unique_ptr<CommentHandler> IWYUHandler = nullptr;
   const clang::LangOptions *LangOpts = nullptr;
   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);
+  // 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);
   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;
+  PreprocessOnlyAction 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())

clang-tools-extra/clangd/unittests/PreambleTests.cpp

llvm::StringRef Cases[] = {
         ^#import "a.h"
         #endif)cpp",
       // Broken directives
       R"cpp(
         #include "a
         ^#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) {
     Annotations Test(Case);
     const auto Code = Test.code();
     SCOPED_TRACE(Code);
 
     auto Includes =