[clang-tools-extra] [clangd] Support go-to-definition on type hints. The core part (PR #86629)

Younan Zhang via cfe-commits cfe-commits at lists.llvm.org
Sun Mar 31 08:30:50 PDT 2024


https://github.com/zyn0217 updated https://github.com/llvm/llvm-project/pull/86629

>From b8a69cbd9e0ee0aa35b38b7e3a78048cbe61447e Mon Sep 17 00:00:00 2001
From: Younan Zhang <zyn7109 at gmail.com>
Date: Sat, 16 Mar 2024 23:30:10 +0800
Subject: [PATCH 1/5] [clangd] Support go-to-definition on type hints. The core
 part

---
 clang-tools-extra/clangd/AST.cpp              |   9 +
 clang-tools-extra/clangd/AST.h                |   2 +
 clang-tools-extra/clangd/InlayHints.cpp       | 251 +++++++++++++++++-
 .../clangd/index/IndexAction.cpp              |   9 +-
 .../clangd/unittests/InlayHintTests.cpp       |  22 ++
 5 files changed, 279 insertions(+), 14 deletions(-)

diff --git a/clang-tools-extra/clangd/AST.cpp b/clang-tools-extra/clangd/AST.cpp
index 1b86ea19cf28da..ef87f1bcb8443c 100644
--- a/clang-tools-extra/clangd/AST.cpp
+++ b/clang-tools-extra/clangd/AST.cpp
@@ -1019,5 +1019,14 @@ bool isExpandedFromParameterPack(const ParmVarDecl *D) {
   return getUnderlyingPackType(D) != nullptr;
 }
 
+std::optional<URI> toURI(OptionalFileEntryRef File) {
+  if (!File)
+    return std::nullopt;
+  auto AbsolutePath = File->getFileEntry().tryGetRealPathName();
+  if (AbsolutePath.empty())
+    return std::nullopt;
+  return URI::create(AbsolutePath);
+}
+
 } // namespace clangd
 } // namespace clang
diff --git a/clang-tools-extra/clangd/AST.h b/clang-tools-extra/clangd/AST.h
index fb0722d697cd06..3ae624b1ab7415 100644
--- a/clang-tools-extra/clangd/AST.h
+++ b/clang-tools-extra/clangd/AST.h
@@ -250,6 +250,8 @@ resolveForwardingParameters(const FunctionDecl *D, unsigned MaxDepth = 10);
 /// reference to one (e.g. `Args&...` or `Args&&...`).
 bool isExpandedFromParameterPack(const ParmVarDecl *D);
 
+std::optional<URI> toURI(OptionalFileEntryRef File);
+
 } // namespace clangd
 } // namespace clang
 
diff --git a/clang-tools-extra/clangd/InlayHints.cpp b/clang-tools-extra/clangd/InlayHints.cpp
index cd4f1931b3ce1d..f9e0a51ddcc9f0 100644
--- a/clang-tools-extra/clangd/InlayHints.cpp
+++ b/clang-tools-extra/clangd/InlayHints.cpp
@@ -21,6 +21,7 @@
 #include "clang/AST/Stmt.h"
 #include "clang/AST/StmtVisitor.h"
 #include "clang/AST/Type.h"
+#include "clang/AST/TypeVisitor.h"
 #include "clang/Basic/Builtins.h"
 #include "clang/Basic/OperatorKinds.h"
 #include "clang/Basic/SourceManager.h"
@@ -372,6 +373,197 @@ maybeDropCxxExplicitObjectParameters(ArrayRef<const ParmVarDecl *> Params) {
   return Params;
 }
 
