[clangd] Support go-to-definition on type hints. The core part #86629
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
zyn0217
force-pushed
the
inlay-hint-label-part-impl
branch
from
c8f1495 to
fa5fb4a
Compare
|
I think I'm ready for feedback after around one week of dogfooding myself. |
|
@llvm/pr-subscribers-clang-tools-extra Author: Younan Zhang (zyn0217) ChangesThis patch is going to offer go-to-definition support on nested type template arguments, for example: the clickable links on template names don't involve their namespace specifiers, e.g. the Things we probably want besides this patch:
Resolves clangd/clangd#1535 Patch is 23.50 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/86629.diff 5 Files Affected:
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..8d8fbc4bcde042 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"
@@ -21,6 +22,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 +374,276 @@ 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 CurrentType;
+ ASTContext &Context;
+ const PrintingPolicy &PP;
+ std::vector<InlayHintLabelPart> &LabelChunks;
+
+ bool ShouldAddLinksToTagTypes;
+
+ struct CurrentTypeRAII {
+ TypeInlayHintLabelPartBuilder &Builder;
+ QualType PreviousType;
+ bool PreviousShouldAddLinksToTagTypes;
+ CurrentTypeRAII(TypeInlayHintLabelPartBuilder &Builder, QualType New,
+ bool ShouldAddLinksToTagTypes)
+ : Builder(Builder), PreviousType(Builder.CurrentType),
+ PreviousShouldAddLinksToTagTypes(Builder.ShouldAddLinksToTagTypes) {
+ Builder.CurrentType = New;
+ Builder.ShouldAddLinksToTagTypes = ShouldAddLinksToTagTypes;
+ }
+ ~CurrentTypeRAII() {
+ Builder.CurrentType = PreviousType;
+ Builder.ShouldAddLinksToTagTypes = PreviousShouldAddLinksToTagTypes;
+ }
+ };
+
+ void addLabel(llvm::function_ref<void(llvm::raw_ostream &)> NamePrinter,
+ llvm::function_ref<SourceRange()> SourceRangeGetter) {
+ 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) {
+ auto &TA = Args[I];
+ if (PP.SuppressDefaultTemplateArgs && TA.getIsDefaulted())
+ continue;
+ if (I)
+ addLabel(", ");
+ printTemplateArgument(TA);
+ }
+ }
+
+ void printTemplateArgument(const TemplateArgument &TA) {
+ switch (TA.getKind()) {
+ case TemplateArgument::Pack:
+ return printTemplateArgumentList(TA.pack_elements());
+ case TemplateArgument::Type: {
+ CurrentTypeRAII Guard(*this, TA.getAsType(),
+ /*ShouldAddLinksToTagTypes=*/true);
+ return Visit(TA.getAsType().getTypePtr());
+ }
+ // 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
+ handleTemplateSpecialization(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;
+ });
+
+ addLabel("<");
+ printTemplateArgumentList(Args);
+ addLabel(">");
+ }
+
+ void maybeAddQualifiers() {
+ auto Quals = CurrentType.split().Quals;
+ if (!Quals.empty()) {
+ addLabel(Quals.getAsString());
+ addLabel(" ");
+ }
+ }
+
+ // 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,
+ bool ShouldAddLinksToTagTypes,
+ llvm::StringRef Prefix,
+ std::vector<InlayHintLabelPart> &LabelChunks)
+ : CurrentType(Current), Context(Context), PP(PP),
+ LabelChunks(LabelChunks),
+ ShouldAddLinksToTagTypes(ShouldAddLinksToTagTypes) {
+ LabelChunks.reserve(16);
+ if (!Prefix.empty())
+ addLabel(Prefix.str());
+ }
+
+ void VisitType(const Type *) { addLabel(CurrentType.getAsString(PP)); }
+
+ void VisitTagType(const TagType *TT) {
+ if (!ShouldAddLinksToTagTypes)
+ return VisitType(TT);
+ auto *D = TT->getDecl();
+ if (auto *Specialization = dyn_cast<ClassTemplateSpecializationDecl>(D))
+ return handleTemplateSpecialization(
+ 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;
+ 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:
+ case NestedNameSpecifier::Namespace:
+ case NestedNameSpecifier::NamespaceAlias:
+ case NestedNameSpecifier::Global:
+ case NestedNameSpecifier::Super: {
+ std::string Label;
+ llvm::raw_string_ostream OS(Label);
+ addLabel(std::move(Label));
+ } 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());
+ addLabel("::");
+ break;
+ }
+ }
+ CurrentTypeRAII Guard(*this, ET->getNamedType(), ShouldAddLinksToTagTypes);
+ return Visit(ET->getNamedType().getTypePtr());
+ }
+
+ void VisitReferenceType(const ReferenceType *RT) {
+ maybeAddQualifiers();
+ QualType Next = skipTopLevelReferences(RT->getPointeeTypeAsWritten());
+ CurrentTypeRAII Guard(*this, Next, ShouldAddLinksToTagTypes);
+ Visit(Next.getTypePtr());
+ if (Next->getPointeeType().isNull())
+ addLabel(" ");
+ if (RT->isLValueReferenceType())
+ addLabel("&");
+ if (RT->isRValueReferenceType())
+ 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();
+ }
+
+ void VisitTemplateSpecializationType(const TemplateSpecializationType *TST) {
+ maybeAddQualifiers();
+ SourceRange Range;
+ if (auto *Specialization =
+ dyn_cast_if_present<ClassTemplateSpecializationDecl>(
+ TST->desugar().getCanonicalType()->getAsCXXRecordDecl()))
+ Range = Specialization->getSourceRange();
+ return handleTemplateSpecialization(TST->getTemplateName(),
+ TST->template_arguments(), Range);
+ }
+
+ void VisitSubstTemplateTypeParmType(const SubstTemplateTypeParmType *ST) {
+ maybeAddQualifiers();
+ 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 +1221,30 @@ class InlayHintVisitor : public RecursiveASTVisitor<InlayHintVisitor> {
addInlayHint(*LSPRange, Side, Kind, Prefix, Label, Suffix);
}
+ void addInlayHint(SourceRange R, HintSide Side, InlayHintKind Kind,
+ llvm::StringRef Prefix,
+ std::vector<InlayHintLabelPart> Labels,
+ llvm::StringRef Suffix) {
+ auto LSPRange = getHintRange(R);
+ if (!LSPRange)
+ return;
+
+ addInlayHint(*LSPRange, Side, Kind, Prefix, std::move(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,
+ 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.
@@ -977,9 +1270,27 @@ 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;
+ }
+ 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=*/{(Prefix + Label + Suffix).str()},
- Kind, PadLeft, PadRight, LSPRange});
+ /*label=*/std::move(Labels), Kind, PadLeft,
+ PadRight, LSPRange});
}
// Get the range of the main file that *exactly* corresponds to R.
@@ -1005,14 +1316,24 @@ 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, Prefix, 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,
+ Prefix, 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=*/"", // We have handled prefixes in the builder.
+ std::move(Chunks),
/*Suffix=*/"");
}
@@ -1021,9 +1342,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..a69422d825f30b 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> PointsTo = std::nullopt;
+
+ friend llvm::raw_ostream &operator<<(llvm::raw_ostream &Stream,
+ const ExpectedHintLabelPiece &Hint) {
+ Stream << Hint.Label;
+ if (Hint.PointsTo)
+ Stream << " that points to $" << Hint.PointsTo;
+ 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) {
+ *result_listener << "label is '" << ResultLabel << "'";
+ return false;
+ }
+ if (!Expected.PointsTo && !arg.location)
+ return true;
+ if (Expected.PointsTo && !arg.location) {
+ *result_listener << " range " << *Expected.PointsTo
+ << " is expected, but we have nothing.";
+ return false;
+ }
+ 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.PointsTo)) {
+ *result_listener << "range is " << llvm::to_string(arg.location->range)
+ << " but $" << *Expected.PointsTo << " points to "
+ << llvm::to_string(Code.range(*Expected.PointsTo));
+ return false;
+ }
+ return true;
+}
+
MATCHER_P(labelIs, Label, "") { return arg.joinLabels() == Label; }
Config noHintsConfig() {
@@ -1637,6 +1678,125 @@ 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) {
+ StringRef Source(R"cpp(
+ $Package[[template <class T, class U>
+ struct Package {]]};
+
+ $SpecializationOfPackage[[template ...
[truncated]
|
|
@llvm/pr-subscribers-clangd Author: Younan Zhang (zyn0217) ChangesThis patch is going to offer go-to-definition support on nested type template arguments, for example: the clickable links on template names don't involve their namespace specifiers, e.g. the Things we probably want besides this patch:
Resolves clangd/clangd#1535 Patch is 23.50 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/86629.diff 5 Files Affected:
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..8d8fbc4bcde042 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"
@@ -21,6 +22,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 +374,276 @@ 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 CurrentType;
+ ASTContext &Context;
+ const PrintingPolicy &PP;
+ std::vector<InlayHintLabelPart> &LabelChunks;
+
+ bool ShouldAddLinksToTagTypes;
+
+ struct CurrentTypeRAII {
+ TypeInlayHintLabelPartBuilder &Builder;
+ QualType PreviousType;
+ bool PreviousShouldAddLinksToTagTypes;
+ CurrentTypeRAII(TypeInlayHintLabelPartBuilder &Builder, QualType New,
+ bool ShouldAddLinksToTagTypes)
+ : Builder(Builder), PreviousType(Builder.CurrentType),
+ PreviousShouldAddLinksToTagTypes(Builder.ShouldAddLinksToTagTypes) {
+ Builder.CurrentType = New;
+ Builder.ShouldAddLinksToTagTypes = ShouldAddLinksToTagTypes;
+ }
+ ~CurrentTypeRAII() {
+ Builder.CurrentType = PreviousType;
+ Builder.ShouldAddLinksToTagTypes = PreviousShouldAddLinksToTagTypes;
+ }
+ };
+
+ void addLabel(llvm::function_ref<void(llvm::raw_ostream &)> NamePrinter,
+ llvm::function_ref<SourceRange()> SourceRangeGetter) {
+ 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) {
+ auto &TA = Args[I];
+ if (PP.SuppressDefaultTemplateArgs && TA.getIsDefaulted())
+ continue;
+ if (I)
+ addLabel(", ");
+ printTemplateArgument(TA);
+ }
+ }
+
+ void printTemplateArgument(const TemplateArgument &TA) {
+ switch (TA.getKind()) {
+ case TemplateArgument::Pack:
+ return printTemplateArgumentList(TA.pack_elements());
+ case TemplateArgument::Type: {
+ CurrentTypeRAII Guard(*this, TA.getAsType(),
+ /*ShouldAddLinksToTagTypes=*/true);
+ return Visit(TA.getAsType().getTypePtr());
+ }
+ // 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
+ handleTemplateSpecialization(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;
+ });
+
+ addLabel("<");
+ printTemplateArgumentList(Args);
+ addLabel(">");
+ }
+
+ void maybeAddQualifiers() {
+ auto Quals = CurrentType.split().Quals;
+ if (!Quals.empty()) {
+ addLabel(Quals.getAsString());
+ addLabel(" ");
+ }
+ }
+
+ // 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,
+ bool ShouldAddLinksToTagTypes,
+ llvm::StringRef Prefix,
+ std::vector<InlayHintLabelPart> &LabelChunks)
+ : CurrentType(Current), Context(Context), PP(PP),
+ LabelChunks(LabelChunks),
+ ShouldAddLinksToTagTypes(ShouldAddLinksToTagTypes) {
+ LabelChunks.reserve(16);
+ if (!Prefix.empty())
+ addLabel(Prefix.str());
+ }
+
+ void VisitType(const Type *) { addLabel(CurrentType.getAsString(PP)); }
+
+ void VisitTagType(const TagType *TT) {
+ if (!ShouldAddLinksToTagTypes)
+ return VisitType(TT);
+ auto *D = TT->getDecl();
+ if (auto *Specialization = dyn_cast<ClassTemplateSpecializationDecl>(D))
+ return handleTemplateSpecialization(
+ 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;
+ 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:
+ case NestedNameSpecifier::Namespace:
+ case NestedNameSpecifier::NamespaceAlias:
+ case NestedNameSpecifier::Global:
+ case NestedNameSpecifier::Super: {
+ std::string Label;
+ llvm::raw_string_ostream OS(Label);
+ addLabel(std::move(Label));
+ } 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());
+ addLabel("::");
+ break;
+ }
+ }
+ CurrentTypeRAII Guard(*this, ET->getNamedType(), ShouldAddLinksToTagTypes);
+ return Visit(ET->getNamedType().getTypePtr());
+ }
+
+ void VisitReferenceType(const ReferenceType *RT) {
+ maybeAddQualifiers();
+ QualType Next = skipTopLevelReferences(RT->getPointeeTypeAsWritten());
+ CurrentTypeRAII Guard(*this, Next, ShouldAddLinksToTagTypes);
+ Visit(Next.getTypePtr());
+ if (Next->getPointeeType().isNull())
+ addLabel(" ");
+ if (RT->isLValueReferenceType())
+ addLabel("&");
+ if (RT->isRValueReferenceType())
+ 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();
+ }
+
+ void VisitTemplateSpecializationType(const TemplateSpecializationType *TST) {
+ maybeAddQualifiers();
+ SourceRange Range;
+ if (auto *Specialization =
+ dyn_cast_if_present<ClassTemplateSpecializationDecl>(
+ TST->desugar().getCanonicalType()->getAsCXXRecordDecl()))
+ Range = Specialization->getSourceRange();
+ return handleTemplateSpecialization(TST->getTemplateName(),
+ TST->template_arguments(), Range);
+ }
+
+ void VisitSubstTemplateTypeParmType(const SubstTemplateTypeParmType *ST) {
+ maybeAddQualifiers();
+ 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 +1221,30 @@ class InlayHintVisitor : public RecursiveASTVisitor<InlayHintVisitor> {
addInlayHint(*LSPRange, Side, Kind, Prefix, Label, Suffix);
}
+ void addInlayHint(SourceRange R, HintSide Side, InlayHintKind Kind,
+ llvm::StringRef Prefix,
+ std::vector<InlayHintLabelPart> Labels,
+ llvm::StringRef Suffix) {
+ auto LSPRange = getHintRange(R);
+ if (!LSPRange)
+ return;
+
+ addInlayHint(*LSPRange, Side, Kind, Prefix, std::move(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,
+ 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.
@@ -977,9 +1270,27 @@ 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;
+ }
+ 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=*/{(Prefix + Label + Suffix).str()},
- Kind, PadLeft, PadRight, LSPRange});
+ /*label=*/std::move(Labels), Kind, PadLeft,
+ PadRight, LSPRange});
}
// Get the range of the main file that *exactly* corresponds to R.
@@ -1005,14 +1316,24 @@ 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, Prefix, 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,
+ Prefix, 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=*/"", // We have handled prefixes in the builder.
+ std::move(Chunks),
/*Suffix=*/"");
}
@@ -1021,9 +1342,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..a69422d825f30b 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> PointsTo = std::nullopt;
+
+ friend llvm::raw_ostream &operator<<(llvm::raw_ostream &Stream,
+ const ExpectedHintLabelPiece &Hint) {
+ Stream << Hint.Label;
+ if (Hint.PointsTo)
+ Stream << " that points to $" << Hint.PointsTo;
+ 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) {
+ *result_listener << "label is '" << ResultLabel << "'";
+ return false;
+ }
+ if (!Expected.PointsTo && !arg.location)
+ return true;
+ if (Expected.PointsTo && !arg.location) {
+ *result_listener << " range " << *Expected.PointsTo
+ << " is expected, but we have nothing.";
+ return false;
+ }
+ 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.PointsTo)) {
+ *result_listener << "range is " << llvm::to_string(arg.location->range)
+ << " but $" << *Expected.PointsTo << " points to "
+ << llvm::to_string(Code.range(*Expected.PointsTo));
+ return false;
+ }
+ return true;
+}
+
MATCHER_P(labelIs, Label, "") { return arg.joinLabels() == Label; }
Config noHintsConfig() {
@@ -1637,6 +1678,125 @@ 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) {
+ StringRef Source(R"cpp(
+ $Package[[template <class T, class U>
+ struct Package {]]};
+
+ $SpecializationOfPackage[[template ...
[truncated]
|
|
@HighCommander4 Any chance to have a try on this patch? I'm looking forward to the feedback, thanks! |
|
Sorry I haven't had a chance to look at this yet. I've been busy with another project, and will continue to be for another week or so. I hope to look at this soon! |
|
@HighCommander4 No problem, I'm always delighted to receive responses! However it might take me some time to pick up my thoughts, as the patch has been a while. Anyway, I will try to revive this patch in the next few days and let's see if we can make it into clangd 19. |
|
@HighCommander4 Sorry for my delay. This is ready for the next round of review, and I'm prepared for a lot of feedback! |
HighCommander4
requested changes
Jun 19, 2024
There was a problem hiding this comment.
Thanks for the updated patch, this generally looks very nice!
It's a bit unfortunate that we have to reimplement parts of type printing like handling qualifiers and pointers/references. I wouldn't be surprised if we get some edge cases wrong compared to TypePrinter. However, I don't have a better idea currently (other than invasive changes like adding extension points to TypePrinter), and I find two things reassuring:
- Our existing
TypeHints.*tests exercise the type printing logic of this class, so we know we are not regressing those cases. - If we do regress some edge cases, it's not a big problem, since this feature is just a reading aid and not something critical to correctness.
So, I think it's fine to go with this approach. Thanks for all your efforts putting this together, and sorry it took me this long to get around to revieiwng the implementation!
|
Thanks for the prompt and insightful feedback; I really appreciate it! There are a lot of comments, so it takes some time to digest them. I'll make an effort to address them in the coming days. |
…onPattern() for specializations
|
@HighCommander4 Given that this PR has been growing larger, I plan to split it up. Hopefully, this will make the code review easier. Moreover, I realized it's probably insufficient to build the whole thing on top of a |
HighCommander4
requested changes
Sep 4, 2024
There was a problem hiding this comment.
(@zyn0217 I'm going to mark this as "Request changes" to reflect my understanding from the previous comment that the next step here is you splitting up the patch. If I've misunderstood and you'd like me to have a look at the current version, let me know!)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This patch is going to offer go-to-definition support on nested type template arguments, for example:
the clickable links on template names don't involve their namespace specifiers, e.g. the
llvm::part inllvm::SmallVectorwill not be combined together with the followingSmallVectorlinks. (This is just because I'm too lazy to implement the combination logic, not because we want to distinguish something. The fun fact is, which I've also checked, that CLion doesn't even offer namespaces/type specifiers at all for inlay type hints.)Things we probably want besides this patch:
Resolves clangd/clangd#1535