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