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

Reply via email to