[PATCH] D48159: [clangd] Implement hover for "auto" and "decltype"

[Manuel Klimek via Phabricator via cfe-commits]

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