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

Reply via email to