ioeric added inline comments.
================ Comment at: clangd/index/SymbolCollector.cpp:293 assert(CompletionAllocator && CompletionTUInfo); + // A declaration created for a friend declaration should not be used as the + // canonical declaration in the index. ---------------- ilya-biryukov wrote: > ioeric wrote: > > ilya-biryukov wrote: > > > Maybe move this closer to `shouldFilterDecl()`? We have similar filters > > > there. > > > That would also mean we properly add the reference counts for friend > > > declarations that get a normal declaration after their usage later. > > I didn't put this filter there because I think it's a bit more special than > > those filters in `shouldFilterDecl`. We check the `OrigD` and we could > > potentially replace `D` with `OrigD`. We could change `shouldFilterDecl` to > > handle that, but I'm not sure if it's worth it. > > > > Reference counting for friend declarations is actually a bit tricky as USRs > > of the generated declarations might be ambiguous. > > > > Consider the following exmaple: > > ``` > > namespace a { > > class A {}; > > namespace b { class B { friend class A; }; } // b > > > > } // a > > ``` > > > > I would expect the generated friend decl to be `a::A`, but it's actually > > `a::b::A`! So getting USR right is a bit tricky, and I think it's probably > > ok to ignore references in friend decls. > > > > For reference, `[namespace.memdef]p3`: > > "If the name in a friend declaration is neither qualified nor a template-id > > and the declaration is a function or an elaborated-type-specifier, the > > lookup to determine whether the entity has been previously declared shall > > not consider any scopes outside the innermost enclosing namespace. > > Reference counting for friend declarations is actually a bit tricky as USRs > > of the generated declarations might be ambiguous. > This seems like an obvious bug in the USRs that we should fix. WDYT? Sorry, I think I was not clear... I think this is intended according to the standard. So in the example, the qualified name of the friend decl `a::b::A` is, although confusing, correct, and o the actual problem is not with the USR. See `[namespace.memdef]p3`: "If the name in a friend declaration is neither qualified nor a template-id and the declaration is a function or an elaborated-type-specifier, the lookup to determine whether the entity has been previously declared shall not consider any scopes outside the innermost enclosing namespace. 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