sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.
LG, though I think your patch might be based on top of some uncommitted changes.
I noticed template names rather than type names are shown (not new in this
patch) - we should fix that I think.
================
Comment at: clang-tools-extra/unittests/clangd/TypeHierarchyTests.cpp:384
TEST(TypeHierarchy, RecursiveHierarchy1) {
Annotations Source(R"cpp(
----------------
can we give this a more descriptive name like RecursiveHierarchyUnbounded?
================
Comment at: clang-tools-extra/unittests/clangd/TypeHierarchyTests.cpp:405
+ *Result,
+ AllOf(WithName("S"), WithKind(SymbolKind::Struct),
+ Parents(AllOf(WithName("S"), WithKind(SymbolKind::Struct),
----------------
Sorry, I realize this isn't related to this patch, but I didn't see the final
form of the previous one.
This should be `WithName("S<0>"), ... Parents(AllOf(WithName("S<1>")), ...)`. S
is the name of the template, not the name of the type.
Can you add a fixme?
================
Comment at: clang-tools-extra/unittests/clangd/TypeHierarchyTests.cpp:410
TEST(TypeHierarchy, RecursiveHierarchy2) {
Annotations Source(R"cpp(
----------------
RecursiveHierarchyBounded
================
Comment at: clang-tools-extra/unittests/clangd/TypeHierarchyTests.cpp:447
struct Foo {
S^<N> s;
};
----------------
this can be merged with the previous test case, using two named points.
================
Comment at: clang-tools-extra/unittests/clangd/TypeHierarchyTests.cpp:463
+ AllOf(WithName("S"), WithKind(SymbolKind::Struct),
+ Parents(AllOf(WithName("S"), WithKind(SymbolKind::Struct),
+ SelectionRangeIs(Source.range("SDef")),
Parents()))));
----------------
(in this case the printed name for the parent might end up looking odd -
printing the spelled `S<N - 1>` is probably ideal but I don't really mind what
we end up with, this is an edge case. Not for this patch, in any case)
================
Comment at: clang-tools-extra/unittests/clangd/TypeHierarchyTests.cpp:609
-// Disabled for the same reason as TypeParents.DependentBase.
-TEST(DISABLED_Subtypes, DependentBase) {
+TEST(Subtypes, DependentBase) {
Annotations Source(R"cpp(
----------------
this patch doesn't seem to be against HEAD
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D59756/new/
https://reviews.llvm.org/D59756
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
