[clang-tools-extra] r333369 - [clangd] Workaround the comments crash, reenable the test.

Ilya Biryukov via cfe-commits cfe-commits at lists.llvm.org
Mon May 28 02:54:51 PDT 2018


Author: ibiryukov
Date: Mon May 28 02:54:51 2018
New Revision: 333369

URL: http://llvm.org/viewvc/llvm-project?rev=333369&view=rev
Log:
[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.

Modified:
    clang-tools-extra/trunk/clangd/CodeCompletionStrings.cpp
    clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp

Modified: clang-tools-extra/trunk/clangd/CodeCompletionStrings.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CodeCompletionStrings.cpp?rev=333369&r1=333368&r2=333369&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/CodeCompletionStrings.cpp (original)
+++ clang-tools-extra/trunk/clangd/CodeCompletionStrings.cpp Mon May 28 02:54:51 2018
@@ -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 CodeComp
   }
 }
 
-std::string getFormattedComment(const ASTContext &Ctx, const RawComment &RC,
-                                bool CommentsFromHeaders) {
+bool canRequestComment(const ASTContext &Ctx, const NamedDecl &D,
+                       bool CommentsFromHeaders) {
+  if (CommentsFromHeaders)
+    return true;
   auto &SourceMgr = Ctx.getSourceManager();
-  // Parsing comments 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.
+  // 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.
-  if (!CommentsFromHeaders && !SourceMgr.isWrittenInMainFile(RC.getLocStart()))
-    return "";
-  return RC.getFormattedText(Ctx.getSourceManager(), Ctx.getDiagnostics());
+  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);
 }
 } // namespace
 
@@ -145,26 +161,26 @@ std::string getDocComment(const ASTConte
   // get this declaration, so we don't show documentation in that case.
   if (Result.Kind != CodeCompletionResult::RK_Declaration)
     return "";
-  auto Decl = Result.getDeclaration();
-  if (!Decl)
+  auto *Decl = Result.getDeclaration();
+  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();
-  if (!Func)
+  auto *Func = Result.getFunction();
+  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,

Modified: clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp?rev=333369&r1=333368&r2=333369&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp Mon May 28 02:54:51 2018
@@ -1000,7 +1000,7 @@ TEST(CompletionTest, NoIndexCompletionsI
 }
 
 // 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");




More information about the cfe-commits mailing list