klimek accepted this revision.
klimek added inline comments.

================
Comment at: clangd/XRefs.cpp:559
+  //- auto& i = 1;
+  bool VisitDeclaratorDecl(DeclaratorDecl *D) {
+    if (!D->getTypeSourceInfo() ||
----------------
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 :)


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

Reply via email to