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

[kadircet]

Summary

Clangd normally skips inline and anon namespaces while printing nested name
specifiers. It also drops any tag specifiers since we make use of HoverInfo::Kind
instead of some text in HoverInfo::Name

There was a bug causing us to print innermost inline/anon namespace, this patch
fixes that by skipping those.
Also changes printing and kind detection of deduced types to be similar to decl
case.

[merge_guards_bot]

Unit tests: pass. 60932 tests passed, 0 failed and 726 were skipped.

clang-format: pass.

Build artifacts: diff.json, CMakeCache.txt, console-log.txt, test-results.xml

[ilya-biryukov]

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.

clang-tools-extra/clangd/Hover.cpp

353

Not related to this patch, but what is D here? Is this getting hover contents for a type or for a decl?

356

NIT: could be simplified to

if (!D)
  D = T->getAsTagDecl();
if (!D)
  return HI;

// ... body of FillInHover goes here

clang-tools-extra/clangd/unittests/HoverTests.cpp

365

NIT: could you give an example how you want the output to look like?

377

Foo<int> actually looked better. Do you consider this a regression or is this intended?

389

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.

533

What if anon/inline namespace are interleaves with named ones?
What would it print?

namespace a { inline namespace inl {  namespace b { namespace { namespace c { namespace {
struct X {};
}}}}}}

Could we test this?

1196

This looks like a regression. What's stopping us from fixing this right away?

[kadircet]

• Address comments

[kadircet]

In 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 to reduce that regression.

clang-tools-extra/clangd/Hover.cpp

353

it represents the deduced decl for Type, if any.

clang-tools-extra/clangd/unittests/HoverTests.cpp

365

See D71544

377

See D71545

389

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)

[merge_guards_bot]

Unit tests: fail. 60930 tests passed, 2 failed and 726 were skipped.

failed: libc++.std/thread/thread_mutex/thread_mutex_requirements/thread_mutex_requirements_mutex/thread_mutex_recursive/lock.pass.cpp
failed: libc++.std/thread/thread_mutex/thread_mutex_requirements/thread_mutex_requirements_mutex/thread_mutex_recursive/try_lock.pass.cpp

clang-format: pass.

Build artifacts: diff.json, CMakeCache.txt, console-log.txt, test-results.xml

[ilya-biryukov]

In D71543#1785842, @kadircet wrote:

I've got D71545 to reduce that regression.

D71545 seems to be pretty small, yet depends on this change. Maybe add it right here?
Otherwise clangd is in a kinda broken state between those two commits, not a very good state to be at.
Would also make it simpler to revert the change in case something bad happens.

[ilya-biryukov]

clang-tools-extra/clangd/Hover.cpp

353

What is a "deduced decl for Type"?

clang-tools-extra/clangd/unittests/HoverTests.cpp

365

D71544 is trivial, could you put it into this patch?
Having a FIXME that is fixed in the follow-up with a one-line change seems to potentially complicate things (reverts, reading change history).

[kadircet]

clang-tools-extra/clangd/Hover.cpp

353

it was referring to the tagdecl referred by decltypes, i am not sure if there are cases in which this can be different than T->getAsTagDecl.
Always making use of T->getAsTagDecl doesn't seem to be causing any test failures.

[kadircet]

• Squash follow-ups

[kadircet]

• Delete fixme

[merge_guards_bot]

Unit tests: pass. 60932 tests passed, 0 failed and 726 were skipped.

clang-tidy: pass.

clang-format: pass.

Build artifacts: diff.json, clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

[merge_guards_bot]

Unit tests: pass. 60932 tests passed, 0 failed and 726 were skipped.

clang-tidy: pass.

clang-format: pass.

Build artifacts: diff.json, clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

[ilya-biryukov]

clang-tools-extra/clangd/Hover.cpp

353

if T->getAsTagDecl() is non-null, why does D being passed to the function is null?
How can they be different?

[ilya-biryukov]

clang-tools-extra/clangd/Hover.cpp

382

NIT: is flush redundant? I believe it's called in destructor

434

Is this the only callsite of getHoverContents that we are changin?
We could just move the logic that computes D into the getHoverContents.

[kadircet]

• Address comments
• Get rid of deduced decl in decltype/auto case

clang-tools-extra/clangd/Hover.cpp

353

see D71597

[kadircet]

• Rebase

[ilya-biryukov]

LGTM

[merge_guards_bot]

Unit tests: fail. 60977 tests passed, 1 failed and 727 were skipped.

