From 6f33b330ae1808ab2a5d7184abafefc5d5821059 Mon Sep 17 00:00:00 2001
From: Ilya Biryukov <ibiryukov@google.com>
Date: Mon, 9 Jul 2018 11:33:31 +0000
Subject: [PATCH] [clangd] Do not write comments into Preamble PCH

Summary:
To avoid wasting time deserializing them on code completion and
further reparses.

We do not use the comments anyway, because we cannot rely on the file
contents staying the same for reparses that reuse the prebuilt
preamble PCH.

Reviewers: sammccall

Reviewed By: sammccall

Subscribers: ioeric, MaskRay, jkorous, cfe-commits

Differential Revision: https://reviews.llvm.org/D48943

llvm-svn: 336540
---
 clang-tools-extra/clangd/ClangdUnit.cpp            |  4 +++
 clang-tools-extra/clangd/CodeCompletionStrings.cpp | 40 +++++-----------------
 2 files changed, 12 insertions(+), 32 deletions(-)

diff --git a/clang-tools-extra/clangd/ClangdUnit.cpp b/clang-tools-extra/clangd/ClangdUnit.cpp
index a6ec193..d91be1e 100644
--- a/clang-tools-extra/clangd/ClangdUnit.cpp
+++ b/clang-tools-extra/clangd/ClangdUnit.cpp
@@ -24,6 +24,7 @@
 #include "clang/Lex/Lexer.h"
 #include "clang/Lex/MacroInfo.h"
 #include "clang/Lex/Preprocessor.h"
+#include "clang/Lex/PreprocessorOptions.h"
 #include "clang/Sema/Sema.h"
 #include "clang/Serialization/ASTWriter.h"
 #include "clang/Tooling/CompilationDatabase.h"
@@ -323,6 +324,9 @@ std::shared_ptr<const PreambleData> clangd::buildPreamble(
   // the preamble and make it smaller.
   assert(!CI.getFrontendOpts().SkipFunctionBodies);
   CI.getFrontendOpts().SkipFunctionBodies = true;
+  // We don't want to write comment locations into PCH. They are racy and slow
+  // to read back. We rely on dynamic index for the comments instead.
+  CI.getPreprocessorOpts().WriteCommentListToPCH = false;
 
   CppFilePreambleCallbacks SerializedDeclsCollector(FileName, PreambleCallback);
   if (Inputs.FS->setCurrentWorkingDirectory(Inputs.CompileCommand.Directory)) {
diff --git a/clang-tools-extra/clangd/CodeCompletionStrings.cpp b/clang-tools-extra/clangd/CodeCompletionStrings.cpp
index 6bad0ea..474419d 100644
--- a/clang-tools-extra/clangd/CodeCompletionStrings.cpp
+++ b/clang-tools-extra/clangd/CodeCompletionStrings.cpp
@@ -32,34 +32,6 @@ void appendEscapeSnippet(const llvm::StringRef Text, std::string *Out) {
   }
 }
 
-bool canRequestComment(const ASTContext &Ctx, const NamedDecl &D,
-                       bool CommentsFromHeaders) {
-  if (CommentsFromHeaders)
-    return true;
-  auto &SourceMgr = Ctx.getSourceManager();
-  // Accessing comments for decls from  invalid preamble can lead to crashes.
-  // So we only return comments from the main file when doing code completion.
-  // For indexing, we still read all the comments.
-  // FIXME: find a better fix, e.g. store file contents in the preamble or get
-  // doc comments from the index.
-  auto canRequestForDecl = [&](const NamedDecl &D) -> bool {
-    for (auto *Redecl : D.redecls()) {
-      auto Loc = SourceMgr.getSpellingLoc(Redecl->getLocation());
-      if (!SourceMgr.isWrittenInMainFile(Loc))
-        return false;
-    }
-    return true;
-  };
-  // First, check the decl itself.
-  if (!canRequestForDecl(D))
-    return false;
-  // Completion also returns comments for properties, corresponding to ObjC
-  // methods.
-  const ObjCMethodDecl *M = dyn_cast<ObjCMethodDecl>(&D);
-  const ObjCPropertyDecl *PDecl = M ? M->findPropertyDecl() : nullptr;
-  return !PDecl || canRequestForDecl(*PDecl);
-}
-
 bool LooksLikeDocComment(llvm::StringRef CommentText) {
   // We don't report comments that only contain "special" chars.
   // This avoids reporting various delimiters, like:
@@ -87,12 +59,13 @@ std::string getDocComment(const ASTContext &Ctx,
     // the comments for namespaces.
     return "";
   }
-  if (!canRequestComment(Ctx, *Decl, CommentsFromHeaders))
-    return "";
-
   const RawComment *RC = getCompletionComment(Ctx, Decl);
   if (!RC)
     return "";
+
+  // Sanity check that the comment does not come from the PCH. We choose to not
+  // write them into PCH, because they are racy and slow to load.
+  assert(!Ctx.getSourceManager().isLoadedSourceLocation(RC->getLocStart()));
   std::string Doc = RC->getFormattedText(Ctx.getSourceManager(), Ctx.getDiagnostics());
   if (!LooksLikeDocComment(Doc))
     return "";
@@ -104,11 +77,14 @@ getParameterDocComment(const ASTContext &Ctx,
                        const CodeCompleteConsumer::OverloadCandidate &Result,
                        unsigned ArgIndex, bool CommentsFromHeaders) {
   auto *Func = Result.getFunction();
-  if (!Func || !canRequestComment(Ctx, *Func, CommentsFromHeaders))
+  if (!Func)
     return "";
   const RawComment *RC = getParameterComment(Ctx, Result, ArgIndex);
   if (!RC)
     return "";
+  // Sanity check that the comment does not come from the PCH. We choose to not
+  // write them into PCH, because they are racy and slow to load.
+  assert(!Ctx.getSourceManager().isLoadedSourceLocation(RC->getLocStart()));
   std::string Doc = RC->getFormattedText(Ctx.getSourceManager(), Ctx.getDiagnostics());
   if (!LooksLikeDocComment(Doc))
     return "";
-- 
2.7.4