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

Library is highly unintuitive and lacks support for objc interop #553

Closed
Yuri6037 opened this issue Jan 13, 2024 · 4 comments
Closed

Library is highly unintuitive and lacks support for objc interop #553

Yuri6037 opened this issue Jan 13, 2024 · 4 comments
Labels
A-framework Affects the framework crates and the translator for them documentation Improvements or additions to documentation

Comments

@Yuri6037
Copy link

Yuri6037 commented Jan 13, 2024

I recently used 2 crates from this repository: icrate and objc2. Intitially I've seen the README in this repository which looked like I may have finally found a crate which will do the heavy-lifting of interacting with most Foundation types and provide Objective-C interoperability, however I've got a LOT of issues which makes me re-consider using the objc crate instead which was much lower-level and certainly has more unsafe. Nevertheless, I have found workarounds for now, I just wanted to leave a list of what I felt very unintuitive and undocumented and where I spent a considerable amount of time:

  • The Foundation feature of icrate isn't really a feature it's about 1% of one. Intuitively this looked like the Apple framework of the same name containing all base objects like NSString, NSDictionary, etc; this was however not the case. To get the actual feature you need Foundation_all, that's highly unintuitive and totally undocumented.
  • The NSMutableDictionary wrapper in icrate has a broken un-callable function named insert, only the insert_id function is callable, that's also very unintuitive and undocumented as well, considering the example for that function uses insert_id instead of insert.
  • Use of AnyObject and NSObject. I found these wrappers mostly unintuitive as you can't do anything with them (i.e: no way to turn another object into NSObject as everything requires full specified type name), the into and most other built-in trait function are not implemented or not easily usable and I couldn't find a direct way to easily .into() a NSString to turn it into a NSObject or a AnyObject. To do this one needs to call Id::<NSObject>::cast(mystring), the documentation for this behavior is nowhere to be found and I lost nearly 30 minutes figuring out how to do this by reading the very large code base as I was expecting to see a trait which would do the conversion between any wrapper struct and AnyObject/NSObject. I was nearly at giving up on this crate and returning to the good old objc crate which I know is working and wouldn't have caused such a pain to crate and fill a simple NSMutableDictionary.
  • It is impossible to turn an Id<NSError> into a *mut NSError, at least not easily, more on that later. That means you cannot easily call into other functions implemented in XCode/Objective-C. In order to do that I had to use a function I feel shouldn't be necessary: std::mem::transmute(Id::as_ptr(&obj)). Again I was looking for a .into_raw_ptr or .as_mut_ptr inside the NSError wrapper.
@madsmtm
Copy link
Owner

madsmtm commented Jan 13, 2024

Hey @Yuri6037, thanks a lot for the review, I hear your concerns and would like to resolve them.

The Foundation feature of icrate isn't really a feature it's about 1% of one. Intuitively this looked like the Apple framework of the same name containing all base objects like NSString, NSDictionary, etc; this was however not the case. To get the actual feature you need Foundation_all, that's highly unintuitive and totally undocumented.

I haven't written down that this is how it works, though I think it's somewhat documented though over on docs.rs, e.g. icrate::Foundation::NSNumber says that it requires the crate features "Foundation" and "Foundation_NSNumber".

But agree that this can be confusing, though there is a reason it is done this way: enabling "Foundation_all" takes a long time to compile, so I'd like to steer users away from using that. But I should probably rename the plain "Foundation" feature to "Foundation_core". Do you think that would that have helped you?

The NSMutableDictionary wrapper in icrate has a broken un-callable function named insert, only the insert_id function is callable, that's also very unintuitive and undocumented as well, considering the example for that function uses insert_id instead of insert.

It's not actually broken, but it only works for things that are "retainable", which works for e.g. NSView, but notably not for NSString, since that may have been a NSMutableString, and there's no way for the type-system to know.

That said, I agree that it is confusing, I'll fix that in the next (breaking) version of icrate, see #554.

Use of AnyObject and NSObject. I found these wrappers mostly unintuitive as you can't do anything with them (i.e: no way to turn another object into NSObject as everything requires full specified type name), the into and most other built-in trait function are not implemented or not easily usable and I couldn't find a direct way to easily .into() a NSString to turn it into a NSObject or a AnyObject. To do this one needs to call Id::<NSObject>::cast(mystring), the documentation for this behavior is nowhere to be found and I lost nearly 30 minutes figuring out how to do this by reading the very large code base as I was expecting to see a trait which would do the conversion between any wrapper struct and AnyObject/NSObject. I was nearly at giving up on this crate and returning to the good old objc crate which I know is working and wouldn't have caused such a pain to crate and fill a simple NSMutableDictionary.

This is being worked on, and my idea is indeed to add a helper trait, and to make let obj: Id<NSString> = obj.into() just work - but the issue is that it's hard to do fully safely, and I haven't spent enough time on it yet.

It is impossible to turn an Id<NSError> into a *mut NSError, at least not easily, more on that later. That means you cannot easily call into other functions implemented in XCode/Objective-C. In order to do that I had to use a function I feel shouldn't be necessary: std::mem::transmute(Id::as_ptr(&obj)). Again I was looking for a .into_raw_ptr or .as_mut_ptr inside the NSError wrapper.

That's because it depends on how you want to transfer ownership:

  • You want to pass the ownership out of a function: Use return Id::into_raw(obj), just like you would with Box::into_raw (recently added in 1b8a9ee, not released yet).
  • You want to pass autoreleased ownership out of a function: Use return Id::autorelease_return(obj).
  • You want to borrow from the object: Use myFunction(Id::as_ptr(&obj).cast_mut()). The transmute isn't necessary.

In general, I agree that the introduction-level docs are basically non-existent, I tried to remedy that somewhat in #494, but my prose writing skills aren't that great, and I wasn't really happy with the example.

I'll try to pick it up again when I get the time, and also to write some explanation notes, in particular on why Id exists and what you can do with it.

@madsmtm madsmtm added A-framework Affects the framework crates and the translator for them documentation Improvements or additions to documentation labels Jan 13, 2024
@Yuri6037
Copy link
Author

Yuri6037 commented Jan 14, 2024

Thanks a lot for your reply.

Hey @Yuri6037, thanks a lot for the review, I hear your concerns and would like to resolve them.

The Foundation feature of icrate isn't really a feature it's about 1% of one. Intuitively this looked like the Apple framework of the same name containing all base objects like NSString, NSDictionary, etc; this was however not the case. To get the actual feature you need Foundation_all, that's highly unintuitive and totally undocumented.

I haven't written down that this is how it works, though I think it's somewhat documented though over on docs.rs, e.g. icrate::Foundation::NSNumber says that it requires the crate features "Foundation" and "Foundation_NSNumber".

But agree that this can be confusing, though there is a reason it is done this way: enabling "Foundation_all" takes a long time to compile, so I'd like to steer users away from using that. But I should probably rename the plain "Foundation" feature to "Foundation_core". Do you think that would that have helped you?

Indeed I think Foundation_core would be much less confusing than Foundation. Also, by the way I have no idea what you talking about with takes a long time to compile, as with Rust 1.69 (due to Rust 1.70 and beyond use of LLVM 17 instead of staying at reasonable versions compatible with XCode) on my MacBook Pro 16 inch M1 Max it only takes a second... On my Mac, the only thing that takes a long time in Rust, is download (updating crates.io index).

The NSMutableDictionary wrapper in icrate has a broken un-callable function named insert, only the insert_id function is callable, that's also very unintuitive and undocumented as well, considering the example for that function uses insert_id instead of insert.

It's not actually broken, but it only works for things that are "retainable", which works for e.g. NSView, but notably not for NSString, since that may have been a NSMutableString, and there's no way for the type-system to know.

That said, I agree that it is confusing, I'll fix that in the next (breaking) version of icrate, see #554.

Thanks, I look forward for the new version to be released. Could you also update the example on crates.io which currently uses the wrong function?