failed: lit.lit/shtest-format.py

clang-tidy: unknown.

clang-format: unknown.

Build artifacts: diff.json, CMakeCache.txt, console-log.txt, test-results.xml

[merge_guards_bot]

Unit tests: fail. 60978 tests passed, 1 failed and 727 were skipped.

failed: lit.lit/shtest-format.py

clang-tidy: unknown.

clang-format: unknown.

Build artifacts: diff.json, CMakeCache.txt, console-log.txt, test-results.xml

clang-tools-extra/clangd/AST.cpp

 //===--- AST.cpp - Utility AST functions  -----------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "AST.h"
 
 #include "SourceCode.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclBase.h"
+#include "clang/AST/DeclCXX.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/AST/DeclarationName.h"
 #include "clang/AST/NestedNameSpecifier.h"
 #include "clang/AST/PrettyPrinter.h"
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/AST/TemplateBase.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"

clang-tools-extra/clangd/Hover.cpp

 #include "FormattedString.h"
 #include "Logger.h"
 #include "Selection.h"
 #include "SourceCode.h"
 #include "index/SymbolCollector.h"
 
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/ASTTypeTraits.h"
+#include "clang/AST/Decl.h"
+#include "clang/AST/DeclBase.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/AST/PrettyPrinter.h"
 #include "clang/Index/IndexSymbol.h"
+#include "llvm/Support/Casting.h"
 #include "llvm/Support/raw_ostream.h"
 
 namespace clang {
 namespace clangd {
 namespace {
 
 PrintingPolicy printingPolicyForDecls(PrintingPolicy Base) {
   PrintingPolicy Policy(Base);
std::string getLocalScope(const Decl *D) {
   return llvm::join(llvm::reverse(Scopes), "::");
 }
 
 /// Returns the human-readable representation for namespace containing the
 /// declaration \p D. Returns empty if it is contained global namespace.
 std::string getNamespaceScope(const Decl *D) {
   const DeclContext *DC = D->getDeclContext();
 
-  if (const TypeDecl *TD = dyn_cast<TypeDecl>(DC))
+  if (const TagDecl *TD = dyn_cast<TagDecl>(DC))
     return getNamespaceScope(TD);
   if (const FunctionDecl *FD = dyn_cast<FunctionDecl>(DC))
     return getNamespaceScope(FD);
+  if (const NamespaceDecl *NSD = dyn_cast<NamespaceDecl>(DC)) {
+    // Skip inline/anon namespaces.
+    if (NSD->isInline() || NSD->isAnonymousNamespace())
+      return getNamespaceScope(NSD);
+  }
   if (const NamedDecl *ND = dyn_cast<NamedDecl>(DC))
-    return ND->getQualifiedNameAsString();
+    return printQualifiedName(*ND);
 
   return "";
 }
 
 std::string printDefinition(const Decl *D) {
   std::string Definition;
   llvm::raw_string_ostream OS(Definition);
   PrintingPolicy Policy =
HoverInfo getHoverContents(const Decl *D, const SymbolIndex *Index) {
   }
 
   HI.Definition = printDefinition(D);
   enhanceFromIndex(HI, D, Index);
   return HI;
 }
 
 /// Generate a \p Hover object given the type \p T.
 HoverInfo getHoverContents(QualType T, const Decl *D, ASTContext &ASTCtx,
-                                  const SymbolIndex *Index) {
   HoverInfo HI;
+  auto FillInHover = [&HI, Index, &ASTCtx](const Decl *D) {
+    if (const auto *ND = llvm::dyn_cast<NamedDecl>(D))
+      HI.Name = printName(ASTCtx, *ND);
+    HI.Kind = index::getSymbolInfo(D).Kind;
+    enhanceFromIndex(HI, D, Index);
+  };
+
+  if (D) {
+    FillInHover(D);
+  } else if (const auto *TD = T->getAsTagDecl()) {
+    FillInHover(TD);
+  }
+
+  if (HI.Name.empty()) {
+    // Builtin types
     llvm::raw_string_ostream OS(HI.Name);
     PrintingPolicy Policy = printingPolicyForDecls(ASTCtx.getPrintingPolicy());
     T.print(OS, Policy);
     OS.flush();
-
-  if (D) {
-    HI.Kind = index::getSymbolInfo(D).Kind;
-    enhanceFromIndex(HI, D, Index);
   }
   return HI;
 }
 
 /// Generate a \p Hover object given the macro \p MacroDecl.
 HoverInfo getHoverContents(const DefinedMacro &Macro, ParsedAST &AST) {
   HoverInfo HI;
   SourceManager &SM = AST.getSourceManager();
   HI.Name = Macro.Name;
   HI.Kind = index::SymbolKind::Macro;
   // FIXME: Populate documentation
   // FIXME: Pupulate parameters
 
   // Try to get the full definition, not just the name
   SourceLocation StartLoc = Macro.Info->getDefinitionLoc();
   SourceLocation EndLoc = Macro.Info->getDefinitionEndLoc();
llvm::Optional<HoverInfo> getHover(ParsedAST &AST, Position Pos,
   } else {
     auto Offset = positionToOffset(SM.getBufferData(SM.getMainFileID()), Pos);
     if (!Offset) {
       llvm::consumeError(Offset.takeError());
       return llvm::None;
     }
     SelectionTree Selection(AST.getASTContext(), AST.getTokens(), *Offset);
     std::vector<const Decl *> Result;
     if (const SelectionTree::Node *N = Selection.commonAncestor()) {
       DeclRelationSet Rel = DeclRelation::TemplatePattern | DeclRelation::Alias;
       auto Decls = targetDecl(N->ASTNode, Rel);
       if (!Decls.empty()) {
         HI = getHoverContents(Decls.front(), Index);
         // Look for a close enclosing expression to show the value of.
         if (!HI->Value)
           HI->Value = printExprValue(N, AST.getASTContext());
       }

clang-tools-extra/clangd/unittests/HoverTests.cpp

const std::function<void(HoverInfo &)> ExpectedBuilder;
       {R"cpp(
           namespace ns1 { namespace {
             struct {
               int [[b^ar]];
             } T;
           }}
           )cpp",
        [](HoverInfo &HI) {
-         HI.NamespaceScope = "ns1::(anonymous)::";
+         HI.NamespaceScope = "ns1::";
          HI.LocalScope = "(anonymous struct)::";
          HI.Name = "bar";
          HI.Kind = index::SymbolKind::Field;
          HI.Definition = "int bar";
          HI.Type = "int";
        }},
       // Variable with template type
       {R"cpp(
void foo())cpp";
 
       // auto on lambda
       {R"cpp(
         void foo() {
           [[au^to]] lamb = []{};
         }
         )cpp",
        [](HoverInfo &HI) {
-         HI.Name = "class (lambda)";
+         // FIXME: Special case lambdas.
+         HI.Name = "(anonymous class)";
          HI.Kind = index::SymbolKind::Class;
        }},
       // auto on template instantiation
       {R"cpp(
         template<typename T> class Foo{};
         void foo() {
           [[au^to]] x = Foo<int>();
         }
         )cpp",
        [](HoverInfo &HI) {
-         HI.Name = "class Foo<int>";
+         HI.Name = "Foo";
          HI.Kind = index::SymbolKind::Class;
        }},
       // auto on specialized template
       {R"cpp(
         template<typename T> class Foo{};
         template<> class Foo<int>{};
         void foo() {
           [[au^to]] x = Foo<int>();
         }
         )cpp",
        [](HoverInfo &HI) {
-         HI.Name = "class Foo<int>";
+         HI.Name = "Foo<int>";
          HI.Kind = index::SymbolKind::Class;
        }},
 
       // macro
       {R"cpp(
         // Best MACRO ever.
         #define MACRO(x,y,z) void foo(x, y, z);
         [[MAC^RO]](int, double d, bool z = false);
void foo())cpp";
        [](HoverInfo &HI) {
          HI.Name = "name";
          HI.Definition = "int name";
          HI.Kind = index::SymbolKind::Parameter;
          HI.Type = "int";
          HI.NamespaceScope = "";
          HI.LocalScope = "boom::";
        }},
+      {
+          R"cpp(// Should not print inline or anon namespaces.
+          namespace ns {
+            inline namespace in_ns {
+              namespace {
+                inline namespace in_ns2 {
+                  class Foo {};
+                } // in_ns2
+              } // anon
+            } // in_ns
+          } // ns
+          void foo() {
+            [[au^to]] x = ns::Foo();
+          }
+          )cpp",
+          [](HoverInfo &HI) {
+            HI.Name = "Foo";
+            HI.Kind = index::SymbolKind::Class;
+          }},
   };
   for (const auto &Case : Cases) {
     SCOPED_TRACE(Case.Code);
 
     Annotations T(Case.Code);
     TestTU TU = TestTU::withCode(T.code());
     TU.ExtraArgs.push_back("-std=c++17");
     TU.ExtraArgs.push_back("-fno-delayed-template-parsing");
const std::function<void(HoverInfo &)> ExpectedBuilder;
                 int foo;
               } // anonymous namespace
             } // namespace ns
             int main() { ns::[[f^oo]]++; }
           )cpp",
           [](HoverInfo &HI) {
             HI.Name = "foo";
             HI.Kind = index::SymbolKind::Variable;
-            HI.NamespaceScope = "ns::(anonymous)::";
+            HI.NamespaceScope = "ns::";
             HI.Type = "int";
             HI.Definition = "int foo";
           }},
       {
           R"cpp(// Macro
             #define MACRO 0
             int main() { return ^[[MACRO]]; }
           )cpp",
const std::function<void(HoverInfo &)> ExpectedBuilder;
               template<class _E>
               class initializer_list {};
             }
             void foo() {
               ^[[auto]] i = {1,2};
             }
           )cpp",
           [](HoverInfo &HI) {
-            HI.Name = "class std::initializer_list<int>";
+            // FIXME: Print template instantiation parameters.
+            HI.Name = "initializer_list";
             HI.Kind = index::SymbolKind::Class;
           }},
       {
           R"cpp(// User defined conversion to auto
             struct Bar {
               operator ^[[auto]]() const { return 10; }
             };
           )cpp",
const std::function<void(HoverInfo &)> ExpectedBuilder;
       {
           R"cpp(// auto function return with trailing type
             struct Bar {};
             ^[[auto]] test() -> decltype(Bar()) {
               return Bar();
             }
           )cpp",
           [](HoverInfo &HI) {
-            HI.Name = "struct Bar";
+            HI.Name = "Bar";
             HI.Kind = index::SymbolKind::Struct;
           }},
       {
           R"cpp(// trailing return type
             struct Bar {};
             auto test() -> ^[[decltype]](Bar()) {
               return Bar();
             }
           )cpp",
           [](HoverInfo &HI) {
-            HI.Name = "struct Bar";
+            HI.Name = "Bar";
             HI.Kind = index::SymbolKind::Struct;
           }},
       {
           R"cpp(// auto in function return
             struct Bar {};
             ^[[auto]] test() {
               return Bar();
             }
           )cpp",
           [](HoverInfo &HI) {
-            HI.Name = "struct Bar";
+            HI.Name = "Bar";
             HI.Kind = index::SymbolKind::Struct;
           }},
       {
           R"cpp(// auto& in function return
             struct Bar {};
             ^[[auto]]& test() {
               static Bar x;
               return x;
             }
           )cpp",
           [](HoverInfo &HI) {
-            HI.Name = "struct Bar";
+            HI.Name = "Bar";
             HI.Kind = index::SymbolKind::Struct;
           }},
       {
           R"cpp(// auto* in function return
             struct Bar {};
             ^[[auto]]* test() {
               Bar* bar;
               return bar;
             }
           )cpp",
           [](HoverInfo &HI) {
-            HI.Name = "struct Bar";
+            HI.Name = "Bar";
             HI.Kind = index::SymbolKind::Struct;
           }},
       {
           R"cpp(// const auto& in function return
             struct Bar {};
             const ^[[auto]]& test() {
               static Bar x;
               return x;
             }
           )cpp",
           [](HoverInfo &HI) {
-            HI.Name = "struct Bar";
+            HI.Name = "Bar";
             HI.Kind = index::SymbolKind::Struct;
           }},
       {
           R"cpp(// decltype(auto) in function return
             struct Bar {};
             ^[[decltype]](auto) test() {
               return Bar();
             }
           )cpp",
           [](HoverInfo &HI) {
-            HI.Name = "struct Bar";
+            HI.Name = "Bar";
             HI.Kind = index::SymbolKind::Struct;
           }},
       {
           R"cpp(// decltype(auto) reference in function return
             ^[[decltype]](auto) test() {
               static int a;
               return (a);
             }
const std::function<void(HoverInfo &)> ExpectedBuilder;
             auto test() -> decltype(Bar()) {
               return Bar();
             }
             void foo() {
               ^[[decltype]](test()) i = test();
             }
           )cpp",
           [](HoverInfo &HI) {
-            HI.Name = "struct Bar";
+            HI.Name = "Bar";
             HI.Kind = index::SymbolKind::Struct;
           }},
       {
           R"cpp(// decltype of var with decltype.
             void foo() {
               int I = 0;
               decltype(I) J = I;
               ^[[decltype]](J) K = J;