Quuxplusone added inline comments.

================
Comment at: lib/AST/TypePrinter.cpp:179
+    // `X<Some::Class>` gets canonicalized to `X<float, int>`. We disable
+    // canonicalization so that `printTag()`` can see the template parameters 
as
+    // written.
----------------
courbet wrote:
> Quuxplusone wrote:
> > Nit: there's an extra ` on this line.
> > 
> > I think I vaguely understand the purpose of this switch now. It feels ugly, 
> > but I have no concrete suggestion for improvement. Do you not need a case 
> > here to delay canonicalization of `X<Some::Class>&` also?
> Indeed. Fixed + added tests.
And `void(*)(X<Some::Class>)`?  And possibly `int[X<Some::Class>::value]`, but 
I think that might be what `Type::DependentSizedArray` is doing in there. And 
`void (^)(X<Some::Class>)` in Objective-C++.

Surely there should be a list somewhere of all the "compound types" that need 
to go here. And then this starts feeling a lot like the visitor pattern. I 
still don't know how it _should_ look, but this switch is feeling ickier and 
ickier.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55933/new/

https://reviews.llvm.org/D55933



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to