Skip to content
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

Obj-C protocol conformance is dropped from types #1462

Open
stuartmorgan opened this issue Aug 26, 2024 · 11 comments · May be fixed by #1959
Open

Obj-C protocol conformance is dropped from types #1462

stuartmorgan opened this issue Aug 26, 2024 · 11 comments · May be fixed by #1959
Labels
lang-objective_c Related to Objective C support package:ffigen

Comments

@stuartmorgan
Copy link

While migrating plugin code from native to Dart, at one intermediate stage I had this:

- (instancetype)initWithPlayerItem:(AVPlayerItem *)item
                      viewProvider:(id<FVPViewProvider>)viewProvider
                         AVFactory:(id<FVPAVFactory>)avFactory
                displayLinkFactory:(id<FVPDisplayLinkFactory>)displayLinkFactory;

which was turned into:

FVPVideoPlayer initWithPlayerItem_viewProvider_AVFactory_displayLinkFactory_(
      AVPlayerItem item,
      objc.ObjCObjectBase viewProvider,
      objc.ObjCObjectBase avFactory,
      objc.ObjCObjectBase displayLinkFactory)

Without any of the protocols in the types, it's incredibly easy to call incorrectly, and also much less self-documenting. Ideally Obj-C protocols would be reflected as Dart abstract interfaces that are implements'd.

@liamappelbe liamappelbe added package:ffigen lang-objective_c Related to Objective C support labels Aug 26, 2024
@liamappelbe
Copy link
Contributor

liamappelbe commented Aug 26, 2024

We can experiment with this, but I held off doing it in the initial implementation for a few reasons:

  1. Dart's interface checking is way stricter than ObjC's, so there will probably be use cases that would compile fine in ObjC that don't compile in Dart. I'd need to make sure there's an escape hatch that lets you ignore the check.
  2. Not all interfaces declare which protocols they implement, so I'd need to proactively check which protocols are implemented
  3. I'm not sure how to handle protocols that have optional methods.
    • I suppose I could just make the codegenned protocol empty, so we get some type checking without actually verifying that the methods exist. You still wouldn't be able to call methods on the protocol from Dart land.
  4. Also not sure how to handle args that implement multiple protocols: - (void)method:(id<Foo, Bar>)obj;
    • We could codegen a FooBar interface, but that gets complicated because now that's another interface that the implementation needs to implement. Better solution is probably to just take the first protocol and ignore the rest.

@stuartmorgan
Copy link
Author

1. Dart's interface checking is way stricter than ObjC's, so there will probably be use cases that would compile fine in ObjC that don't compile in Dart.

Do you have an example of this to illustrate the kind of problem that would happen?

2. Not all interfaces declare which protocols they implement, so I'd need to proactively check which protocols are implemented

Can you elaborate on this one? Are you talking about conformsToProtocol: runtime checks? If so, I wouldn't expect that to be reflected in the type system.

3. I'm not sure how to handle protocols that have optional methods.

  • I suppose I could just make the codegenned protocol empty, so we get some type checking without actually verifying that the methods exist. You still wouldn't be able to call methods on the protocol from Dart land.

I think starting with just the type would be a big improvement. (Type and required methods would be even better, for better code completion and Dart-side IDE experience.)

4. Also not sure how to handle args that implement multiple protocols: - (void)method:(id<Foo, Bar>)obj;

I can't remember ever seeing that used in practice, so it probably wouldn't matter much what happened.

@liamappelbe
Copy link
Contributor

An example for both 1 and 2:

@protocol SomeProtocol<NSObject>
@required
- (int32_t)instanceMethod;
@end


@interface UndeclaredImplementation : NSObject
- (int32_t)instanceMethod;
@end

@implementation UndeclaredImplementation : NSObject
- (int32_t)instanceMethod {
  return 123;
}
@end


@interface Consumer : NSObject
- (int32_t)callInstanceMethod:(id<SomeProtocol>)protocol;
@end

@implementation Consumer : NSObject
- (int32_t)callInstanceMethod:(id<SomeProtocol>)protocol {
  return [protocol instanceMethod];
}
@end

int32_t foo() {
  return [[Consumer new] callInstanceMethod: [UndeclaredImplementation new]];
}

This compiles and runs, and foo returns 123 as expected. But the corresponding Dart bindings would not be invocable, (you couldn't write foo in Dart).

All I get from clang is a warning:

protocol_test.m:139:46: warning: sending 'UndeclaredImplementation *' to parameter of incompatible type 'id<SomeProtocol>'
  return [[Consumer new] callInstanceMethod: [UndeclaredImplementation new]];
                                             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
protocol_test.m:129:49: note: passing argument to parameter 'protocol' here
- (int32_t)callInstanceMethod:(id<SomeProtocol>)protocol;
                                                ^

Whether or not this matters depends on whether there are any APIs that do this in practice. I suppose we can just put this behind a config flag, so users can opt out if necessary.

@stuartmorgan
Copy link
Author

stuartmorgan commented Aug 27, 2024

All I get from clang is a warning:

protocol_test.m:139:46: warning: sending 'UndeclaredImplementation *' to parameter of incompatible type 'id<SomeProtocol>'
  return [[Consumer new] callInstanceMethod: [UndeclaredImplementation new]];

It's Objective-C; almost any type violation will just give you a warning. I would not expect to be able to write foo in Dart because foo is wrong.

If "possible to write/run in Obj-C" were our criteria for mirroring, then we would need to throw away the whole type system, not just protocols. This code also builds and runs with nothing but warnings, for instance:

NSArray* notAnArray = @"actually a string";
[self thisParamTypeIsString:notAnArray];

Whether or not this matters depends on whether there are any APIs that do this in practice.

The code in your example isn't an API, it's an implementation. I don't see any problems in the APIs in that code. Unless you mean that UndeclaredImplementation is third-party code that was supposed to explicitly conform to SomeProtocol? If that's the case, couldn't that be trivially worked around by just subclassing UndeclaredImplementation on either side of the language divide, and adding the protocol to the subclass, then downcasting? Or just casting to dynamic?

@kekland
Copy link

kekland commented Aug 27, 2024

I got around this issue by creating a "proxy" class which takes the protocol object in the init function and calls the methods on that object. For example, for FlutterTextureRegistry:

@objc public class FlutterTextureRegistryProxy: NSObject {
    public init(textures: FlutterTextureRegistry) {
        self.textures = textures
    }
    
    let textures: FlutterTextureRegistry;
    
    @objc public func registerTexture(texture: any FlutterTexture) -> Int64 {
        return self.textures.register(texture);
    }
    
    @objc public func textureFrameAvailable(textureId: Int64) {
        return self.textures.textureFrameAvailable(textureId);
    }
    
    @objc public func unregisterTexture(textureId: Int64) {
        return self.textures.unregisterTexture(textureId);
    }
}

But it would be great to have the protocols be generated to avoid this hassle

@liamappelbe
Copy link
Contributor

@kekland I think your work around is solving a different problem. Are you trying to make a protocol invocable? My fix probably won't help with that.

The fix I'm planning will add a Dart interface for each protocol, have the classes implement those interfaces, and then pass around those interfaces instead of ObjCObjectBase. These protocol interfaces would be empty placeholders (at least initially).

I guess if we want to support methods that return protocols, the protocol interface can't actually be abstract. I'll have to make it also be a concrete implementation of the protocol, so that I have something to construct and return. This will also act as an escape hatch for users to work around the type restrictions if necessary. I suppose if we code gen all the required methods, then this would make the protocol invocable.

@liamappelbe liamappelbe added this to the ffigen 15.0.0 milestone Aug 27, 2024
@kekland
Copy link

kekland commented Aug 28, 2024

@liamappelbe ah, you're right, was a long day yesterday :D

but yes, it would be great to have both of those things

@liamappelbe
Copy link
Contributor

liamappelbe commented Jan 29, 2025

Working on this now. The design is a bit tricky though. Requirements:

  • id can conform to 0 or more protocols: id<A, B, C, D>
  • id<A, B> is a subtype of id<A>. That is, we can pass id<A, B> to a method expecting id<A>
  • These conformances should be reflected in both the generic id and in specific interfaces. If interface Foo conforms to A and B we should be able to pass it to a method expecting id<A>.

Dart doesn't support variadic generics, so it's not obvious how to implement this. In C++ I could do something like this, and it would capture all the semantics:

template <typename... Protocols> class ObjCObject : public Protocols... {};

The brute force solution is to codegen a separate ObjCObject for every combination of protocols mentioned in the APIs.

class ObjCObject_A_B_C implements A implements B implements C {}
class ObjCObject_A_B implements A implements B {}
class ObjCObject_A implements A {}
class ObjCObject_B_C implements B implements C {}

class Foo implements ObjCObject_A_B {}

The problem with codegenning them like this, aside from being verbose and ugly, is that one package's id<A, B> will be incompatible with another package's id<A, B>, even if they share A and B.

Another option, to get around the lack of variadic generics, would be to have a bunch of objects with specific numbers of protocols. We could add these to package:objective_c, or code gen them as needed. The trade-off here is that if we put them in package:objective_c we have to generate them up to some fixed number of protocols (eg up to 8), but if we codegen them on demand we have the same compatibility issues as above.

class ObjCObject {}
class ObjCObject1<T1 extends ObjCProtocol> implements T1 {}
class ObjCObject2<T1 extends ObjCProtocol, T2 extends ObjCProtocol> implements T1 implements T2 {}

As written this doesn't quite work, because we can't directly use T1/T2 as supertypes. So we have to write it like this instead:

class ConformsTo<T extends ObjCProtocol> {}

class ObjCObject {}
class ObjCObject1<T1 extends ObjCProtocol> implements ConformsTo<T1> {}
class ObjCObject2<T1 extends ObjCProtocol, T2 extends ObjCProtocol> implements ConformsTo<T1>, ConformsTo<T2> {}

class Foo implements ConformsTo<A>, ConformsTo<B> { ...

ConformsTo is just a marker type. In particular, it won't inherit any of the methods from the protocols, but that's fine. I think this solution is the cleanest one that captures the semantics.

EDIT: Actually this doesn't quite capture the semantics. If class Foo implements ConformsTo<A>, ConformsTo<B> we still won't be able to pass it to a method expecting ObjCObject1<A>. We will be able to pass it to a method expecting ConformsTo<A> though. The issue with passing it as ConformsTo<A> is that we can't write methods/variables etc that accept or return id<A, B>.

@liamappelbe
Copy link
Contributor

liamappelbe commented Jan 29, 2025

After some investigation I don't think it's possible to capture all the semantics of protocol conformance. The best we can do is have interfaces implement every protocol they conform to, and then write methods etc that require at most one of the protocols to be implements. In other words, if we see an ObjC method that accepts id<A, B>, we'll codegen it as if it accepts id<A>. Hopefully APIs requiring multiple protocols are pretty rare.

// Codegen for protocols will change from abstract final to abstract interface, and extend ObjCProtocolBase.
abstract interface class ProtocolA extends objc.ObjCProtocolBase {
  // Contents of protocol remains the same: only static methods/fields.
  // Implementers won't actually inherit anything.
  ...
}

abstract interface class ProtocolB extends objc.ObjCProtocolBase { ... }

// Interfaces now directly implement all their protocols. Don't need ConformsTo under this approach.
class SomeInterface extends objc.NSObject implements ProtocolA, ProtocolB {
  // Methods accepting id still take ObjCObjectBase.
  void idMethod(objc.ObjCObjectBase x);

  // Methods accepting id<ProtocolA> take ProtocolA directly.
  void protocolAMethod(ProtocolA x);

  // Methods accepting id<ProtocolA, ProtocolB> take ProtocolA. The ProtocolB is lost.
  void protocolABMethod(ProtocolA x);
}

@liamappelbe
Copy link
Contributor

@stuartmorgan Will a static typing solution like ^this^ work? Or are there a lot of cases where an interface implements a protocol without declaring it statically? If I have an interface Foo and a protocol Bar, and Foo implements all Bar's methods, but doesn't declare that it conforms to the protocol like @interface Foo : NSObject <Bar>, would you still expect to be able to pass Foo to a method that takes a id<Bar>? If so I'll have to scrap all this static typing stuff and just add a runtime check to the method.

@stuartmorgan
Copy link
Author

If I have an interface Foo and a protocol Bar, and Foo implements all Bar's methods, but doesn't declare that it conforms to the protocol like @interface Foo : NSObject <Bar>, would you still expect to be able to pass Foo to a method that takes a id<Bar>?

  • Would I expect to be able to do this in Obj-C? Yes1
  • Would I deliberately do this in Obj-C? No; I prefer not to hamstring my compiler's ability to find mistakes 🙂 (In an odd coincidence: I can't think the last time I'd seen code that did this, and then yesterday I was reviewing a PR that tried to add that pattern. I said no because I don't think it's a good idea.)
  • Would I expect to be able to do this in Dart code? No, because I don't expect a language projection to be the same as an actual language, and I know that Dart is strongly typed unlike Obj-C. I can imagine a more junior engineer not thinking about that distinction, or someone who had a different answer to the previous question thinking that we should have made it possible to write similar code in Dart by projecting differently, but my opinion is that it's fine for us to say, "Sorry, Dart is a strongly typed language, so if you want to write code that uses duck typing you should write that part in Obj-C instead." (Again though, I think this is a bad pattern, so my answer is undoubtedly biased by that.)

What would be great is if Dart supported Swift's solution for this use case, which is to allow adding protocol conformance via class extensions, so you can make, in essence, strongly typed ducks2. Obviously that would be a language level feature though, not a bindings generator feature.

Footnotes

  1. Even if Obj-C, there's no guarantee that this will actually work the way the callers wants, because if any code actually checks protocol conformance (conformsToProtocol:), it won't pass.

  2. This doesn't work if Foo doesn't actually implement all of Bar's methods, just the ones that you "know" the code will call, but I view that as a feature because that's exactly the kind of thing that makes the pattern you described a bad idea IMO.

@liamappelbe liamappelbe linked a pull request Jan 31, 2025 that will close this issue
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: Todo
Development

Successfully merging a pull request may close this issue.

4 participants
@stuartmorgan @liamappelbe @kekland and others