[PATCH] D68467: [clangd] If an undocumented definition exists, don't accept documentation from other forward decls.

[Sam McCall via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL373892: [clangd] If an undocumented definition exists, 
don't accept documentation from… (authored by sammccall, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D68467?vs=223224&id=223732#toc

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68467/new/

https://reviews.llvm.org/D68467

Files:
  clang-tools-extra/trunk/clangd/index/Merge.cpp
  clang-tools-extra/trunk/clangd/unittests/IndexTests.cpp


Index: clang-tools-extra/trunk/clangd/unittests/IndexTests.cpp
===================================================================
--- clang-tools-extra/trunk/clangd/unittests/IndexTests.cpp
+++ clang-tools-extra/trunk/clangd/unittests/IndexTests.cpp
@@ -413,6 +413,16 @@
                                        FileURI("unittest:///test2.cc"))))));
 }
 
+TEST(MergeIndexTest, NonDocumentation) {
+  Symbol L, R;
+  L.ID = R.ID = SymbolID("x");
+  L.Definition.FileURI = "file:/x.h";
+  R.Documentation = "Forward declarations because x.h is too big to include";
+
+  Symbol M = mergeSymbol(L, R);
+  EXPECT_EQ(M.Documentation, "");
+}
+
 MATCHER_P2(IncludeHeaderWithRef, IncludeHeader, References, "") {
   return (arg.IncludeHeader == IncludeHeader) && (arg.References == 
References);
 }
Index: clang-tools-extra/trunk/clangd/index/Merge.cpp
===================================================================
--- clang-tools-extra/trunk/clangd/index/Merge.cpp
+++ clang-tools-extra/trunk/clangd/index/Merge.cpp
@@ -186,7 +186,10 @@
     S.Signature = O.Signature;
   if (S.CompletionSnippetSuffix == "")
     S.CompletionSnippetSuffix = O.CompletionSnippetSuffix;
-  if (S.Documentation == "")
+  // Don't accept documentation from bare forward declarations, if there is a
+  // definition and it didn't provide one. S is often an undocumented class,
+  // and O is a non-canonical forward decl preceded by an irrelevant comment.
+  if (S.Documentation == "" && !S.Definition)
     S.Documentation = O.Documentation;
   if (S.ReturnType == "")
     S.ReturnType = O.ReturnType;



Index: clang-tools-extra/trunk/clangd/unittests/IndexTests.cpp
===================================================================
--- clang-tools-extra/trunk/clangd/unittests/IndexTests.cpp
+++ clang-tools-extra/trunk/clangd/unittests/IndexTests.cpp
@@ -413,6 +413,16 @@
                                        FileURI("unittest:///test2.cc"))))));
 }
 
+TEST(MergeIndexTest, NonDocumentation) {
+  Symbol L, R;
+  L.ID = R.ID = SymbolID("x");
+  L.Definition.FileURI = "file:/x.h";
+  R.Documentation = "Forward declarations because x.h is too big to include";
+
+  Symbol M = mergeSymbol(L, R);
+  EXPECT_EQ(M.Documentation, "");
+}
+
 MATCHER_P2(IncludeHeaderWithRef, IncludeHeader, References, "") {
   return (arg.IncludeHeader == IncludeHeader) && (arg.References == References);
 }
Index: clang-tools-extra/trunk/clangd/index/Merge.cpp
===================================================================
--- clang-tools-extra/trunk/clangd/index/Merge.cpp
+++ clang-tools-extra/trunk/clangd/index/Merge.cpp
@@ -186,7 +186,10 @@
     S.Signature = O.Signature;
   if (S.CompletionSnippetSuffix == "")
     S.CompletionSnippetSuffix = O.CompletionSnippetSuffix;
-  if (S.Documentation == "")
+  // Don't accept documentation from bare forward declarations, if there is a
+  // definition and it didn't provide one. S is often an undocumented class,
+  // and O is a non-canonical forward decl preceded by an irrelevant comment.
+  if (S.Documentation == "" && !S.Definition)
     S.Documentation = O.Documentation;
   if (S.ReturnType == "")
     S.ReturnType = O.ReturnType;

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits