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 <zyn7...@gmail.com> Date: Sat, 16 Mar 2024 23:30:10 +0800 Subject: [PATCH 01/10] [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 1b86ea19cf28d..ef87f1bcb8443 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 fb0722d697cd0..3ae624b1ab741 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 cd4f1931b3ce1..f9e0a51ddcc9f 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 ed56c2a9d2e81..f3fbb9fc307e2 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 5b1531eb2fa60..7f99a3522e490 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 <zyn7...@gmail.com> Date: Tue, 26 Mar 2024 14:41:38 +0800 Subject: [PATCH 02/10] 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 f9e0a51ddcc9f..ed84bd2a643b2 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 <zyn7...@gmail.com> Date: Sun, 31 Mar 2024 22:33:49 +0800 Subject: [PATCH 03/10] 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 ed84bd2a643b2..7304775a3d951 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 7f99a3522e490..20f32bc7b7f6b 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 <zyn7...@gmail.com> Date: Sun, 31 Mar 2024 23:13:03 +0800 Subject: [PATCH 04/10] 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 7304775a3d951..11254c8fc81af 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 20f32bc7b7f6b..2464172a3c68a 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 <zyn7...@gmail.com> Date: Sun, 31 Mar 2024 23:30:18 +0800 Subject: [PATCH 05/10] 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 11254c8fc81af..5045eb9b6fd26 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("::"); >From 9edd0f63541630a0594a4db8b5f517d6e682ddc2 Mon Sep 17 00:00:00 2001 From: Younan Zhang <zyn7...@gmail.com> Date: Mon, 1 Apr 2024 13:28:27 +0800 Subject: [PATCH 06/10] Handle reference/pointer types & Preserve qualifiers & Refactor tests slightly --- clang-tools-extra/clangd/InlayHints.cpp | 30 +++ .../clangd/unittests/InlayHintTests.cpp | 174 +++++++++--------- 2 files changed, 122 insertions(+), 82 deletions(-) diff --git a/clang-tools-extra/clangd/InlayHints.cpp b/clang-tools-extra/clangd/InlayHints.cpp index 5045eb9b6fd26..c28e7671cb401 100644 --- a/clang-tools-extra/clangd/InlayHints.cpp +++ b/clang-tools-extra/clangd/InlayHints.cpp @@ -506,6 +506,14 @@ class TypeInlayHintLabelPartBuilder addLabel(">"); } + void maybeAddQualifiers() { + auto Quals = CurrentType.split().Quals; + if (!Quals.empty()) { + addLabel(Quals.getAsString()); + addLabel(" "); + } + } + public: TypeInlayHintLabelPartBuilder(QualType Current, ASTContext &Context, const PrintingPolicy &PP, @@ -540,12 +548,14 @@ class TypeInlayHintLabelPartBuilder void VisitAutoType(const AutoType *AT) { if (!AT->isDeduced() || AT->getDeducedType()->isDecltypeType()) return; + maybeAddQualifiers(); CurrentTypeRAII Guard(*this, AT->getDeducedType(), ShouldAddLinksToTagTypes); return Visit(AT->getDeducedType().getTypePtr()); } void VisitElaboratedType(const ElaboratedType *ET) { + maybeAddQualifiers(); if (auto *NNS = ET->getQualifier()) { switch (NNS->getKind()) { case NestedNameSpecifier::Identifier: @@ -574,7 +584,26 @@ class TypeInlayHintLabelPartBuilder return Visit(ET->getNamedType().getTypePtr()); } + void VisitReferenceType(const ReferenceType *RT) { + maybeAddQualifiers(); + CurrentTypeRAII Guard(*this, RT->getPointeeTypeAsWritten(), ShouldAddLinksToTagTypes); + Visit(RT->getPointeeTypeAsWritten().getTypePtr()); + if (RT->isLValueReferenceType()) + addLabel(" &"); + if (RT->isRValueReferenceType()) + addLabel(" &&"); + } + + void VisitPointerType(const PointerType *PT) { + maybeAddQualifiers(); + CurrentTypeRAII Guard(*this, PT->getPointeeType(), + ShouldAddLinksToTagTypes); + Visit(PT->getPointeeType().getTypePtr()); + addLabel(" *"); + } + void VisitTemplateSpecializationType(const TemplateSpecializationType *TST) { + maybeAddQualifiers(); SourceRange Range; if (auto *Specialization = dyn_cast_if_present<ClassTemplateSpecializationDecl>( @@ -585,6 +614,7 @@ class TypeInlayHintLabelPartBuilder } void VisitSubstTemplateTypeParmType(const SubstTemplateTypeParmType *ST) { + maybeAddQualifiers(); CurrentTypeRAII Guard(*this, ST->getReplacementType(), /*ShouldAddLinksToTagTypes=*/true); return Visit(ST->getReplacementType().getTypePtr()); diff --git a/clang-tools-extra/clangd/unittests/InlayHintTests.cpp b/clang-tools-extra/clangd/unittests/InlayHintTests.cpp index 2464172a3c68a..238a0a120a46e 100644 --- a/clang-tools-extra/clangd/unittests/InlayHintTests.cpp +++ b/clang-tools-extra/clangd/unittests/InlayHintTests.cpp @@ -57,13 +57,13 @@ struct ExpectedHint { struct ExpectedHintLabelPiece { std::string Label; - std::optional<std::string> TargetRangeName = std::nullopt; + std::optional<std::string> PointsTo = 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; + if (Hint.PointsTo) + Stream << " that points to $" << Hint.PointsTo; return Stream; } }; @@ -89,26 +89,26 @@ MATCHER_P2(HintMatcher, Expected, Code, llvm::to_string(Expected)) { MATCHER_P2(HintLabelPieceMatcher, Expected, Code, llvm::to_string(Expected)) { llvm::StringRef ExpectedView(Expected.Label); std::string ResultLabel = arg.value; - if (ResultLabel != ExpectedView.trim(" ")) { + if (ResultLabel != ExpectedView) { *result_listener << "label is '" << ResultLabel << "'"; return false; } - if (!Expected.TargetRangeName && !arg.location) + if (!Expected.PointsTo && !arg.location) return true; - if (Expected.TargetRangeName && !arg.location) { - *result_listener << " range " << *Expected.TargetRangeName + if (Expected.PointsTo && !arg.location) { + *result_listener << " range " << *Expected.PointsTo << " is expected, but we have nothing."; return false; } - if (!Expected.TargetRangeName && arg.location) { + if (!Expected.PointsTo && 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)) { + if (arg.location->range != Code.range(*Expected.PointsTo)) { *result_listener << "range is " << llvm::to_string(arg.location->range) - << " but $" << *Expected.TargetRangeName << " points to " - << llvm::to_string(Code.range(*Expected.TargetRangeName)); + << " but $" << *Expected.PointsTo << " points to " + << llvm::to_string(Code.range(*Expected.PointsTo)); return false; } return true; @@ -1678,8 +1678,33 @@ TEST(TypeHints, SubstTemplateParameterAliases) { ExpectedHint{": static_vector<int>", "vector_name"}); } +template <typename ...Labels> +void assertTypeLinkHints(StringRef Code, StringRef HintRange, Labels ...ExpectedLabels) { + Annotations Source(Code); + 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); + }; + + TestTU TU = TestTU::withCode(Source.code()); + TU.ExtraArgs.push_back("-std=c++2c"); + auto AST = TU.build(); + + Config C; + C.InlayHints.TypeNameLimit = 0; + WithContextValue WithCfg(Config::Key, std::move(C)); + + auto Hints = hintsOfKind(AST, InlayHintKind::Type); + EXPECT_THAT(HintAt(Hints, HintRange), + ElementsAre(HintLabelPieceMatcher(ExpectedLabels, Source)...)); +} + TEST(TypeHints, Links) { - Annotations Source(R"cpp( + StringRef Source(R"cpp( $Package[[template <class T, class U> struct Package {]]}; @@ -1701,89 +1726,74 @@ TEST(TypeHints, Links) { }; namespace ns { - template <class T> + $Nested[[template <class T> struct Nested { - template <class U> - struct Class { + $NestedClass[[template <class U> + struct ]]Class { }; - }; + ]]}; } - void foo() { + void basic() { 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(); - - 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)); + void compounds() { + auto $6[[A]] = Container<ns::Nested<int>::Class<float>&>(); + auto $7[[B]] = Container<ns::Nested<int>::Class<float>&&>(); + auto $8[[C]] = Container<ns::Nested<int>::Class<const Container<int>> *>(); + } - auto Hints = hintsOfKind(AST, InlayHintKind::Type); + )cpp"); - 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), - // 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"}, - Source), - HintLabelPieceMatcher(ExpectedHintLabelPiece{"<char, int>>"}, - Source))); + assertTypeLinkHints(Source, "1", ExpectedHintLabelPiece{": Container<"}, + ExpectedHintLabelPiece{"Package", "Package"}, + ExpectedHintLabelPiece{"<char, int>>"}); + + assertTypeLinkHints( + Source, "2", ExpectedHintLabelPiece{": Container<"}, + ExpectedHintLabelPiece{"Package", "SpecializationOfPackage"}, + ExpectedHintLabelPiece{"<float, const int>>"}); + + assertTypeLinkHints(Source, "3", ExpectedHintLabelPiece{": Container<"}, + ExpectedHintLabelPiece{"Container", "Container"}, + ExpectedHintLabelPiece{"<int, int>, long>"}); + + assertTypeLinkHints( + Source, "4", + ExpectedHintLabelPiece{": NttpContainer<D, E, ScopedEnum::X, Enum::E>"}); + + assertTypeLinkHints(Source, "5", ExpectedHintLabelPiece{": Nested<"}, + ExpectedHintLabelPiece{"Container", "Container"}, + // We don't have links on the inner 'Class' because the + // location is where the 'auto' links to. + ExpectedHintLabelPiece{"<int>>::Class<"}, + ExpectedHintLabelPiece{"Package", "Package"}, + ExpectedHintLabelPiece{"<char, int>>"}); + + assertTypeLinkHints(Source, "6", ExpectedHintLabelPiece{": Container<"}, + ExpectedHintLabelPiece{"Nested", "Nested"}, + ExpectedHintLabelPiece{"<int>::"}, + ExpectedHintLabelPiece{"Class", "NestedClass"}, + ExpectedHintLabelPiece{"<float> &>"}); + + assertTypeLinkHints(Source, "7", ExpectedHintLabelPiece{": Container<"}, + ExpectedHintLabelPiece{"Nested", "Nested"}, + ExpectedHintLabelPiece{"<int>::"}, + ExpectedHintLabelPiece{"Class", "NestedClass"}, + ExpectedHintLabelPiece{"<float> &&>"}); + + assertTypeLinkHints(Source, "8", ExpectedHintLabelPiece{": Container<"}, + ExpectedHintLabelPiece{"Nested", "Nested"}, + ExpectedHintLabelPiece{"<int>::"}, + ExpectedHintLabelPiece{"Class", "NestedClass"}, + ExpectedHintLabelPiece{"<const "}, + ExpectedHintLabelPiece{"Container", "Container"}, + ExpectedHintLabelPiece{"<int>> *>"}); } TEST(DesignatorHints, Basic) { >From 48e21a102c11d4e9f6f6258bbe68085354ce6ef8 Mon Sep 17 00:00:00 2001 From: Younan Zhang <zyn7...@gmail.com> Date: Mon, 1 Apr 2024 13:29:38 +0800 Subject: [PATCH 07/10] Forgot to format, sorry --- clang-tools-extra/clangd/InlayHints.cpp | 3 ++- clang-tools-extra/clangd/unittests/InlayHintTests.cpp | 5 +++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/clang-tools-extra/clangd/InlayHints.cpp b/clang-tools-extra/clangd/InlayHints.cpp index c28e7671cb401..73701c702e3f5 100644 --- a/clang-tools-extra/clangd/InlayHints.cpp +++ b/clang-tools-extra/clangd/InlayHints.cpp @@ -586,7 +586,8 @@ class TypeInlayHintLabelPartBuilder void VisitReferenceType(const ReferenceType *RT) { maybeAddQualifiers(); - CurrentTypeRAII Guard(*this, RT->getPointeeTypeAsWritten(), ShouldAddLinksToTagTypes); + CurrentTypeRAII Guard(*this, RT->getPointeeTypeAsWritten(), + ShouldAddLinksToTagTypes); Visit(RT->getPointeeTypeAsWritten().getTypePtr()); if (RT->isLValueReferenceType()) addLabel(" &"); diff --git a/clang-tools-extra/clangd/unittests/InlayHintTests.cpp b/clang-tools-extra/clangd/unittests/InlayHintTests.cpp index 238a0a120a46e..a69422d825f30 100644 --- a/clang-tools-extra/clangd/unittests/InlayHintTests.cpp +++ b/clang-tools-extra/clangd/unittests/InlayHintTests.cpp @@ -1678,8 +1678,9 @@ TEST(TypeHints, SubstTemplateParameterAliases) { ExpectedHint{": static_vector<int>", "vector_name"}); } -template <typename ...Labels> -void assertTypeLinkHints(StringRef Code, StringRef HintRange, Labels ...ExpectedLabels) { +template <typename... Labels> +void assertTypeLinkHints(StringRef Code, StringRef HintRange, + Labels... ExpectedLabels) { Annotations Source(Code); auto HintAt = [&](llvm::ArrayRef<InlayHint> InlayHints, llvm::StringRef Range) { >From 8c8c036994783c70a16446771a703c6c99094805 Mon Sep 17 00:00:00 2001 From: Younan Zhang <zyn7...@gmail.com> Date: Mon, 1 Apr 2024 14:02:26 +0800 Subject: [PATCH 08/10] Fix regressions --- clang-tools-extra/clangd/InlayHints.cpp | 32 ++++++++++++++++++------- 1 file changed, 23 insertions(+), 9 deletions(-) diff --git a/clang-tools-extra/clangd/InlayHints.cpp b/clang-tools-extra/clangd/InlayHints.cpp index 73701c702e3f5..7a842e2f76423 100644 --- a/clang-tools-extra/clangd/InlayHints.cpp +++ b/clang-tools-extra/clangd/InlayHints.cpp @@ -514,6 +514,14 @@ class TypeInlayHintLabelPartBuilder } } + // When printing a reference, the referenced type might also be a reference. + // If so, we want to skip that before printing the inner type. + static QualType skipTopLevelReferences(QualType T) { + if (auto *Ref = T->getAs<ReferenceType>()) + return skipTopLevelReferences(Ref->getPointeeTypeAsWritten()); + return T; + } + public: TypeInlayHintLabelPartBuilder(QualType Current, ASTContext &Context, const PrintingPolicy &PP, @@ -586,21 +594,27 @@ class TypeInlayHintLabelPartBuilder void VisitReferenceType(const ReferenceType *RT) { maybeAddQualifiers(); - CurrentTypeRAII Guard(*this, RT->getPointeeTypeAsWritten(), - ShouldAddLinksToTagTypes); - Visit(RT->getPointeeTypeAsWritten().getTypePtr()); + QualType Next = skipTopLevelReferences(RT->getPointeeTypeAsWritten()); + CurrentTypeRAII Guard(*this, Next, ShouldAddLinksToTagTypes); + Visit(Next.getTypePtr()); + if (Next->getPointeeType().isNull()) + addLabel(" "); if (RT->isLValueReferenceType()) - addLabel(" &"); + addLabel("&"); if (RT->isRValueReferenceType()) - addLabel(" &&"); + addLabel("&&"); } void VisitPointerType(const PointerType *PT) { + QualType Next = PT->getPointeeType(); + std::optional<CurrentTypeRAII> Guard(std::in_place, *this, Next, + ShouldAddLinksToTagTypes); + Visit(Next.getTypePtr()); + if (Next->getPointeeType().isNull()) + addLabel(" "); + addLabel("*"); + Guard.reset(); maybeAddQualifiers(); - CurrentTypeRAII Guard(*this, PT->getPointeeType(), - ShouldAddLinksToTagTypes); - Visit(PT->getPointeeType().getTypePtr()); - addLabel(" *"); } void VisitTemplateSpecializationType(const TemplateSpecializationType *TST) { >From 0e25c18f6722d4afcf0a479fd0e88ba8f26bb218 Mon Sep 17 00:00:00 2001 From: Younan Zhang <zyn7...@gmail.com> Date: Mon, 1 Apr 2024 14:59:44 +0800 Subject: [PATCH 09/10] Initialize ShouldAddLinksToTagTypes --- 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 7a842e2f76423..8d8fbc4bcde04 100644 --- a/clang-tools-extra/clangd/InlayHints.cpp +++ b/clang-tools-extra/clangd/InlayHints.cpp @@ -396,7 +396,7 @@ class TypeInlayHintLabelPartBuilder const PrintingPolicy &PP; std::vector<InlayHintLabelPart> &LabelChunks; - bool ShouldAddLinksToTagTypes = false; + bool ShouldAddLinksToTagTypes; struct CurrentTypeRAII { TypeInlayHintLabelPartBuilder &Builder; @@ -529,7 +529,8 @@ class TypeInlayHintLabelPartBuilder llvm::StringRef Prefix, std::vector<InlayHintLabelPart> &LabelChunks) : CurrentType(Current), Context(Context), PP(PP), - LabelChunks(LabelChunks) { + LabelChunks(LabelChunks), + ShouldAddLinksToTagTypes(ShouldAddLinksToTagTypes) { LabelChunks.reserve(16); if (!Prefix.empty()) addLabel(Prefix.str()); >From 20607772fdc6e2f054485c3a56ebeab2feaf9e10 Mon Sep 17 00:00:00 2001 From: Younan Zhang <zyn7...@gmail.com> Date: Fri, 12 Apr 2024 17:58:32 +0800 Subject: [PATCH 10/10] Handle TypeAliases & UsingTypes --- clang-tools-extra/clangd/InlayHints.cpp | 15 +++++++++++++++ .../clangd/unittests/InlayHintTests.cpp | 19 +++++++++++++++++++ 2 files changed, 34 insertions(+) diff --git a/clang-tools-extra/clangd/InlayHints.cpp b/clang-tools-extra/clangd/InlayHints.cpp index 8d8fbc4bcde04..9c02f3307611a 100644 --- a/clang-tools-extra/clangd/InlayHints.cpp +++ b/clang-tools-extra/clangd/InlayHints.cpp @@ -618,6 +618,21 @@ class TypeInlayHintLabelPartBuilder maybeAddQualifiers(); } + void VisitUsingType(const UsingType *UT) { + addLabel([&](llvm::raw_ostream &OS) { UT->getFoundDecl()->printName(OS); }, + [&] { + BaseUsingDecl *Introducer = UT->getFoundDecl()->getIntroducer(); + if (auto *UD = dyn_cast<UsingDecl>(Introducer)) + return UD->getSourceRange(); + return Introducer->getSourceRange(); + }); + } + + void VisitTypedefType(const TypedefType *TT) { + addLabel([&](llvm::raw_ostream &OS) { TT->getDecl()->printName(OS); }, + [&] { return TT->getDecl()->getSourceRange(); }); + } + void VisitTemplateSpecializationType(const TemplateSpecializationType *TST) { maybeAddQualifiers(); SourceRange Range; diff --git a/clang-tools-extra/clangd/unittests/InlayHintTests.cpp b/clang-tools-extra/clangd/unittests/InlayHintTests.cpp index a69422d825f30..a43273a35fcd9 100644 --- a/clang-tools-extra/clangd/unittests/InlayHintTests.cpp +++ b/clang-tools-extra/clangd/unittests/InlayHintTests.cpp @@ -1733,6 +1733,8 @@ TEST(TypeHints, Links) { struct ]]Class { }; ]]}; + + $NestedInt[[using NestedInt = Nested<int]]>; } void basic() { @@ -1749,6 +1751,15 @@ TEST(TypeHints, Links) { auto $8[[C]] = Container<ns::Nested<int>::Class<const Container<int>> *>(); } + namespace nns { + $UsingShadow[[using ns::]]NestedInt; + + void aliases() { + auto $9[[A]] = Container<NestedInt>(); + auto $10[[B]] = Container<ns::NestedInt>(); + } + } + )cpp"); assertTypeLinkHints(Source, "1", ExpectedHintLabelPiece{": Container<"}, @@ -1795,6 +1806,14 @@ TEST(TypeHints, Links) { ExpectedHintLabelPiece{"<const "}, ExpectedHintLabelPiece{"Container", "Container"}, ExpectedHintLabelPiece{"<int>> *>"}); + + assertTypeLinkHints(Source, "9", ExpectedHintLabelPiece{": Container<"}, + ExpectedHintLabelPiece{"NestedInt", "UsingShadow"}, + ExpectedHintLabelPiece{">"}); + + assertTypeLinkHints(Source, "10", ExpectedHintLabelPiece{": Container<"}, + ExpectedHintLabelPiece{"NestedInt", "NestedInt"}, + ExpectedHintLabelPiece{">"}); } TEST(DesignatorHints, Basic) { _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits