nridge marked an inline comment as done.
nridge added inline comments.

================
Comment at: clang-tools-extra/unittests/clangd/TypeHierarchyTests.cpp:405
+      *Result,
+      AllOf(WithName("S"), WithKind(SymbolKind::Struct),
+            Parents(AllOf(WithName("S"), WithKind(SymbolKind::Struct),
----------------
sammccall wrote:
> nridge wrote:
> > sammccall wrote:
> > > 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?
> > (I looked into what it would take to fix this, and it seems like we need 
> > D59639, so I'm going to wait for that.)
> AFAICS D59639 is something subtly different - it prints the args of 
> *specializations* as written in the source code, not instantiations. i.e. if 
> you try to use this for `vector<int>`, it won't work as there's no 
> specialization, you'll get `vector<T>` instead.
I had another look, and you're right; I didn't appreciate that distinction.

I filed [an issue](https://github.com/clangd/clangd/issues/31) to track 
implementing your suggestion. I plan to work on it.

In the meantime, could we get this patch committed?


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

Reply via email to