sammccall added a comment. Sorry about being late coming back to this.
================ Comment at: clang-tools-extra/clangd/XRefs.cpp:307 + // alias. + if (llvm::isa<UsingDecl>(D)) { + llvm::for_each(targetDecl(ast_type_traits::DynTypedNode::create(*D), ---------------- I believe this is going to do the wrong thing on overload sets. ``` namespace ns { int x(char); int x(double); } using ns::x; int y = ^x('a'); ``` getDeclAtPosition(..., TemplatePattern|Alias) will yield UsingDecl, and then targetDecl(UsingDecl, Underlying) will yield both overloads of x. However getDeclAtPosition(..., Underlying) would correctly return only the desired overload. (Please add a test like this!) This is awkward to work around, it's a failure of the targetDecl API. Maybe for now, call getDeclAtPosition with TemplatePattern|Alias, and if there's exactly one result, replace D with it? And leave a fixme to address more complicated cases somehow. ================ Comment at: clang-tools-extra/clangd/unittests/XRefsTests.cpp:695 + namespace ns { class [[Foo]] {}; } + using ns::[[F^oo]]; + )cpp", ---------------- nridge wrote: > Why is it useful to give the using-declaration itself as a result? > > It seems like the more useful behaviour from the user's point of view is to > jump directly to the definition of `class Foo`, without having to choose from > a popup first. +1, that would mean adding a continue after the addresultdecl Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74054/new/ https://reviews.llvm.org/D74054 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits