Skip to content

Some ObjC method overrides map to invalid Dart overrides #1220

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
liamappelbe opened this issue Jun 25, 2024 · 4 comments · Fixed by #1714
Closed

Some ObjC method overrides map to invalid Dart overrides #1220

liamappelbe opened this issue Jun 25, 2024 · 4 comments · Fixed by #1714
Assignees
Labels
lang-objective_c Related to Objective C support package:ffigen

Comments

@liamappelbe
Copy link
Contributor

NSNotificationCenter has a addObserver:selector:name:object: method, where the object param has type id (the super type of all objects, like Dart's Object), which makes sense. Meanwhile, NSDistributedNotificationCenter overrides that method but changes the type of the object param to an NSString*. This appears to be a bug in the API.

This is not a big deal for the ObjC compiler, which is generally much less strict than Dart. But when we generate the Dart wrapper API, having a method on NSDistributedNotificationCenter that overrides a method on NSNotificationCenter with more specific arg types is a compile error.

To fix this we'll need to adjust the types of any method args that conflict like this so that the subtype's method args are as general as the supertype's. We should also do the converse for the return type.

@liamappelbe liamappelbe added package:ffigen lang-objective_c Related to Objective C support labels Jun 25, 2024
@liamappelbe liamappelbe self-assigned this Jun 25, 2024
@liamappelbe liamappelbe moved this to In progress in ObjC/Swift interop Jun 25, 2024
@liamappelbe
Copy link
Contributor Author

There's also a similar issue where an ordinary method is overridden by a getter.

This one is trickier because this method/getter difference is not allowed in either direction in Dart. The getter cannot be converted into an ordinary method because there may be a setter that it would conflict with. So we have to convert the ordinary method into a getter instead. This could cause further conflicts with other classes in the inheritance tree, so we'll have to keep converting every such method in the tree into a getter. Specfically, we walk up the tree until we hit a type that doesn't have this method, then convert this method to a getter for every subtype of that subtree.

@liamappelbe
Copy link
Contributor Author

I think the fix for the method/getter clash is going to be fiddly enough that it's probably time to bite the bullet and implement the transformer refactor: #1259

@dcharkes
Copy link
Collaborator

dcharkes commented Jul 8, 2024

To fix this we'll need to adjust the types of any method args that conflict like this so that the subtype's method args are as general as the supertype's. We should also do the converse for the return type.

Drive by comment: I believe this will still work correctly when the bindings happen to be in different FFIgen generated bindings and the import feature is used. E.g. the base bindings don't need to know some extended bindings have type errors, because we'd adjust the types of the extended bindings instead of the base bindings. We should probably make sure this is covered by a test case.

@stuartmorgan-g
Copy link

There's also a similar issue where an ordinary method is overridden by a getter.

If this issue is going to cover this case, I would recommend retitling it; there's nothing invalid about this in Objective-C.

@liamappelbe liamappelbe changed the title Some Apple APIs have invalid overrides Some ObjC method overrides map to invalid Dart overrides Aug 22, 2024
@github-project-automation github-project-automation bot moved this from Todo to Done in ObjC/Swift interop Nov 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lang-objective_c Related to Objective C support package:ffigen
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants