[PATCH] D48943: [clangd] Do not write comments into Preamble PCH

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE336540: [clangd] Do not write comments into Preamble PCH (authored by ibiryukov, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D48943?vs=154569&id=154572#toc

Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D48943

Files:
  clangd/ClangdUnit.cpp
  clangd/CodeCompletionStrings.cpp


Index: clangd/ClangdUnit.cpp
===================================================================
--- clangd/ClangdUnit.cpp
+++ 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 @@
   // 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)) {
Index: clangd/CodeCompletionStrings.cpp
===================================================================
--- clangd/CodeCompletionStrings.cpp
+++ clangd/CodeCompletionStrings.cpp
@@ -32,34 +32,6 @@
   }
 }
 
-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 @@
     // 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 @@
                        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 "";


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D48943.154572.patch
Type: text/x-patch
Size: 3909 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20180709/b670ed95/attachment.bin>