Use of AnyObject and NSObject. I found these wrappers mostly unintuitive as you can't do anything with them (i.e: no way to turn another object into NSObject as everything requires full specified type name), the into and most other built-in trait function are not implemented or not easily usable and I couldn't find a direct way to easily .into() a NSString to turn it into a NSObject or a AnyObject. To do this one needs to call Id::<NSObject>::cast(mystring), the documentation for this behavior is nowhere to be found and I lost nearly 30 minutes figuring out how to do this by reading the very large code base as I was expecting to see a trait which would do the conversion between any wrapper struct and AnyObject/NSObject. I was nearly at giving up on this crate and returning to the good old objc crate which I know is working and wouldn't have caused such a pain to crate and fill a simple NSMutableDictionary.

This is being worked on, and my idea is indeed to add a helper trait, and to make let obj: Id<NSString> = obj.into() just work - but the issue is that it's hard to do fully safely, and I haven't spent enough time on it yet.

That would be fantastic and would indeed fix my problem. Also, if you get hit by the restrictions related to implementing foreign traits I'd also be fine with a custom Cast trait implemented on each object (my IDE should be able to list available functions in an object).

It is impossible to turn an Id<NSError> into a *mut NSError, at least not easily, more on that later. That means you cannot easily call into other functions implemented in XCode/Objective-C. In order to do that I had to use a function I feel shouldn't be necessary: std::mem::transmute(Id::as_ptr(&obj)). Again I was looking for a .into_raw_ptr or .as_mut_ptr inside the NSError wrapper.

That's because it depends on how you want to transfer ownership:

  • You want to pass the ownership out of a function: Use return Id::into_raw(obj), just like you would with Box::into_raw (recently added in 1b8a9ee, not released yet).
  • You want to pass autoreleased ownership out of a function: Use return Id::autorelease_return(obj).
  • You want to borrow from the object: Use myFunction(Id::as_ptr(&obj).cast_mut()). The transmute isn't necessary.

In general, I agree that the introduction-level docs are basically non-existent, I tried to remedy that somewhat in #494, but my prose writing skills aren't that great, and I wasn't really happy with the example.

I'll try to pick it up again when I get the time, and also to write some explanation notes, in particular on why Id exists and what you can do with it.

Usually what I want to do is just pass the object entirely to an Objective-C bridge which itself is used as a bridge to Swift and SwiftUI. So I mostly need Id::into_raw but unfortunately I usually avoid using git based crates I prefer using versioned crates. I even do that for my own crates...

@madsmtm
Copy link
Owner

madsmtm commented Jan 14, 2024

takes a long time to compile

It may not be obvious if you've cached everything, but try running cargo clean && cargo build in a project that uses Foundation_all, this takes like 10 seconds on my M2 Pro, which can quickly add up if used by a dependency, and which makes it unsuitable for Bevy, which tries to keep this stuff as small as possible.

In any case, I've opened #558 for tracking the renaming.

Thanks, I look forward for the new version to be released. Could you also update the example on crates.io which currently uses the wrong function?

I assume you mean the example on docs.rs, if so, then yes, will do.

Usually what I want to do is just pass the object entirely to an Objective-C bridge which itself is used as a bridge to Swift and SwiftUI. So I mostly need Id::into_raw.

Hmm, I'm not sure that's true! Objective-C usually requires that you autorelease things that you pass back to it, otherwise you'll get a memory leak.

Perhaps you could show me a sample of your code, then I can help you with what's required to avoid that?

I usually avoid using git based crates I prefer using versioned crates. I even do that for my own crates...

Yeah, will release it probably somewhere around the end of January.

@madsmtm madsmtm added this to the objc2 v0.6 milestone Sep 17, 2024
@madsmtm
Copy link
Owner

madsmtm commented Sep 17, 2024

I'm going to close this, since the actionable points are tracked elsewhere; reviewing your points again, we have currently:

Again, thank you for the review!

@madsmtm madsmtm closed this as completed Sep 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-framework Affects the framework crates and the translator for them documentation Improvements or additions to documentation
Projects
None yet
Development

No branches or pull requests

2 participants