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

[Sam McCall via Phabricator via cfe-commits]

sammccall accepted this revision.
sammccall added a subscriber: klimek.
sammccall added a comment.

All sounds good to me.



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


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D48159