[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