sammccall added a comment.

Do I understand the intent of this change correctly?

- friend decls that are not definitions should be ignored for indexing purposes
- this means they should never be selected as canonical decl
- if the friend decl is the only decl, then the symbol should not be indexed

if so, that makes sense to me. I think the comments could make this a little 
clearer, but it's not too bad.



================
Comment at: clangd/index/SymbolCollector.cpp:297
+    // If OrigD is an object of a friend declaration, skip it.
+    if (ASTNode.OrigD->getFriendObjectKind() !=
+        Decl::FriendObjectKind::FOK_None)
----------------
this seems suspect, we're going to treat the third decl in `friend X; X; friend 
X` differently from that in `X; friend X; friend X;`.

Why? i.e. why is the inner check necessary and why does it treat the original 
(meaning first, I think) decl specially?


================
Comment at: unittests/clangd/SymbolCollectorTests.cpp:816
+TEST_F(SymbolCollectorTest, DoNotIndexSymbolsInFriendDecl) {
+  Annotations Header(R"(
+    namespace nx {
----------------
Can you also test that:
 - if a friend decl (non-definition) comes first, followed by a non-friend decl 
(non-definition), then the decl *is* indexed. (maybe just drop the definition 
from foo, since it's otherwise the same as Y)
 - if a friend decl has a definition, and there is no other declaration, then 
the decl *is* indexed (and the friend decl is canonical)


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D47623



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

Reply via email to