[PATCH] D114522: [clangd] Add desugared type to hover

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

> My main remaining concern is just that this will trigger too often when there's no confusion over types, and clutter the display.
> Some common cases I'm worried about:
>
> - std::string -> basic_string<char>. Maybe we should special-case this to hide the aka?
> - int64_t vs long long et al. I have no idea what to do about this.

As discussed offline I share these concerns, and it's not the first time we're facing them. I am afraid that this might be disturbing for a good fraction of users if we don't polish it around common types.
A couple of suggestions I have (they don't necessarily need to land as part of this patch, but I think we should land some damage control mechanism before next release):

- Make `AKA-printing` completely optional, to give users a way out (while probably starting with off by default and experimenting for a while to see the most annoying cases and polishing them as needed)
- Have a list of user-configurable FQNs that AKA printing would be disabled for (and provide a good set of defaults inside clangd for STL)



================
Comment at: clang-tools-extra/clangd/Hover.cpp:1271
     OS << " = " << *P.Default;
+  if (P.Type && P.Type->AKA)
+    OS << llvm::formatv(" (aka {0})", *P.Type->AKA);
----------------
sammccall wrote:
> Hmm, this seems to render as:
> 
> ```std::string Foo = "" (aka basic_string<char>)```
> 
> it's not *completely* clear what the aka refers to.
> I don't have a better solution, though.
> @kadircet any opinions here?
i think it's attached to the type of the parameter, not it's name nor the initializer-expr. so I think we should move this closer to the type (i.e. `std::string (aka basic_sting<char>) Foo = ""`, basically `llvm::toString(P.Type)` ?)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D114522/new/

https://reviews.llvm.org/D114522