ilya-biryukov added inline comments.
================ Comment at: clang-tools-extra/clangd/unittests/XRefsTests.cpp:831 + }; return HI; }}, ---------------- kadircet wrote: > ilya-biryukov wrote: > > kadircet wrote: > > > ilya-biryukov wrote: > > > > Could you add another test with even weirder types where we fail to > > > > show the signature? To make sure we don't break when reaching the > > > > limitations of the chosen approach and document what those limitations > > > > are. > > > > > > > > Something like: > > > > ``` > > > > auto a = [](int a) { return 10; }; > > > > auto *b = &a; > > > > auto *c = &b; > > > > ``` > > > > > > > > We would fail to show the signature here, but it's totally ok to ignore > > > > it. > > > added cases, and changed code(a lot simpler now) to generate signatures > > > for those cases as well. > > Here's an example when the new approach falls short too: > > > > ``` > > auto a = [](int) { return 10; } > > std::function<void(decltype(a) x)> b; > > ``` > > > > In general, are we ok with loosing all the information about the type that > > we drop? > > One level of references and pointers seemed ok, dropping more is a bit more > > cheesy.. > > > > At the same time, either case is **so** rare that we probably don't care. > are you talking about hovering over `x` ? I don't think AST contains > information regarding that one. > > for a code like this: > ``` > auto foo = []() { return 5; }; > > template <class T> > class Cls {}; > > Cls<void(decltype(foo) bar)> X; > ``` > > This is the AST dump for variable X: > ``` > `-VarDecl 0x2b0e808 <line:6:1, col:30> col:30 X 'Cls<void > (decltype(foo))>':'Cls<void ((lambda at a.cc:1:12))>' callinit > `-CXXConstructExpr 0x2b12e80 <col:30> 'Cls<void (decltype(foo))>':'Cls<void > ((lambda at a.cc:1:12))>' 'void () noexcept' > ``` I'm talking about hovering over `b` and, as Sam mentioned, there's a good chance you don't have this information in the type and we need to look at `TypeLocs` instead. Also agree with Sam, we don't want **any** complexity for that case. Just wanted to make sure we added a test like this just to make sure we have some idea of what's produced there and it does not crash. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62814/new/ https://reviews.llvm.org/D62814 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits