Author: sammccall
Date: Mon Oct 7 03:53:56 2019
New Revision: 373892
URL: http://llvm.org/viewvc/llvm-project?rev=373892&view=rev
Log:
[clangd] If an undocumented definition exists, don't accept documentation from
other forward decls.
Summary:
This fixes cases like:
foo.h
class Undocumented{}
bar.h
// break an include cycle. we should refactor this!
class Undocumented;
Where the comment doesn't describe the class.
Note that a forward decl that is *visible to the definition* will still have
its doc comment used, by SymbolCollector: Merge isn't involved here.
Reviewers: ilya-biryukov
Subscribers: MaskRay, jkorous, arphaman, kadircet, usaxena95, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D68467
Modified:
clang-tools-extra/trunk/clangd/index/Merge.cpp
clang-tools-extra/trunk/clangd/unittests/IndexTests.cpp
Modified: clang-tools-extra/trunk/clangd/index/Merge.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/Merge.cpp?rev=373892&r1=373891&r2=373892&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/index/Merge.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/Merge.cpp Mon Oct 7 03:53:56 2019
@@ -186,7 +186,10 @@ Symbol mergeSymbol(const Symbol &L, cons
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;
Modified: clang-tools-extra/trunk/clangd/unittests/IndexTests.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/unittests/IndexTests.cpp?rev=373892&r1=373891&r2=373892&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/unittests/IndexTests.cpp (original)
+++ clang-tools-extra/trunk/clangd/unittests/IndexTests.cpp Mon Oct 7 03:53:56
2019
@@ -413,6 +413,16 @@ TEST(MergeIndexTest, Refs) {
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);
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
