[PATCH] D62814: [clangd] Treat lambdas as functions when preparing hover response

[Ilya Biryukov via Phabricator via cfe-commits]

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