malaperle added inline comments.
================
Comment at: clangd/XRefs.cpp:559
+ //- auto& i = 1;
+ bool VisitDeclaratorDecl(DeclaratorDecl *D) {
+ if (!D->getTypeSourceInfo() ||
----------------
klimek wrote:
> klimek wrote:
> > malaperle wrote:
> > > klimek wrote:
> > > > sammccall wrote:
> > > > > malaperle wrote:
> > > > > > sammccall wrote:
> > > > > > > out of curiosity, why not implement `VisitTypeLoc` and handle all
> > > > > > > the cases where it turns out to be `auto` etc?
> > > > > > > Even for `auto&` I'd expect the inner `auto` to have a `TypeLoc`
> > > > > > > you could visit, saving the trouble of unwrapping.
> > > > > > >
> > > > > > > (I'm probably wrong about all this, I don't know the AST well.
> > > > > > > But I'd like to learn!)
> > > > > > From what I saw, there are actually two different AutoType* for
> > > > > > each textual "auto". The AutoType* containing the deduced type does
> > > > > > not get visited via a typeloc. It's not entirely clear to me why
> > > > > > since I don't know the AST well either. I was thinking maybe the
> > > > > > first is created when the type is not deduced yet and later on,
> > > > > > then the rest of the function or expression is parsed, a second one
> > > > > > with the actual type deduced is created. If I look at the code
> > > > > > paths where they are created, it seems like this is roughly what's
> > > > > > happening. The first one is created when the declarator is parsed
> > > > > > (no deduced type yet) and the second is created when the expression
> > > > > > of the initializer (or return statement) is evaluated and the type
> > > > > > is then deduced. The visitor only visits the first one's typeloc. I
> > > > > > don't think I'm knowledgeable enough to say whether or not that's a
> > > > > > bug but it seems on purpose that it is modelled this way. Although
> > > > > > it would be much nicer to only have to visit typelocs...
> > > > > > The AutoType* containing the deduced type does not get visited via
> > > > > > a typeloc
> > > > > Ah, OK.
> > > > > Could you add a high level comment (maybe on the class) saying this
> > > > > is the reason for the implementation? Otherwise as a reader I'll
> > > > > think "this seems unneccesarily complicated" but not understand why.
> > > > >
> > > > > @klimek Can you shed any light on this?
> > > > Can't you go from AutoTypeLoc -> AutoType -> getDeducedType()?
> > > The visitor doesn't visit the AutoTypeLoc that has the deduced type. In
> > > fact, there are two AutoType* instances. I'm not sure that's is a bug
> > > that there are two AutoType*, or if not visiting both AutoTypeLoc is a
> > > bug...or neither.
> > +Richard Smith:
> >
> > This is weird. If I just create a minimal example:
> > int f() {
> > auto i = f();
> > return i;
> > }
> >
> > I only get the undeduced auto type - Richard, in which cases are auto-typed
> > being deduced? The AST dump doens't give an indication that there was an
> > auto involved at all. Is this the famous syntactic vs. smenatic form
> > problem? Do we have a backlink between the AutoTypeLoc and the deduced type
> > somewhere?
> Given that Richard is known to have ~1 month ping times now and then I think
> it's fine to land this with a FIXME above to figure out how to represent this
> better in the AST. I'd still say it's a missing feature in the AST :)
Thanks! I'm looking forward to simplifying this code when the AST is improved.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D48159
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
