ilya-biryukov added a comment. In https://reviews.llvm.org/D47623#1118810, @sammccall wrote:
> - friend decls that are not definitions should be ignored for indexing > purposes This is not generally true IIUC. A friend declaration can be a reference, a declaration or a definition. int foo(int); int bar(int, int); class ExistingFwdCls; class X { friend class ExistingFwdCls; // a reference and a declaration. friend class NewClass; // a new declaration. friend int foo(int); // a reference and a declaration. friend int baz(int, int, int); // a new declaration. }; class Y { friend class ::ExistingFwdCls; // qualified => just a reference. friend int ::bar(int a, int b); // qualified => just a reference. friend int foo(int) { // a reference and a definition return 100; } }; Note that friend functions with bodies are probably ok as canonical declaration, as they are often the only declaration, e.g. class Foo { friend bool operator < (Foo const& lhs, Foo const&lhs) { return false; } }; ================ 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. ---------------- 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? 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