[PATCH] D23079: ObjC: Use a new type for ObjC type parameter (patch 2 out of 3)

[Manman Ren via cfe-commits]

manmanren added a comment.

Thanks for reviewing!

Manman


================
Comment at: include/clang/AST/RecursiveASTVisitor.h:1037
@@ -1036,1 +1036,3 @@
 
+DEF_TRAVERSE_TYPE(ObjCTypeParamType, {})
+
----------------
doug.gregor wrote:
> I'm sorta shocked that we don't visit the protocol qualifiers here, but I guess we haven't been doing that for ObjCObjectType all along. Weird.
Right below, we don't visit the protocol qualifiers for ObjCObjectType :]
If you think we should, I can patch it up for both types.

================
Comment at: include/clang/AST/RecursiveASTVisitor.h:1270
@@ -1267,1 +1269,3 @@
 
+DEF_TRAVERSE_TYPELOC(ObjCTypeParamType, {})
+
----------------
doug.gregor wrote:
> Same shock at not visiting the protocol qualifiers here :)
Same here.

================
Comment at: include/clang/AST/Type.h:4786
@@ +4785,3 @@
+  bool isSugared() const { return false; }
+  QualType desugar() const { return QualType(this, 0); }
+
----------------
doug.gregor wrote:
> This is an interesting choice. Objective-C type parameters were treated like typedefs before, so they always act like their underlying type (e.g., because they are typedef name declarations). Why isn't ObjCTypeParamType sugar for the underlying type of the type parameter (I.e., the bound) w/ the protocol qualifiers?
Are you suggesting to canonicalize ObjCTypeParamType to the underlying type with the protocol qualifiers as well? Or just desugaring?

I think I have tried to set ObjCTypeParamType as NON_CANONICAL_TYPE in TypeNodes.def, but had some issues.


https://reviews.llvm.org/D23079