Page MenuHomePhabricator

[ObjC] Fix type checking for qualified id block parameters.
ClosedPublic

Authored by vsapsai on Tue, Aug 27, 2:40 PM.

Details

Summary

When checking if block types are compatible, we are checking for
compatibility their return types and parameters' types. As these types
have different variance, we need to check them in different order.

rdar://problem/52788423

Diff Detail

Repository
rL LLVM

Event Timeline

vsapsai created this revision.Tue, Aug 27, 2:40 PM
vsapsai marked 2 inline comments as done.Tue, Aug 27, 2:44 PM
vsapsai added inline comments.
clang/test/SemaObjC/block-type-safety.m
141 ↗(On Diff #217500)

This is an existing error, just added a test for it.

148–149 ↗(On Diff #217500)

And this is a new error. Earlier we would emit an error on blockWithParam = genericBlockWithParam assignment instead.

This revision is now accepted and ready to land.Tue, Aug 27, 3:54 PM

Thanks for the fast review.

This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptTue, Aug 27, 5:27 PM

We're getting a bunch of errors looking like ../../AppsListViewController.m:11:37: error: incompatible block pointer types assigning to 'void (^)(__strong id<AppCellProtocol>)' from 'void (^)(AppCollectionViewCell *__strong)' on code that looks fairly harmless to me. It looks something like this:

@protocol AppCellProtocol <NSObject> ... @end
@interface AppCollectionViewCell : NSObject <AppCellProtocol> ...@enderby 

@interface Cell : NSObject
@property(nonatomic, copy) void (^buttonPressed)(id<AppCellProtocol> cell);
@end

@implementation Bar
- (void) f {
  __weak __typeof(self) weakSelf = self;
  cell.buttonPressed = ^(AppCollectionViewCell *pressedCell) {
    // ...
  };
}
@end

The code doesn't say __strong anywhere as far as I can tell; it looks like regular protocol code.

Is this expected?

We're getting a bunch of errors looking like ../../AppsListViewController.m:11:37: error: incompatible block pointer types assigning to 'void (^)(__strong id<AppCellProtocol>)' from 'void (^)(AppCollectionViewCell *__strong)' on code that looks fairly harmless to me. It looks something like this:

@protocol AppCellProtocol <NSObject> ... @end
@interface AppCollectionViewCell : NSObject <AppCellProtocol> ...@enderby 

@interface Cell : NSObject
@property(nonatomic, copy) void (^buttonPressed)(id<AppCellProtocol> cell);
@end

@implementation Bar
- (void) f {
  __weak __typeof(self) weakSelf = self;
  cell.buttonPressed = ^(AppCollectionViewCell *pressedCell) {
    // ...
  };
}
@end

The code doesn't say __strong anywhere as far as I can tell; it looks like regular protocol code.

Is this expected?

That's the intention of the change. When you call void (^buttonPressed)(id<AppCellProtocol> cell), it is only enforced that the argument conforms to AppCellProtocol. But you can do

cell.buttonPressed = ^(AppCollectionViewCell *pressedCell) {
  // call some AppCollectionViewCell method not present in AppCellProtocol
};

and get unrecognized selector error. The change should help to avoid such situations.

Does my reasoning look correct to you?

Yes, that makes sense, thanks.