[PATCH] D71543: [clangd] Fix handling of inline/anon namespaces and names of deduced types in hover

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

In D71543#1785785 <https://reviews.llvm.org/D71543#1785785>, @ilya-biryukov wrote:

> My biggest concern is that we seem to make output for template instantiation worse.
>  There should be a way to stop showing anonymous namespace without introducing such regressions.


I've got D71545 <https://reviews.llvm.org/D71545> to reduce that regression.



================
Comment at: clang-tools-extra/clangd/Hover.cpp:353
 /// Generate a \p Hover object given the type \p T.
 HoverInfo getHoverContents(QualType T, const Decl *D, ASTContext &ASTCtx,
+                           const SymbolIndex *Index) {
----------------
ilya-biryukov wrote:
> Not related to this patch, but what is `D` here? Is this getting hover contents for a type or for a decl?
it represents the deduced decl for Type, if any.


================
Comment at: clang-tools-extra/clangd/unittests/HoverTests.cpp:365
        [](HoverInfo &HI) {
-         HI.Name = "class (lambda)";
+         // FIXME: Special case lambdas.
+         HI.Name = "(anonymous class)";
----------------
ilya-biryukov wrote:
> NIT: could you give an example how you want the output to look like?
See D71544


================
Comment at: clang-tools-extra/clangd/unittests/HoverTests.cpp:377
        [](HoverInfo &HI) {
-         HI.Name = "class Foo<int>";
+         HI.Name = "Foo";
          HI.Kind = index::SymbolKind::Class;
----------------
ilya-biryukov wrote:
> `Foo<int>` actually looked better. Do you consider this a regression or is this intended?
See  D71545


================
Comment at: clang-tools-extra/clangd/unittests/HoverTests.cpp:389
        [](HoverInfo &HI) {
-         HI.Name = "class Foo<int>";
+         HI.Name = "Foo<int>";
          HI.Kind = index::SymbolKind::Class;
----------------
ilya-biryukov wrote:
> Why does this give different output from the previous example?
> I would argue they should both be consistent. Users shouldn't care if there's an explicit specialization or not.
i totally agree. this one has a different output because of explicit specializations having a template pattern.
this is a temporary regression that should be fixed by D71545 (i am planning to land those patches as a whole)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71543