Page MenuHomePhabricator

ObjC: Use a new type for ObjC type parameter (patch 3 out of 3)
ClosedPublic

Authored by manmanren on Aug 2 2016, 12:49 PM.

Details

Summary

Depends on https://reviews.llvm.org/D23078 and https://reviews.llvm.org/D23079

We say ObjCTypeParamType is ObjCObjectPointerType, but we assert
fail when trying to call Type::getPointeeType.

We compare the types between declaration and definition for ObjCMethods that
use the type parameter, by using simpleTransform that transforms the
ObjCTypeParamType to the underlying type of the ObjCTypeParamDecl. This matches
our previous behavior.

rdar://24619481
rdar://25060179

Diff Detail

Repository
rL LLVM

Event Timeline

manmanren updated this revision to Diff 66529.Aug 2 2016, 12:49 PM
manmanren retitled this revision from to ObjC: Use a new type for ObjC type parameter (patch 3 out of 3).
manmanren updated this object.
manmanren added a reviewer: doug.gregor.
manmanren added a subscriber: cfe-commits.
doug.gregor requested changes to this revision.Aug 19 2016, 4:39 PM
doug.gregor edited edge metadata.

I think most of the complexity here will fold away when ObjCTypeParamType becomes sugar for the underlying ObjCObjectPointerType.

lib/AST/Type.cpp
1095 ↗(On Diff #66529)

I think we should combine the protocol lists.

1290 ↗(On Diff #66529)

I think you won't need this if you take the approach I suggested on the previous patch of making ObjCTypeParamType type sugar for, effectively, the type you're computing here.

lib/Sema/SemaExprObjC.cpp
388 ↗(On Diff #66529)

Dropping protocol qualifiers here? Again, I think this would be solved by treating ObjCTypeParamType as sugar.

824 ↗(On Diff #66529)

Also unnecessary w/ the desugaring approach?

lib/Sema/SemaType.cpp
901 ↗(On Diff #66529)

... same comment about desugaring.

917 ↗(On Diff #66529)

Here too :)

3397 ↗(On Diff #66529)

... same comment about desugaring.

lib/StaticAnalyzer/Core/CallEvent.cpp
63 ↗(On Diff #66529)

... same comment about desugaring.

133 ↗(On Diff #66529)

... same comment about desugaring.

This revision now requires changes to proceed.Aug 19 2016, 4:39 PM

I will update this patch once the 2nd patch is done.

Thanks for reviewing,
Manman

lib/AST/Type.cpp
1095 ↗(On Diff #66529)

I did combine the protocol lists in the implementation, but forgot to remove the comments :]

1290 ↗(On Diff #66529)

I will update this once we fix the previous patch.

manmanren updated this revision to Diff 69765.Aug 30 2016, 3:15 PM
manmanren updated this object.
manmanren edited edge metadata.

This patch is now much simpler with the updated version of D23079.

doug.gregor accepted this revision.Sep 12 2016, 4:06 PM
doug.gregor edited edge metadata.

Ahhh, much cleaner. Thanks!

This revision is now accepted and ready to land.Sep 12 2016, 4:06 PM
This revision was automatically updated to reflect the committed changes.