sammccall added inline comments.
================
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)
----------------
ioeric wrote:
> sammccall wrote:
> > 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?
> > 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;`.
> I'm not very sure if I understand the problem. But I'll try to explain what
> would happen for these two examples.
> - For the first example, the first friend decl will be the canonical decl,
> and we would only index the second `X` since its `OrigD` is not in friend
> decl. Both the first and third friend decl will not be indexed.
> - For the second example, the first non-friend `X` will be the canonical
> decl, and all three occurrences will have the same `D` pointing to it. This
> probably means that the same X will be processed three times, but it's
> probably fine (we might want to optimize it).
>
> Basically, `D` is always the canonical declaration in AST and `OrigD` is the
> declaration that the indexer is currently visiting. I agree it's confusing...
>
> > Why? i.e. why is the inner check necessary and why does it treat the
> > original (meaning first, I think) decl specially?
>
> The inner check handles the following case:
> ```
> class X {
> friend void foo();
> }
> void foo();
> ```
>
> There will be two occurrences of `foo` in the index:
> 1. The friend decl, where `D` will be the canonical decl (i.e. friend foo)
> and `OrigD` will also be friend foo.
> 2. The non-friend decl, where `D` will still be the canonical decl (i.e.
> friend foo) and `OrigD` is now the non-friend decl.
> Basically, D is always the canonical declaration in AST and OrigD is the
> declaration that the indexer is currently visiting. I agree it's confusing...
Whoops, I had this backwards. So I guess I mean the outer check.
Fundamentally my question is: in an *arbitrary* list of redecls of a symbol,
what is special about the canonical declaration (D, which is just the first in
the list) that we particularly care whether it's a friend?
I would expect that either we're ignoring the other redecls, or we're looping
over all redecls.
And here, I think we could just skip all friend decls (that are not
definitions), regardless of what `D->getFriendObjectKind()` is. If we have
other decls, the symbol was indexed already. If we do not, then we don't want
to index it anyway.
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
