[clangd] Workaround the comments crash, reenable the test.

This fix is still quite fragile, the underlying problem is that the
code should not rely on source ranges coming from the preamble to be
correct when reading from the text buffers.
This is probably not possible to achieve in practice, so we would
probably have to keep the contents of old headers around for the
lifetime of the preamble.

llvm-svn: 333369

clang-tools-extra/clangd/CodeCompletionStrings.cpp

@@ -9,6 +9,7 @@
 
 #include "CodeCompletionStrings.h"
 #include "clang/AST/ASTContext.h"
+#include "clang/AST/DeclObjC.h"
 #include "clang/AST/RawCommentList.h"
 #include "clang/Basic/SourceManager.h"
 #include <utility>
@@ -123,17 +124,32 @@ void processSnippetChunks(const CodeCompletionString &CCS,
   }
 }
 
-std::string getFormattedComment(const ASTContext &Ctx, const RawComment &RC,
+bool canRequestComment(const ASTContext &Ctx, const NamedDecl &D,
-                                bool CommentsFromHeaders) {
+                       bool CommentsFromHeaders) {
+  if (CommentsFromHeaders)
+    return true;
   auto &SourceMgr = Ctx.getSourceManager();
-  // Parsing comments from invalid preamble can lead to crashes. So we only
+  // Accessing comments for decls from  invalid preamble can lead to crashes.
-  // return comments from the main file when doing code completion. For
+  // So we only return comments from the main file when doing code completion.
-  // indexing, we still read all the comments.
+  // 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.
-  if (!CommentsFromHeaders && !SourceMgr.isWrittenInMainFile(RC.getLocStart()))
+  auto canRequestForDecl = [&](const NamedDecl &D) -> bool {
-    return "";
+    for (auto *Redecl : D.redecls()) {
-  return RC.getFormattedText(Ctx.getSourceManager(), Ctx.getDiagnostics());
+      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);
 }
 } // namespace
 
@@ -145,26 +161,26 @@ std::string getDocComment(const ASTContext &Ctx,
   // get this declaration, so we don't show documentation in that case.
   if (Result.Kind != CodeCompletionResult::RK_Declaration)
     return "";
-  auto Decl = Result.getDeclaration();
+  auto *Decl = Result.getDeclaration();
-  if (!Decl)
+  if (!Decl || !canRequestComment(Ctx, *Decl, CommentsFromHeaders))
     return "";
   const RawComment *RC = getCompletionComment(Ctx, Decl);
   if (!RC)
     return "";
-  return getFormattedComment(Ctx, *RC, CommentsFromHeaders);
+  return RC->getFormattedText(Ctx.getSourceManager(), Ctx.getDiagnostics());
 }
 
 std::string
 getParameterDocComment(const ASTContext &Ctx,
                        const CodeCompleteConsumer::OverloadCandidate &Result,
                        unsigned ArgIndex, bool CommentsFromHeaders) {
-  auto Func = Result.getFunction();
+  auto *Func = Result.getFunction();
-  if (!Func)
+  if (!Func || !canRequestComment(Ctx, *Func, CommentsFromHeaders))
     return "";
   const RawComment *RC = getParameterComment(Ctx, Result, ArgIndex);
   if (!RC)
     return "";
-  return getFormattedComment(Ctx, *RC, CommentsFromHeaders);
+  return RC->getFormattedText(Ctx.getSourceManager(), Ctx.getDiagnostics());
 }
 
 void getLabelAndInsertText(const CodeCompletionString &CCS, std::string *Label,

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

@@ -1000,7 +1000,7 @@ TEST(CompletionTest, NoIndexCompletionsInsideDependentCode) {
 }
 
 // FIXME: This still crashes under asan. Fix it and reenable the test.
-TEST(CompletionTest, DISABLED_DocumentationFromChangedFileCrash) {
+TEST(CompletionTest, DocumentationFromChangedFileCrash) {
   MockFSProvider FS;
   auto FooH = testPath("foo.h");
   auto FooCpp = testPath("foo.cpp");