ilya-biryukov marked an inline comment as done.
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:
> > > > 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.
Sorry, I missed that we still have the `Type` field set which will contain all
pointers/references.
We don't seem to loose any information in that case, it's really up to the
presentation layer to figure it out. LG
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