+std::optional<Location> toLocation(SourceManager &SM, SourceRange Range) {
+  if (Range.isInvalid())
+    return std::nullopt;
+  if (auto URI =
+          toURI(SM.getFileEntryRefForID(SM.getFileID(Range.getBegin())))) {
+    Location L;
+    L.range.start = sourceLocToPosition(SM, Range.getBegin());
+    L.range.end = sourceLocToPosition(SM, Range.getEnd());
+    if (auto File = URIForFile::fromURI(*URI, ""))
+      L.uri = File.get();
+    return L;
+  }
+  return std::nullopt;
+}
+
+class TypeInlayHintLabelPartBuilder
+    : public TypeVisitor<TypeInlayHintLabelPartBuilder> {
+  QualType Current;
+  ASTContext &Context;
+  const PrintingPolicy &PP;
+  std::vector<InlayHintLabelPart> &LabelChunks;
+
+  bool ShouldAddLinksToTagTypes = false;
+
+  struct CurrentTypeRAII {
+    TypeInlayHintLabelPartBuilder &Builder;
+    QualType PreviousType;
+    bool PreviousShouldAddLinksToTagTypes;
+    CurrentTypeRAII(TypeInlayHintLabelPartBuilder &Builder, QualType New,
+                    bool ShouldAddLinksToTagTypes)
+        : Builder(Builder), PreviousType(Builder.Current) {
+      Builder.Current = New;
+      Builder.ShouldAddLinksToTagTypes = ShouldAddLinksToTagTypes;
+    }
+    ~CurrentTypeRAII() {
+      Builder.Current = PreviousType;
+      Builder.ShouldAddLinksToTagTypes = PreviousShouldAddLinksToTagTypes;
+    }
+  };
+
+  void addLabel(llvm::function_ref<void(llvm::raw_ostream &)> NamePrinter,
+                llvm::function_ref<SourceRange()> SourceRangeGetter) {
+    auto &Name = LabelChunks.emplace_back();
+    llvm::raw_string_ostream OS(Name.value);
+    NamePrinter(OS);
+    Name.location = toLocation(Context.getSourceManager(), SourceRangeGetter());
+  }
+
+  void printTemplateArgumentList(llvm::ArrayRef<TemplateArgument> Args) {
+    unsigned Size = Args.size();
+    for (unsigned I = 0; I < Size; ++I) {
+      auto &TA = Args[I];
+      if (PP.SuppressDefaultTemplateArgs && TA.getIsDefaulted())
+        continue;
+      if (I)
+        LabelChunks.emplace_back(", ");
+      printTemplateArgument(TA);
+    }
+  }
+
+  void printTemplateArgument(const TemplateArgument &TA) {
+    if (TA.getKind() == TemplateArgument::Pack)
+      return printTemplateArgumentList(TA.pack_elements());
+    if (TA.getKind() == TemplateArgument::Type) {
+      CurrentTypeRAII Guard(*this, TA.getAsType(),
+                            /*ShouldAddLinksToTagTypes=*/true);
+      return Visit(TA.getAsType().getTypePtr());
+    }
+    llvm::raw_string_ostream OS(LabelChunks.emplace_back().value);
+    TA.print(PP, OS, /*IncludeType=*/true);
+  }
+
+  void
+  processTemplateSpecialization(TemplateName TN,
+                                llvm::ArrayRef<TemplateArgument> Args,
+                                SourceRange TemplateNameRange = SourceRange()) {
+    SourceRange Range;
+    TemplateDecl *TD = nullptr;
+    switch (TN.getKind()) {
+    case TemplateName::Template:
+      TD = TN.getAsTemplateDecl();
+      Range = TD->getSourceRange();
+      LLVM_FALLTHROUGH;
+    default:
+      break;
+    }
+
+    addLabel([&](llvm::raw_ostream &OS) { TN.print(OS, PP); },
+             [&] {
+               if (TemplateNameRange.isValid())
+                 return TemplateNameRange;
+               return Range;
+             });
+
+    LabelChunks.emplace_back("<");
+    printTemplateArgumentList(Args);
+    LabelChunks.emplace_back(">");
+  }
+
+public:
+
+#ifndef NDEBUG
+  ~TypeInlayHintLabelPartBuilder() {
+    llvm::errs() << "TypeInlayHintLabelPartBuilder:\n";
+    Current->dump();
+    for (auto &L : LabelChunks)
+      llvm::errs() << L << ", ";
+    llvm::errs() << "\n";
+  }
+#endif
+
+  TypeInlayHintLabelPartBuilder(QualType Current, ASTContext &Context,
+                                const PrintingPolicy &PP,
+                                bool ShouldAddLinksToTagTypes,
+                                std::vector<InlayHintLabelPart> &LabelChunks)
+      : Current(Current), Context(Context), PP(PP), LabelChunks(LabelChunks) {}
+
+  void VisitType(const Type *) {
+    LabelChunks.emplace_back(Current.getAsString(PP));
+  }
+
+  void VisitTagType(const TagType *TT) {
+    if (!ShouldAddLinksToTagTypes)
+      return VisitType(TT);
+    auto *D = TT->getDecl();
+    if (auto *Specialization = dyn_cast<ClassTemplateSpecializationDecl>(D))
+      return processTemplateSpecialization(
+          TemplateName(Specialization->getSpecializedTemplate()),
+          Specialization->getTemplateArgs().asArray());
+    if (auto *RD = dyn_cast<CXXRecordDecl>(D);
+        RD && !RD->getTemplateInstantiationPattern())
+      return addLabel(
+          [&](llvm::raw_ostream &OS) { return RD->printName(OS, PP); },
+          [&] { return RD->getSourceRange(); });
+    return VisitType(TT);
+  }
+
+  void VisitAutoType(const AutoType *AT) {
+    if (!AT->isDeduced() || AT->getDeducedType()->isDecltypeType())
+      return;
+    CurrentTypeRAII Guard(*this, AT->getDeducedType(),
+                          ShouldAddLinksToTagTypes);
+    return Visit(AT->getDeducedType().getTypePtr());
+  }
+
+  void VisitElaboratedType(const ElaboratedType *ET) {
+    if (auto *NNS = ET->getQualifier()) {
+      switch (NNS->getKind()) {
+      case NestedNameSpecifier::Identifier:
+      case NestedNameSpecifier::Namespace:
+      case NestedNameSpecifier::NamespaceAlias:
+      case NestedNameSpecifier::Global:
+      case NestedNameSpecifier::Super: {
+        auto &Name = LabelChunks.emplace_back();
+        llvm::raw_string_ostream OS(Name.value);
+        NNS->print(OS, PP);
+      } break;
+      case NestedNameSpecifier::TypeSpec:
+      case NestedNameSpecifier::TypeSpecWithTemplate:
+        Visit(NNS->getAsType());
+        break;
+      }
+    }
+    CurrentTypeRAII Guard(*this, ET->getNamedType(), ShouldAddLinksToTagTypes);
+    return Visit(ET->getNamedType().getTypePtr());
+  }
+
+  void VisitTemplateSpecializationType(const TemplateSpecializationType *TST) {
+    SourceRange Range;
+    if (auto *Specialization =
+            dyn_cast_if_present<ClassTemplateSpecializationDecl>(
+                TST->desugar().getCanonicalType()->getAsCXXRecordDecl()))
+      Range = Specialization->getSourceRange();
+    return processTemplateSpecialization(TST->getTemplateName(),
+                                         TST->template_arguments(), Range);
+  }
+
+  void VisitSubstTemplateTypeParmType(const SubstTemplateTypeParmType *ST) {
+    CurrentTypeRAII Guard(*this, ST->getReplacementType(),
+                          /*ShouldAddLinksToTagTypes=*/true);
+    return Visit(ST->getReplacementType().getTypePtr());
+  }
+};
+
+unsigned lengthOfInlayHintLabelPart(llvm::ArrayRef<InlayHintLabelPart> Labels) {
+  unsigned Size = 0;
+  for (auto &P : Labels)
+    Size += P.value.size();
+  return Size;
+}
+
 struct Callee {
   // Only one of Decl or Loc is set.
   // Loc is for calls through function pointers.
@@ -949,9 +1141,29 @@ class InlayHintVisitor : public RecursiveASTVisitor<InlayHintVisitor> {
     addInlayHint(*LSPRange, Side, Kind, Prefix, Label, Suffix);
   }
 
+  void addInlayHint(SourceRange R, HintSide Side, InlayHintKind Kind,
+                    llvm::StringRef Prefix,
+                    llvm::ArrayRef<InlayHintLabelPart> Labels,
+                    llvm::StringRef Suffix) {
+    auto LSPRange = getHintRange(R);
+    if (!LSPRange)
+      return;
+
+    addInlayHint(*LSPRange, Side, Kind, Prefix, Labels, Suffix);
+  }
+
   void addInlayHint(Range LSPRange, HintSide Side, InlayHintKind Kind,
                     llvm::StringRef Prefix, llvm::StringRef Label,
                     llvm::StringRef Suffix) {
+    return addInlayHint(LSPRange, Side, Kind, Prefix,
+                        /*Labels=*/std::vector<InlayHintLabelPart>{Label.str()},
+                        Suffix);
+  }
+
+  void addInlayHint(Range LSPRange, HintSide Side, InlayHintKind Kind,
+                    llvm::StringRef Prefix,
+                    llvm::ArrayRef<InlayHintLabelPart> Labels,
+                    llvm::StringRef Suffix) {
     // We shouldn't get as far as adding a hint if the category is disabled.
     // We'd like to disable as much of the analysis as possible above instead.
     // Assert in debug mode but add a dynamic check in production.
@@ -977,8 +1189,21 @@ class InlayHintVisitor : public RecursiveASTVisitor<InlayHintVisitor> {
       return;
     bool PadLeft = Prefix.consume_front(" ");
     bool PadRight = Suffix.consume_back(" ");
+    if (Prefix.empty() && Suffix.empty()) {
+      Results.push_back(InlayHint{LSPPos,
+                                  /*label=*/Labels, Kind, PadLeft, PadRight,
+                                  LSPRange});
+      return;
+    }
+    std::vector<InlayHintLabelPart> JoinedLabels;
+    JoinedLabels.reserve(Labels.size() + !Prefix.empty() + !Suffix.empty());
+    if (!Prefix.empty())
+      JoinedLabels.push_back(InlayHintLabelPart(Prefix.str()));
+    llvm::copy(Labels, std::back_inserter(JoinedLabels));
+    if (!Suffix.empty())
+      JoinedLabels.push_back(InlayHintLabelPart(Suffix.str()));
     Results.push_back(InlayHint{LSPPos,
-                                /*label=*/{(Prefix + Label + Suffix).str()},
+                                /*label=*/std::move(JoinedLabels),
                                 Kind, PadLeft, PadRight, LSPRange});
   }
 
@@ -1005,14 +1230,22 @@ class InlayHintVisitor : public RecursiveASTVisitor<InlayHintVisitor> {
     // The sugared type is more useful in some cases, and the canonical
     // type in other cases.
     auto Desugared = maybeDesugar(AST, T);
-    std::string TypeName = Desugared.getAsString(TypeHintPolicy);
-    if (T != Desugared && !shouldPrintTypeHint(TypeName)) {
+    std::vector<InlayHintLabelPart> Chunks;
+    TypeInlayHintLabelPartBuilder Builder(
+        Desugared, AST, TypeHintPolicy,
+        /*ShouldAddLinksToTagTypes=*/T != Desugared, Chunks);
+    Builder.Visit(Desugared.getTypePtr());
+    if (T != Desugared && shouldPrintTypeHint(Chunks)) {
       // If the desugared type is too long to display, fallback to the sugared
       // type.
-      TypeName = T.getAsString(TypeHintPolicy);
+      Chunks.clear();
+      TypeInlayHintLabelPartBuilder Builder(T, AST, TypeHintPolicy,
+                                            /*ShouldAddLinksToTagTypes=*/false,
+                                            Chunks);
+      Builder.Visit(T.getTypePtr());
     }
-    if (shouldPrintTypeHint(TypeName))
-      addInlayHint(R, HintSide::Right, InlayHintKind::Type, Prefix, TypeName,
+    if (shouldPrintTypeHint(Chunks))
+      addInlayHint(R, HintSide::Right, InlayHintKind::Type, Prefix, Chunks,
                    /*Suffix=*/"");
   }
 
@@ -1021,9 +1254,11 @@ class InlayHintVisitor : public RecursiveASTVisitor<InlayHintVisitor> {
                  /*Prefix=*/"", Text, /*Suffix=*/"=");
   }
 
-  bool shouldPrintTypeHint(llvm::StringRef TypeName) const noexcept {
+  bool shouldPrintTypeHint(
+      llvm::ArrayRef<InlayHintLabelPart> TypeLabels) const noexcept {
     return Cfg.InlayHints.TypeNameLimit == 0 ||
-           TypeName.size() < Cfg.InlayHints.TypeNameLimit;
+           lengthOfInlayHintLabelPart(TypeLabels) <
+               Cfg.InlayHints.TypeNameLimit;
   }
 
   void addBlockEndHint(SourceRange BraceRange, StringRef DeclPrefix,
diff --git a/clang-tools-extra/clangd/index/IndexAction.cpp b/clang-tools-extra/clangd/index/IndexAction.cpp
index ed56c2a9d2e811..f3fbb9fc307e2e 100644
--- a/clang-tools-extra/clangd/index/IndexAction.cpp
+++ b/clang-tools-extra/clangd/index/IndexAction.cpp
@@ -32,12 +32,9 @@ namespace clangd {
 namespace {
 
 std::optional<std::string> toURI(OptionalFileEntryRef File) {
-  if (!File)
-    return std::nullopt;
-  auto AbsolutePath = File->getFileEntry().tryGetRealPathName();
-  if (AbsolutePath.empty())
-    return std::nullopt;
-  return URI::create(AbsolutePath).toString();
+  if (auto URI = clang::clangd::toURI(File))
+    return URI->toString();
+  return std::nullopt;
 }
 
 // Collects the nodes and edges of include graph during indexing action.
diff --git a/clang-tools-extra/clangd/unittests/InlayHintTests.cpp b/clang-tools-extra/clangd/unittests/InlayHintTests.cpp
index 5b1531eb2fa60b..7f99a3522e4907 100644
--- a/clang-tools-extra/clangd/unittests/InlayHintTests.cpp
+++ b/clang-tools-extra/clangd/unittests/InlayHintTests.cpp
@@ -1637,6 +1637,28 @@ TEST(TypeHints, SubstTemplateParameterAliases) {
                         ExpectedHint{": static_vector<int>", "vector_name"});
 }
 
+TEST(TypeHints, Links) {
+  TestTU TU = TestTU::withCode(R"cpp(
+struct S {};
+template <class T, unsigned V, class... U>
+struct W {
+};
+enum Kind {
+  K = 1,
+};
+namespace std {
+template <class E> struct vector {};
+template <> struct vector<bool> {};
+} // namespace std
+int main() {
+  auto v = std::vector<bool>();
+})cpp");
+  TU.ExtraArgs.push_back("-std=c++2c");
+  auto AST = TU.build();
+
+  hintsOfKind(AST, InlayHintKind::Type);
+}
+
 TEST(DesignatorHints, Basic) {
   assertDesignatorHints(R"cpp(
     struct S { int x, y, z; };

>From fb3bbd0a53d3d4f85a3df4fefda4ae932a44beff Mon Sep 17 00:00:00 2001
From: Younan Zhang <zyn7109 at gmail.com>
Date: Tue, 26 Mar 2024 14:41:38 +0800
Subject: [PATCH 2/5] Small fixes

---
 clang-tools-extra/clangd/InlayHints.cpp | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/clang-tools-extra/clangd/InlayHints.cpp b/clang-tools-extra/clangd/InlayHints.cpp
index f9e0a51ddcc9f0..ed84bd2a643b2b 100644
--- a/clang-tools-extra/clangd/InlayHints.cpp
+++ b/clang-tools-extra/clangd/InlayHints.cpp
@@ -474,7 +474,7 @@ class TypeInlayHintLabelPartBuilder
 
 public:
 
-#ifndef NDEBUG
+#if 0
   ~TypeInlayHintLabelPartBuilder() {
     llvm::errs() << "TypeInlayHintLabelPartBuilder:\n";
     Current->dump();
@@ -532,7 +532,13 @@ class TypeInlayHintLabelPartBuilder
       } break;
       case NestedNameSpecifier::TypeSpec:
       case NestedNameSpecifier::TypeSpecWithTemplate:
+        CurrentTypeRAII Guard(
+            *this,
+            QualType(NNS->getAsType(),
+                     /*Quals=*/0), // Do we need cv-qualifiers on type specifiers?
+            ShouldAddLinksToTagTypes);
         Visit(NNS->getAsType());
+        LabelChunks.emplace_back("::");
         break;
       }
     }
@@ -1235,7 +1241,7 @@ class InlayHintVisitor : public RecursiveASTVisitor<InlayHintVisitor> {
         Desugared, AST, TypeHintPolicy,
         /*ShouldAddLinksToTagTypes=*/T != Desugared, Chunks);
     Builder.Visit(Desugared.getTypePtr());
-    if (T != Desugared && shouldPrintTypeHint(Chunks)) {
+    if (T != Desugared && !shouldPrintTypeHint(Chunks)) {
       // If the desugared type is too long to display, fallback to the sugared
       // type.
       Chunks.clear();

>From 36199ce6b52acba62743adfc4ef80cb2640889f7 Mon Sep 17 00:00:00 2001
From: Younan Zhang <zyn7109 at gmail.com>
Date: Sun, 31 Mar 2024 22:33:49 +0800
Subject: [PATCH 3/5] Add unittests

---
 clang-tools-extra/clangd/InlayHints.cpp       | 129 +++++++++------
 .../clangd/unittests/InlayHintTests.cpp       | 156 ++++++++++++++++--
 2 files changed, 221 insertions(+), 64 deletions(-)

diff --git a/clang-tools-extra/clangd/InlayHints.cpp b/clang-tools-extra/clangd/InlayHints.cpp
index ed84bd2a643b2b..7304775a3d9511 100644
--- a/clang-tools-extra/clangd/InlayHints.cpp
+++ b/clang-tools-extra/clangd/InlayHints.cpp
@@ -11,6 +11,7 @@
 #include "Config.h"
 #include "HeuristicResolver.h"
 #include "ParsedAST.h"
+#include "Protocol.h"
 #include "SourceCode.h"
 #include "clang/AST/ASTDiagnostic.h"
 #include "clang/AST/Decl.h"
@@ -415,12 +416,30 @@ class TypeInlayHintLabelPartBuilder
 
   void addLabel(llvm::function_ref<void(llvm::raw_ostream &)> NamePrinter,
                 llvm::function_ref<SourceRange()> SourceRangeGetter) {
-    auto &Name = LabelChunks.emplace_back();
-    llvm::raw_string_ostream OS(Name.value);
+    std::string Label;
+    llvm::raw_string_ostream OS(Label);
     NamePrinter(OS);
+    if (!ShouldAddLinksToTagTypes)
+      return addLabel(std::move(Label));
+    auto &Name = LabelChunks.emplace_back();
+    Name.value = std::move(Label);
     Name.location = toLocation(Context.getSourceManager(), SourceRangeGetter());
   }
 
+  void addLabel(std::string Label) {
+    if (LabelChunks.empty()) {
+      LabelChunks.emplace_back(std::move(Label));
+      return;
+    }
+    auto &Back = LabelChunks.back();
+    if (Back.location) {
+      LabelChunks.emplace_back(std::move(Label));
+      return;
+    }
+    // Let's combine the "unclickable" pieces together.
+    Back.value += std::move(Label);
+  }
+
   void printTemplateArgumentList(llvm::ArrayRef<TemplateArgument> Args) {
     unsigned Size = Args.size();
     for (unsigned I = 0; I < Size; ++I) {
@@ -428,27 +447,41 @@ class TypeInlayHintLabelPartBuilder
       if (PP.SuppressDefaultTemplateArgs && TA.getIsDefaulted())
         continue;
       if (I)
-        LabelChunks.emplace_back(", ");
+        addLabel(", ");
       printTemplateArgument(TA);
     }
   }
 
   void printTemplateArgument(const TemplateArgument &TA) {
-    if (TA.getKind() == TemplateArgument::Pack)
+    switch (TA.getKind()) {
+    case TemplateArgument::Pack:
       return printTemplateArgumentList(TA.pack_elements());
-    if (TA.getKind() == TemplateArgument::Type) {
+    case TemplateArgument::Type: {
       CurrentTypeRAII Guard(*this, TA.getAsType(),
                             /*ShouldAddLinksToTagTypes=*/true);
       return Visit(TA.getAsType().getTypePtr());
     }
-    llvm::raw_string_ostream OS(LabelChunks.emplace_back().value);
+    // TODO: Add support for NTTP arguments.
+    case TemplateArgument::Expression:
+    case TemplateArgument::StructuralValue:
+    case TemplateArgument::Null:
+    case TemplateArgument::Declaration:
+    case TemplateArgument::NullPtr:
+    case TemplateArgument::Integral:
+    case TemplateArgument::Template:
+    case TemplateArgument::TemplateExpansion:
+      break;
+    }
+    std::string Label;
+    llvm::raw_string_ostream OS(Label);
     TA.print(PP, OS, /*IncludeType=*/true);
+    addLabel(std::move(Label));
   }
 
   void
-  processTemplateSpecialization(TemplateName TN,
-                                llvm::ArrayRef<TemplateArgument> Args,
-                                SourceRange TemplateNameRange = SourceRange()) {
+  handleTemplateSpecialization(TemplateName TN,
+                               llvm::ArrayRef<TemplateArgument> Args,
+                               SourceRange TemplateNameRange = SourceRange()) {
     SourceRange Range;
     TemplateDecl *TD = nullptr;
     switch (TN.getKind()) {
@@ -467,39 +500,31 @@ class TypeInlayHintLabelPartBuilder
                return Range;
              });
 
-    LabelChunks.emplace_back("<");
+    addLabel("<");
     printTemplateArgumentList(Args);
-    LabelChunks.emplace_back(">");
+    addLabel(">");
   }
 
 public:
-
-#if 0
-  ~TypeInlayHintLabelPartBuilder() {
-    llvm::errs() << "TypeInlayHintLabelPartBuilder:\n";
-    Current->dump();
-    for (auto &L : LabelChunks)
-      llvm::errs() << L << ", ";
-    llvm::errs() << "\n";
-  }
-#endif
-
   TypeInlayHintLabelPartBuilder(QualType Current, ASTContext &Context,
                                 const PrintingPolicy &PP,
                                 bool ShouldAddLinksToTagTypes,
+                                llvm::StringRef Prefix,
                                 std::vector<InlayHintLabelPart> &LabelChunks)
-      : Current(Current), Context(Context), PP(PP), LabelChunks(LabelChunks) {}
-
-  void VisitType(const Type *) {
-    LabelChunks.emplace_back(Current.getAsString(PP));
+      : Current(Current), Context(Context), PP(PP), LabelChunks(LabelChunks) {
+    LabelChunks.reserve(16);
+    if (!Prefix.empty())
+      addLabel(Prefix.str());
   }
 
+  void VisitType(const Type *) { addLabel(Current.getAsString(PP)); }
+
   void VisitTagType(const TagType *TT) {
     if (!ShouldAddLinksToTagTypes)
       return VisitType(TT);
     auto *D = TT->getDecl();
     if (auto *Specialization = dyn_cast<ClassTemplateSpecializationDecl>(D))
-      return processTemplateSpecialization(
+      return handleTemplateSpecialization(
           TemplateName(Specialization->getSpecializedTemplate()),
           Specialization->getTemplateArgs().asArray());
     if (auto *RD = dyn_cast<CXXRecordDecl>(D);
@@ -526,9 +551,9 @@ class TypeInlayHintLabelPartBuilder
       case NestedNameSpecifier::NamespaceAlias:
       case NestedNameSpecifier::Global:
       case NestedNameSpecifier::Super: {
-        auto &Name = LabelChunks.emplace_back();
-        llvm::raw_string_ostream OS(Name.value);
-        NNS->print(OS, PP);
+        std::string Label;
+        llvm::raw_string_ostream OS(Label);
+        addLabel(std::move(Label));
       } break;
       case NestedNameSpecifier::TypeSpec:
       case NestedNameSpecifier::TypeSpecWithTemplate:
@@ -538,7 +563,7 @@ class TypeInlayHintLabelPartBuilder
                      /*Quals=*/0), // Do we need cv-qualifiers on type specifiers?
             ShouldAddLinksToTagTypes);
         Visit(NNS->getAsType());
-        LabelChunks.emplace_back("::");
+        addLabel("::");
         break;
       }
     }
@@ -552,8 +577,8 @@ class TypeInlayHintLabelPartBuilder
             dyn_cast_if_present<ClassTemplateSpecializationDecl>(
                 TST->desugar().getCanonicalType()->getAsCXXRecordDecl()))
       Range = Specialization->getSourceRange();
-    return processTemplateSpecialization(TST->getTemplateName(),
-                                         TST->template_arguments(), Range);
+    return handleTemplateSpecialization(TST->getTemplateName(),
+                                        TST->template_arguments(), Range);
   }
 
   void VisitSubstTemplateTypeParmType(const SubstTemplateTypeParmType *ST) {
@@ -1149,13 +1174,13 @@ class InlayHintVisitor : public RecursiveASTVisitor<InlayHintVisitor> {
 
   void addInlayHint(SourceRange R, HintSide Side, InlayHintKind Kind,
                     llvm::StringRef Prefix,
-                    llvm::ArrayRef<InlayHintLabelPart> Labels,
+                    std::vector<InlayHintLabelPart> Labels,
                     llvm::StringRef Suffix) {
     auto LSPRange = getHintRange(R);
     if (!LSPRange)
       return;
 
-    addInlayHint(*LSPRange, Side, Kind, Prefix, Labels, Suffix);
+    addInlayHint(*LSPRange, Side, Kind, Prefix, std::move(Labels), Suffix);
   }
 
   void addInlayHint(Range LSPRange, HintSide Side, InlayHintKind Kind,
@@ -1168,8 +1193,9 @@ class InlayHintVisitor : public RecursiveASTVisitor<InlayHintVisitor> {
 
   void addInlayHint(Range LSPRange, HintSide Side, InlayHintKind Kind,
                     llvm::StringRef Prefix,
-                    llvm::ArrayRef<InlayHintLabelPart> Labels,
+                    std::vector<InlayHintLabelPart> Labels,
                     llvm::StringRef Suffix) {
+    assert(!Labels.empty() && "Expected non-empty labels");
     // We shouldn't get as far as adding a hint if the category is disabled.
     // We'd like to disable as much of the analysis as possible above instead.
     // Assert in debug mode but add a dynamic check in production.
@@ -1201,16 +1227,21 @@ class InlayHintVisitor : public RecursiveASTVisitor<InlayHintVisitor> {
                                   LSPRange});
       return;
     }
-    std::vector<InlayHintLabelPart> JoinedLabels;
-    JoinedLabels.reserve(Labels.size() + !Prefix.empty() + !Suffix.empty());
-    if (!Prefix.empty())
-      JoinedLabels.push_back(InlayHintLabelPart(Prefix.str()));
-    llvm::copy(Labels, std::back_inserter(JoinedLabels));
-    if (!Suffix.empty())
-      JoinedLabels.push_back(InlayHintLabelPart(Suffix.str()));
+    if (!Prefix.empty()) {
+      if (auto &Label = Labels.front(); !Label.location)
+        Label.value = Prefix.str() + Label.value;
+      else
+        Labels.insert(Labels.begin(), InlayHintLabelPart(Prefix.str()));
+    }
+    if (!Suffix.empty()) {
+      if (auto &Label = Labels.back(); !Label.location)
+        Label.value += Suffix.str();
+      else
+        Labels.push_back(InlayHintLabelPart(Suffix.str()));
+    }
     Results.push_back(InlayHint{LSPPos,
-                                /*label=*/std::move(JoinedLabels),
-                                Kind, PadLeft, PadRight, LSPRange});
+                                /*label=*/std::move(Labels), Kind, PadLeft,
+                                PadRight, LSPRange});
   }
 
   // Get the range of the main file that *exactly* corresponds to R.
@@ -1239,7 +1270,7 @@ class InlayHintVisitor : public RecursiveASTVisitor<InlayHintVisitor> {
     std::vector<InlayHintLabelPart> Chunks;
     TypeInlayHintLabelPartBuilder Builder(
         Desugared, AST, TypeHintPolicy,
-        /*ShouldAddLinksToTagTypes=*/T != Desugared, Chunks);
+        /*ShouldAddLinksToTagTypes=*/T != Desugared, Prefix, Chunks);
     Builder.Visit(Desugared.getTypePtr());
     if (T != Desugared && !shouldPrintTypeHint(Chunks)) {
       // If the desugared type is too long to display, fallback to the sugared
@@ -1247,11 +1278,13 @@ class InlayHintVisitor : public RecursiveASTVisitor<InlayHintVisitor> {
       Chunks.clear();
       TypeInlayHintLabelPartBuilder Builder(T, AST, TypeHintPolicy,
                                             /*ShouldAddLinksToTagTypes=*/false,
-                                            Chunks);
+                                            Prefix, Chunks);
       Builder.Visit(T.getTypePtr());
     }
     if (shouldPrintTypeHint(Chunks))
-      addInlayHint(R, HintSide::Right, InlayHintKind::Type, Prefix, Chunks,
+      addInlayHint(R, HintSide::Right, InlayHintKind::Type,
+                   /*Prefix=*/"", // We have handled prefixes in the builder.
+                   std::move(Chunks),
                    /*Suffix=*/"");
   }
 
diff --git a/clang-tools-extra/clangd/unittests/InlayHintTests.cpp b/clang-tools-extra/clangd/unittests/InlayHintTests.cpp
index 7f99a3522e4907..20f32bc7b7f6b8 100644
--- a/clang-tools-extra/clangd/unittests/InlayHintTests.cpp
+++ b/clang-tools-extra/clangd/unittests/InlayHintTests.cpp
@@ -55,6 +55,19 @@ struct ExpectedHint {
   }
 };
 
+struct ExpectedHintLabelPiece {
+  std::string Label;
+  std::optional<std::string> TargetRangeName = std::nullopt;
+
+  friend llvm::raw_ostream &operator<<(llvm::raw_ostream &Stream,
+                                       const ExpectedHintLabelPiece &Hint) {
+    Stream << Hint.Label;
+    if (!Hint.TargetRangeName)
+      Stream << " that points to $" << Hint.TargetRangeName;
+    return Stream;
+  }
+};
+
 MATCHER_P2(HintMatcher, Expected, Code, llvm::to_string(Expected)) {
   llvm::StringRef ExpectedView(Expected.Label);
   std::string ResultLabel = arg.joinLabels();
@@ -73,6 +86,34 @@ MATCHER_P2(HintMatcher, Expected, Code, llvm::to_string(Expected)) {
   return true;
 }
 
+MATCHER_P2(HintLabelPieceMatcher, Expected, Code, llvm::to_string(Expected)) {
+  llvm::StringRef ExpectedView(Expected.Label);
+  std::string ResultLabel = arg.value;
+  if (ResultLabel != ExpectedView.trim(" ")) {
+    *result_listener << "label is '" << ResultLabel << "'";
+    return false;
+  }
+  if (!Expected.TargetRangeName && !arg.location)
+    return true;
+  if (Expected.TargetRangeName && !arg.location) {
+    *result_listener << " range " << *Expected.TargetRangeName
+                     << " is expected, but we have nothing.";
+    return false;
+  }
+  if (!Expected.TargetRangeName && arg.location) {
+    *result_listener << " the link points to " << llvm::to_string(arg.location)
+                     << ", but we expect nothing.";
+    return false;
+  }
+  if (arg.location->range != Code.range(*Expected.TargetRangeName)) {
+    *result_listener << "range is " << llvm::to_string(arg.location->range)
+                     << " but $" << *Expected.TargetRangeName << " points to "
+                     << llvm::to_string(Code.range(*Expected.TargetRangeName));
+    return false;
+  }
+  return true;
+}
+
 MATCHER_P(labelIs, Label, "") { return arg.joinLabels() == Label; }
 
 Config noHintsConfig() {
@@ -1638,25 +1679,108 @@ TEST(TypeHints, SubstTemplateParameterAliases) {
 }
 
 TEST(TypeHints, Links) {
-  TestTU TU = TestTU::withCode(R"cpp(
-struct S {};
-template <class T, unsigned V, class... U>
-struct W {
-};
-enum Kind {
-  K = 1,
-};
-namespace std {
-template <class E> struct vector {};
-template <> struct vector<bool> {};
-} // namespace std
-int main() {
-  auto v = std::vector<bool>();
-})cpp");
+  Annotations Source(R"cpp(
+    $Package[[template <class T, class U>
+    struct Package {]]};
+
+    $SpecializationOfPackage[[template <>
+    struct Package<float, const int> {]]};
+
+    $Container[[template <class... T>
+    struct Container {]]};
+
+    $NttpContainer[[template <auto... T>
+    struct NttpContainer {]]};
+
+    enum struct ScopedEnum {
+      X = 1,
+    };
+
+    enum Enum {
+      E = 2,
+    };
+
+    namespace ns {
+      template <class T>
+      struct Nested {
+        template <class U>
+        struct Class {
+        };
+      };
+    }
+
+    void foo() {
+      auto $1[[C]] = Container<Package<char, int>>();
+      auto $2[[D]] = Container<Package<float, const int>>();
+      auto $3[[E]] = Container<Container<int, int>, long>();
+      auto $4[[F]] = NttpContainer<D, E, ScopedEnum::X, Enum::E>();
+      auto $5[[G]] = ns::Nested<Container<int>>::Class<Package<char, int>>();
+    }
+  )cpp");
+  TestTU TU = TestTU::withCode(Source.code());
   TU.ExtraArgs.push_back("-std=c++2c");
   auto AST = TU.build();
 
-  hintsOfKind(AST, InlayHintKind::Type);
+  auto HintAt = [&](llvm::ArrayRef<InlayHint> InlayHints,
+                    llvm::StringRef Range) {
+    auto *Hint = llvm::find_if(InlayHints, [&](const InlayHint &InlayHint) {
+      return InlayHint.range == Source.range(Range);
+    });
+    assert(Hint && "No range was found");
+    return llvm::ArrayRef(Hint->label);
+  };
+
+  Config C;
+  C.InlayHints.TypeNameLimit = 0;
+  WithContextValue WithCfg(Config::Key, std::move(C));
+
+  auto Hints = hintsOfKind(AST, InlayHintKind::Type);
+
+  EXPECT_THAT(
+      HintAt(Hints, "1"),
+      ElementsAre(
+          HintLabelPieceMatcher(ExpectedHintLabelPiece{": Container<"}, Source),
+          HintLabelPieceMatcher(ExpectedHintLabelPiece{"Package", "Package"},
+                                Source),
+          HintLabelPieceMatcher(ExpectedHintLabelPiece{"<char, int>>"},
+                                Source)));
+
+  EXPECT_THAT(
+      HintAt(Hints, "2"),
+      ElementsAre(
+          HintLabelPieceMatcher(ExpectedHintLabelPiece{": Container<"}, Source),
+          HintLabelPieceMatcher(
+              ExpectedHintLabelPiece{"Package", "SpecializationOfPackage"},
+              Source),
+          HintLabelPieceMatcher(ExpectedHintLabelPiece{"<float, const int>>"},
+                                Source)));
+
+  EXPECT_THAT(
+      HintAt(Hints, "3"),
+      ElementsAre(
+          HintLabelPieceMatcher(ExpectedHintLabelPiece{": Container<"}, Source),
+          HintLabelPieceMatcher(
+              ExpectedHintLabelPiece{"Container", "Container"}, Source),
+          HintLabelPieceMatcher(ExpectedHintLabelPiece{"<int, int>, long>"},
+                                Source)));
+
+  EXPECT_THAT(HintAt(Hints, "4"),
+              ElementsAre(HintLabelPieceMatcher(
+                  ExpectedHintLabelPiece{
+                      ": NttpContainer<D, E, ScopedEnum::X, Enum::E>"},
+                  Source)));
+  EXPECT_THAT(
+      HintAt(Hints, "5"),
+      ElementsAre(
+          HintLabelPieceMatcher(ExpectedHintLabelPiece{": Nested<"}, Source),
+          HintLabelPieceMatcher(
+              ExpectedHintLabelPiece{"Container", "Container"}, Source),
+          HintLabelPieceMatcher(ExpectedHintLabelPiece{"<int>>::Class<"},
+                                Source),
+          HintLabelPieceMatcher(ExpectedHintLabelPiece{"Package", "Package"},
+                                Source),
+          HintLabelPieceMatcher(ExpectedHintLabelPiece{"<char, int>>"},
+                                Source)));
 }
 
 TEST(DesignatorHints, Basic) {

>From fa5fb4a93ca64163428cc5ccab25391d4ff4580d Mon Sep 17 00:00:00 2001
From: Younan Zhang <zyn7109 at gmail.com>
Date: Sun, 31 Mar 2024 23:13:03 +0800
Subject: [PATCH 4/5] Fix an uninitialization bug

---
 clang-tools-extra/clangd/InlayHints.cpp            | 14 ++++++++------
 .../clangd/unittests/InlayHintTests.cpp            |  5 ++++-
 2 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/clang-tools-extra/clangd/InlayHints.cpp b/clang-tools-extra/clangd/InlayHints.cpp
index 7304775a3d9511..11254c8fc81aff 100644
--- a/clang-tools-extra/clangd/InlayHints.cpp
+++ b/clang-tools-extra/clangd/InlayHints.cpp
@@ -391,7 +391,7 @@ std::optional<Location> toLocation(SourceManager &SM, SourceRange Range) {
 
 class TypeInlayHintLabelPartBuilder
     : public TypeVisitor<TypeInlayHintLabelPartBuilder> {
-  QualType Current;
+  QualType CurrentType;
   ASTContext &Context;
   const PrintingPolicy &PP;
   std::vector<InlayHintLabelPart> &LabelChunks;
@@ -404,12 +404,13 @@ class TypeInlayHintLabelPartBuilder
     bool PreviousShouldAddLinksToTagTypes;
     CurrentTypeRAII(TypeInlayHintLabelPartBuilder &Builder, QualType New,
                     bool ShouldAddLinksToTagTypes)
-        : Builder(Builder), PreviousType(Builder.Current) {
-      Builder.Current = New;
+        : Builder(Builder), PreviousType(Builder.CurrentType),
+          PreviousShouldAddLinksToTagTypes(Builder.ShouldAddLinksToTagTypes) {
+      Builder.CurrentType = New;
       Builder.ShouldAddLinksToTagTypes = ShouldAddLinksToTagTypes;
     }
     ~CurrentTypeRAII() {
-      Builder.Current = PreviousType;
+      Builder.CurrentType = PreviousType;
       Builder.ShouldAddLinksToTagTypes = PreviousShouldAddLinksToTagTypes;
     }
   };
@@ -511,13 +512,14 @@ class TypeInlayHintLabelPartBuilder
                                 bool ShouldAddLinksToTagTypes,
                                 llvm::StringRef Prefix,
                                 std::vector<InlayHintLabelPart> &LabelChunks)
-      : Current(Current), Context(Context), PP(PP), LabelChunks(LabelChunks) {
+      : CurrentType(Current), Context(Context), PP(PP),
+        LabelChunks(LabelChunks) {
     LabelChunks.reserve(16);
     if (!Prefix.empty())
       addLabel(Prefix.str());
   }
 
-  void VisitType(const Type *) { addLabel(Current.getAsString(PP)); }
+  void VisitType(const Type *) { addLabel(CurrentType.getAsString(PP)); }
 
   void VisitTagType(const TagType *TT) {
     if (!ShouldAddLinksToTagTypes)
diff --git a/clang-tools-extra/clangd/unittests/InlayHintTests.cpp b/clang-tools-extra/clangd/unittests/InlayHintTests.cpp
index 20f32bc7b7f6b8..2464172a3c68a0 100644
--- a/clang-tools-extra/clangd/unittests/InlayHintTests.cpp
+++ b/clang-tools-extra/clangd/unittests/InlayHintTests.cpp
@@ -62,7 +62,7 @@ struct ExpectedHintLabelPiece {
   friend llvm::raw_ostream &operator<<(llvm::raw_ostream &Stream,
                                        const ExpectedHintLabelPiece &Hint) {
     Stream << Hint.Label;
-    if (!Hint.TargetRangeName)
+    if (Hint.TargetRangeName)
       Stream << " that points to $" << Hint.TargetRangeName;
     return Stream;
   }
@@ -1769,12 +1769,15 @@ TEST(TypeHints, Links) {
                   ExpectedHintLabelPiece{
                       ": NttpContainer<D, E, ScopedEnum::X, Enum::E>"},
                   Source)));
+
   EXPECT_THAT(
       HintAt(Hints, "5"),
       ElementsAre(
           HintLabelPieceMatcher(ExpectedHintLabelPiece{": Nested<"}, Source),
           HintLabelPieceMatcher(
               ExpectedHintLabelPiece{"Container", "Container"}, Source),
+          // We don't have links on the inner 'Class' because the location is
+          // where the 'auto' links to.
           HintLabelPieceMatcher(ExpectedHintLabelPiece{"<int>>::Class<"},
                                 Source),
           HintLabelPieceMatcher(ExpectedHintLabelPiece{"Package", "Package"},

>From 31a0dafeae52e3b1b90bc191c0e595596d3b6a26 Mon Sep 17 00:00:00 2001
From: Younan Zhang <zyn7109 at gmail.com>
Date: Sun, 31 Mar 2024 23:30:18 +0800
Subject: [PATCH 5/5] format

---
 clang-tools-extra/clangd/InlayHints.cpp | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/clang-tools-extra/clangd/InlayHints.cpp b/clang-tools-extra/clangd/InlayHints.cpp
index 11254c8fc81aff..5045eb9b6fd266 100644
--- a/clang-tools-extra/clangd/InlayHints.cpp
+++ b/clang-tools-extra/clangd/InlayHints.cpp
@@ -561,8 +561,9 @@ class TypeInlayHintLabelPartBuilder
       case NestedNameSpecifier::TypeSpecWithTemplate:
         CurrentTypeRAII Guard(
             *this,
-            QualType(NNS->getAsType(),
-                     /*Quals=*/0), // Do we need cv-qualifiers on type specifiers?
+            QualType(
+                NNS->getAsType(),
+                /*Quals=*/0), // Do we need cv-qualifiers on type specifiers?
             ShouldAddLinksToTagTypes);
         Visit(NNS->getAsType());
         addLabel("::");



More information about the cfe-commits mailing list