kadircet added inline comments.
================
Comment at: clang-tools-extra/clangd/unittests/XRefsTests.cpp:831
+ };
return HI;
}},
----------------
ilya-biryukov wrote:
> 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.
I see, but then I don't think this case has anything to do with current patch,
right?
It becomes a matter of decomposing a type with sugared components(which I
believe should be visited but not in this patch) rather than expanding a lambda
to a function like.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D62814/new/
https://reviews.llvm.org/D62814
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
