[PATCH] D66831: [ObjC] Fix type checking for qualified id block parameters.

2020-05-06 Thread Volodymyr Sapsai via Phabricator via cfe-commits
vsapsai added a comment. Patch for the suggested compatibility flag is available at https://reviews.llvm.org/D79511 Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66831/new/ https://reviews.llvm.org/D66831 ___

[PATCH] D66831: [ObjC] Fix type checking for qualified id block parameters.

2020-05-04 Thread James Y Knight via Phabricator via cfe-commits
jyknight added a comment. In D66831#2019125 , @vsapsai wrote: > Agree that is a mistake in `NSItemProvider` API. The solution I offer is not > to revert the change but to add cc1 flag > `-fcompatibility-qualified-id-block-type-checking` and to make the

[PATCH] D66831: [ObjC] Fix type checking for qualified id block parameters.

2020-05-04 Thread Volodymyr Sapsai via Phabricator via cfe-commits
vsapsai added a comment. Agree that is a mistake in `NSItemProvider` API. The solution I offer is not to revert the change but to add cc1 flag `-fcompatibility-qualified-id-block-type-checking` and to make the driver for Darwin platforms to add this flag. Thus developers using Apple SDKs will

[PATCH] D66831: [ObjC] Fix type checking for qualified id block parameters.

2020-05-04 Thread James Y Knight via Phabricator via cfe-commits
jyknight added a comment. Hi, it seems that this change is incompatible with the `[NSItemProvider loadItemForTypeIdentifier:options:completionHandler:]` method's expected (but extremely weird and unusual!) usage. Per

[PATCH] D66831: [ObjC] Fix type checking for qualified id block parameters.

2020-03-27 Thread Volodymyr Sapsai via Phabricator via cfe-commits
vsapsai added a comment. There should be no error for `blockWithParam = genericBlockWithParam;` because `blockWithParam` is called with `I *` and it is safe to substitute `genericBlockWithParam`. Basically you have I *blockArg; id genericParam = blockArg; And for `genericBlockWithParam =

[PATCH] D66831: [ObjC] Fix type checking for qualified id block parameters.

2020-03-27 Thread ChatWyn via Phabricator via cfe-commits
chatwyn added a comment. Hi: we're in the process of testing with Clang 10 and noticed a behavior change caused by this commit. Consider the following piece of code: @protocol P @end @protocol Q @end @interface I @end void a() { void (^genericBlockWithParam)(id );

[PATCH] D66831: [ObjC] Fix type checking for qualified id block parameters.

2020-03-03 Thread Volodymyr Sapsai via Phabricator via cfe-commits
vsapsai added a comment. In D66831#1902176 , @smeenai wrote: > Thanks for that explanation; that makes sense. Do you think that the > `@interface J : I` would also ideally be an error then? Theoretically, I think `@interface J : I` should also trigger

[PATCH] D66831: [ObjC] Fix type checking for qualified id block parameters.

2020-03-02 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai added a comment. Thanks for that explanation; that makes sense. Do you think that the `@interface J : I` would also ideally be an error then? Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66831/new/ https://reviews.llvm.org/D66831

[PATCH] D66831: [ObjC] Fix type checking for qualified id block parameters.

2020-03-02 Thread Volodymyr Sapsai via Phabricator via cfe-commits
vsapsai added a comment. In D66831#1901972 , @smeenai wrote: > I'm not very familiar with Objective-C semantics. Does saying `@interface J : > I ` mean we're overriding the conformance being specified by `@interface I > `? In that case, the Clang 10

[PATCH] D66831: [ObjC] Fix type checking for qualified id block parameters.

2020-03-02 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai added a comment. Sorry for the late feedback here – we're in the process of testing with Clang 10 and noticed a behavior change caused by this commit. Consider the following piece of code: @protocol P @end @protocol Q @end @interface I @end @interface J : I

[PATCH] D66831: [ObjC] Fix type checking for qualified id block parameters.

2019-08-28 Thread Nico Weber via Phabricator via cfe-commits
thakis added a comment. Yes, that makes sense, thanks. Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66831/new/ https://reviews.llvm.org/D66831 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D66831: [ObjC] Fix type checking for qualified id block parameters.

2019-08-28 Thread Volodymyr Sapsai via Phabricator via cfe-commits
vsapsai added a comment. In D66831#1650015 , @thakis wrote: > We're getting a bunch of errors looking like > `../../AppsListViewController.m:11:37: error: incompatible block pointer > types assigning to 'void (^)(__strong id)' from 'void >

[PATCH] D66831: [ObjC] Fix type checking for qualified id block parameters.

2019-08-28 Thread Nico Weber via Phabricator via cfe-commits
thakis added a comment. We're getting a bunch of errors looking like `../../AppsListViewController.m:11:37: error: incompatible block pointer types assigning to 'void (^)(__strong id)' from 'void (^)(AppCollectionViewCell *__strong)'` on code that looks fairly harmless to me. It looks

[PATCH] D66831: [ObjC] Fix type checking for qualified id block parameters.

2019-08-27 Thread Volodymyr Sapsai via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL370130: [ObjC] Fix type checking for qualified id block parameters. (authored by vsapsai, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit:

[PATCH] D66831: [ObjC] Fix type checking for qualified id block parameters.

2019-08-27 Thread Volodymyr Sapsai via Phabricator via cfe-commits
vsapsai added a comment. Thanks for the fast review. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66831/new/ https://reviews.llvm.org/D66831 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D66831: [ObjC] Fix type checking for qualified id block parameters.

2019-08-27 Thread Alex Lorenz via Phabricator via cfe-commits
arphaman accepted this revision. arphaman added a comment. This revision is now accepted and ready to land. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66831/new/ https://reviews.llvm.org/D66831 ___ cfe-commits mailing list

[PATCH] D66831: [ObjC] Fix type checking for qualified id block parameters.

2019-08-27 Thread Volodymyr Sapsai via Phabricator via cfe-commits
vsapsai marked 2 inline comments as done. vsapsai added inline comments. Comment at: clang/test/SemaObjC/block-type-safety.m:141 genericBlock = block; +block = genericBlock; // expected-error {{incompatible block pointer types assigning to 'NSAllArray *(^)(id)' from

[PATCH] D66831: [ObjC] Fix type checking for qualified id block parameters.

2019-08-27 Thread Volodymyr Sapsai via Phabricator via cfe-commits
vsapsai created this revision. vsapsai added reviewers: erik.pilkington, arphaman. Herald added subscribers: ributzka, dexonsmith, jkorous. When checking if block types are compatible, we are checking for compatibility their return types and parameters' types. As these types have different