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