kadircet added inline comments.

================
Comment at: clang-tools-extra/clangd/FindTarget.cpp:415
 
 Optional<ReferenceLoc> refInDecl(const Decl *D) {
   struct Visitor : ConstDeclVisitor<Visitor> {
----------------
ilya-biryukov wrote:
> kadircet wrote:
> > ilya-biryukov wrote:
> > > hokein wrote:
> > > > ilya-biryukov wrote:
> > > > > This should return `SmallVector<ReferenceLoc, 2>` now, some 
> > > > > declarations can have both decl and non-decl references.
> > > > Can you give some examples? It seems that non-decl references are 
> > > > handled by other `refIn*` functions.
> > > > 
> > > > ```
> > > > int Foo = OtherBar; // OtherBar is captured at the momment.
> > > > ```
> > > ```
> > > namespace $1[[a]] = $2[[std]];
> > > ```
> > > 
> > > `$1` is a declaration of a namespace alias.
> > > `$2` is a reference to namespace std.
> > I was just about to make the same point. After this patch lands I would 
> > like to detect `using namespace`s inside function body, and currently there 
> > is no way to distinguish between a usingdirective and a namespacealias. 
> > Since both is only referencing the target namespace.
> > 
> > So even if this starts reporting `$2` in the comment above, there should be 
> > a difference between that and `$3` in `using namespace $3[[std]];`
> That's intentional, though. When we report references, we deliberately leave 
> out information on where it's coming from - otherwise it's hard to encode it.
> 
> However, if we really need that, we could also pass a `NamedDecl*` when 
> reporting declarations (that would mean a slightly different interface, 
> though).
> 
> We still have some complexity budget in `findExplicitReferences` and we 
> should probably spend it here.
> OTOH, adding more stuff will probably bloat it too much, I hope we'll stop 
> there... There will still be cases when one would have to write a separate 
> AST traversal to do more complicated stuff. It's unfortunate that this is 
> twice as expensive, but it shouldn't matter in most cases (e.g. when running 
> inside `Tweak::apply`)
> 
> I'd suggest we keep this patch with `IsDecl` flag, but happy to look into 
> your use-case with `using namespaces` more closely...
as discussed offline, it is a special enough use-case to handle in 
define-inline.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68977/new/

https://reviews.llvm.org/D68977



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

Reply via email to