Initial conversion to `objc2`

[madsmtm]

~Blocked on #29~. Part of https://github.com/ryanmcgrath/cacao/issues/28.

An initial pass of just getting things to compile. Upgrade instructions for most of the breaking changes can be found in objc2's CHANGELOG, I've essentially just done as instructed by the examples there.

[ryanmcgrath]

Alright, #29 is merged and done... albeit through some extra steps because I am of the idiot clan tonight.

You should be able to rebase this onto trunk cleanly (ish) now. Thanks again for all your work!

[madsmtm]

Wonderful, no worries about the merge mistake, happens to the best of us ;)

This PR is fairly close to serviceable now (just need to do a new release of some things in objc2, and give things an extra review myself), I'll press the button when I've done so!

Note that we're currently getting a lot of deprecation warnings, will fix those in a subsequent PR.

[ryanmcgrath]

Cool, excited!

[ryanmcgrath]

I'll find time later this week (or this weekend) to review - I've been at a racing thing for most of the beginning of this week and am exhausted by the time I sit down to my laptop at night. :(

Really excited for this though! Your work is super appreciated.

[madsmtm]

No problem, take all the time you need - I'm just enjoying my summer holidays, have lots of other things to distract me with ;)

[ryanmcgrath]

I am comfortable merging this one whenever makes sense - I want to merge #48 soon, but I'm thinking it might be smarter to get the objc2 stuff out of the way first.

I'd be fine with using the 0.3.x beta series of objc2, but figured I'd ask if there's any bigger concerns you can think of before I merge this?

(Aside: msg_send_id is ergonomically so much better than what was going on with pure msg_send before, so thank you so much for your work!)

[ryanmcgrath]

(slash, could you rebase this onto trunk and we can let the updated CI pieces run?)

[madsmtm]

Just do https://github.com/ryanmcgrath/cacao/pull/48 first, I'm gonna be away for a few days so I won't have time for this before around Friday.

Re beta series, I'm fairly certain I know where I want objc2 to end up for a stable 0.3, the milestone should be fairly accurate and in order now, it's just a matter of actually finishing implementing it. I expect it to take maybe two months in total, and I think this is a fairly accurate assessment, but yeah, anything can happen.

[madsmtm]

Okay, I've rebased this now, thanks for your patience!

[ryanmcgrath]

I haven't forgotten about this - last I checked in on it, I had a few examples crash and I need to look into why. I get the sense it's probably small stuff but I've just been too busy to sit down and poke at it.

(Hoping I have time this weekend tho since I'll be mostly chilling at home)

[ryanmcgrath]

Sounds good, appreciate it!

On Mon, Aug 22, 2022 at 01:35, Mads Marquart @.***> wrote:

Just do #48 first, I'm gonna be away for a few days so I won't have time for this before around Friday.

Re beta series, I'm fairly certain I know where I want objc2 to end up for a stable 0.3, the milestone should be fairly accurate and in order now, it's just a matter of actually finishing implementing it. I expect it to take maybe two months in total, and I think this is a fairly accurate assessment, but yeah, anything can happen.

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you modified the open/close state.Message ID: @.***>

[ryanmcgrath]

Hmmm, so the examples crashing really just appears to be something in color/appkit_dynamic_color.rs. For whatever reason, when going to set it as the layer background color in view/appkit.rs, it changes out underneath from CacaoDynamicColor to a private _NSTrackingAreaAKViewHelper instance.

That symbol is, unfortunately, not simple to search. Offhand I'm unsure if there's a particular change in objc2 that'd cause this - any ideas? Maybe something funky with the way the pointer for the color is vended in color/mod.rs:to_objc?

I've also been musing on this, and I think I ultimately want to change the guts of cacao out to rely on your work over in https://github.com/madsmtm/objc2/pull/264.

My rationale here is that your binding work covers a lot of the stuff that I've done by hand here, and doing it by hand isn't really scalable (though I'd like to think it's worked well insofar as exploring how I want cacao to feel from a dev perspective). I'm leaning towards letting cacao effectively be an opinionated way to integrate with the overall lifecycle of being a "good citizen" app - e.g, app delegates, view delegates, list views, making them more or less the same across macOS/iOS due to the differences in APIs, etc.

I sincerely hope you don't feel like I'm rejecting/putting off your effort on this PR in an asshole way, for what it's worth - this is moreso me wanting to cede work to someone who's clearly on the best path and then wanting to figure out the best way to build on it. :)

(I'm somewhat eating my words here since I think I said to @simlay in the past that I dislike the bindings route, but it's just hard to justify otherwise at this point)

[madsmtm]

I've also been musing on this, and I think I ultimately want to change the guts of cacao out to rely on your work over in madsmtm/objc2#264.

My rationale here is that your binding work covers a lot of the stuff that I've done by hand here, and doing it by hand isn't really scalable (though I'd like to think it's worked well insofar as exploring how I want cacao to feel from a dev perspective). I'm leaning towards letting cacao effectively be an opinionated way to integrate with the overall lifecycle of being a "good citizen" app - e.g, app delegates, view delegates, list views, making them more or less the same across macOS/iOS due to the differences in APIs, etc.

I think that's a great idea, and it aligns quite well with my own goals! I can very well see myself becoming a regular user of that, honestly I'm still quite bad at actually using AppKit.

I sincerely hope you don't feel like I'm rejecting/putting off your effort on this PR in an asshole way, for what it's worth - this is moreso me wanting to cede work to someone who's clearly on the best path and then wanting to figure out the best way to build on it. :)

Not at all!

(I won't have a problem if you close this PR to go do the transition in your own tempo btw, it was a useful exercise for me in getting to know a few usage patterns).

As I said, I'm by far not experienced in actually using AppKit/UIKit, so feel free to give input once/if you start to use objc2/icrate. I'd also be happy to make a fix similar to https://github.com/ryanmcgrath/cacao/pull/67 that we could put behind a feature flag, or other such fixes to better support cacao.

(I'm somewhat eating my words here since I think I said to @simlay in the past that I dislike the bindings route, but it's just hard to justify otherwise at this point)

Hehe, I started with the same outlook, "but autogenerated APIs are so ugly, there must be a better way", but given enough time I eventually came to the same conclusion ;)

While you're here, I could use some input on two things though, just, you know, if you have any advice ;)

• Putting generated files in git? https://github.com/madsmtm/objc2/pull/264#issuecomment-1298604747 - this is what the windows crate does, but not sure of the downsides?
• Your thoughts on a naming scheme: https://github.com/madsmtm/objc2/issues/284

[madsmtm]

I've rebased this now, though it's still very much an initial conversion. It's difficult for me to do the entire conversion at once though, since just one small change to trunk means a hell of a rebase later on, so I'd prefer to merge this now, and then incrementally work improving things.

Note that I've kept it at objc2 v0.3.0-beta.2 for now, since the next versions enable encoding checking by default, which needs a lot of work in cacao to avoid runtime errors (actually it'll just be fixing soundness bugs, but it'll still be work).

Feel free to ask any questions about anything, if you're unsure of how objc2 works!

[ryanmcgrath]

I really appreciate your work here. :)

I think what I'll do is cut a 0.4 release with the changes that are currently sitting in the beta release, and then start a 0.5 effort which this can be merged into first thing.

which needs a lot of work in cacao to avoid runtime errors (actually it'll just be fixing soundness bugs, but it'll still be work).

Hmmm, can you give me an example? Mostly just looking to make sure I'm following along here.

[ryanmcgrath]

I wanted to get #89 in for a 0.4 release, but alas I did invoke two merge conflicts here as a result. They're small and if you don't have time for them I can look into them once I get the 0.5 pieces going, but you have my word that your PR is next. :)

[madsmtm]

which needs a lot of work in cacao to avoid runtime errors (actually it'll just be fixing soundness bugs, but it'll still be work).

Hmmm, can you give me an example? Mostly just looking to make sure I'm following along here.

Okay, so objc2 requires all arguments and return types to implement the special trait Encode, which ensures that the types are not accidentally things that don't have a stable ABI, like Vec<u8> or &str. That's being done in this PR, and already helps a lot with soundness.

But, every method in Objective-C also has the types it takes available in the runtime (somewhat limited, e.g. generics are not available, but still), and we take advantage of that to check, when debug assertions are enabled, whether the type on the Rust side matches what's declared on the Objective-C side.

An example of an error thrown by this could be the following (the issue: the code is setBorderType: 0, which means the type of the integer falls back to i32, while the function expects a NSBorderType (which is NSUInteger (which is usize))):

thread 'scrollview::test_scrollview' panicked at 'invalid message send to -[RSTScrollView_NSScrollView_13638830006937841139 setBorderType:]: expected argument at index 0 to have type code 'Q', but found 'i'', src/scrollview/mod.rs:89:25

Of course in this process, there are many false positives (and I actually recently implemented something to reduce that rate, making transitions easier), like the following (the issue: an object pointer nil is being passed as the action, even though a selector is expected, which is valid ABI-wise, but confusing to the type-system).

thread 'button::test_button' panicked at 'invalid message send to +[RSTButton_NSButton_9287538210046046270 buttonWithTitle:target:action:]: expected argument at index 2 to have type code ':', but found '@'', src/button/mod.rs:127:30

---

I wanted to get #89 in for a 0.4 release, but alas I did invoke two merge conflicts here as a result. They're small and if you don't have time for them I can look into them once I get the 0.5 pieces going, but you have my word that your PR is next. :)

No worries, it's just troublesome if I have multiple PRs that need to be rebased on top of each other, and I was a bit tired and grumpy yesterday ;).

[ryanmcgrath]

Appreciate the breakdown, makes sense to me.

And no worries - I actually feel bad for how long this has sat so definitely looking forward to merging it. Will give it one last once over tomorrow and we should be good I think.

On Tue, Aug 1, 2023 at 04:55, Mads Marquart @.***(mailto:On Tue, Aug 1, 2023 at 04:55, Mads Marquart < wrote:

which needs a lot of work in cacao to avoid runtime errors (actually it'll just be fixing soundness bugs, but it'll still be work).

Hmmm, can you give me an example? Mostly just looking to make sure I'm following along here.

Okay, so objc2 requires all arguments and return types to implement the special trait Encode, which ensures that the types are not accidentally things that don't have a stable ABI, like Vec or &str. That's being done in this PR, and already helps a lot with soundness.

But, every method in Objective-C also has the types it takes available in the runtime (somewhat limited, e.g. generics are not available, but still), and we take advantage of that to check, when debug assertions are enabled, whether the type on the Rust side matches what's declared on the Objective-C side.

An example of an error thrown by this could be the following (the issue: the code is setBorderType: 0, which means the type of the integer falls back to i32, while the function expects a NSBorderType (which is NSUInteger (which is usize))):

thread 'scrollview::test_scrollview' panicked at 'invalid message send to -[RSTScrollView_NSScrollView_13638830006937841139 setBorderType:]: expected argument at index 0 to have type code 'Q', but found 'i'', src/scrollview/mod.rs:89:25

Of course in this process, there are many false positives (and I actually recently implemented something to reduce that rate, making transitions easier), like the following (the issue: an object pointer nil is being passed as the action, even though a selector is expected, which is valid ABI-wise, but confusing to the type-system).

thread 'button::test_button' panicked at 'invalid message send to +[RSTButton_NSButton_9287538210046046270 buttonWithTitle:target:action:]: expected argument at index 2 to have type code ':', but found '@'', src/button/mod.rs:127:30

---

I wanted to get #89 in for a 0.4 release, but alas I did invoke two merge conflicts here as a result. They're small and if you don't have time for them I can look into them once I get the 0.5 pieces going, but you have my word that your PR is next. :)

No worries, it's just troublesome if I have multiple PRs that need to be rebased on top of each other, and I was a bit tired and grumpy yesterday ;).

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you modified the open/close state.Message ID: @.***>

[madsmtm]

Okay, so I went over the examples, and fixed the memory safety issues I could find by running those. That said, there are probably still more that I haven't found yet, but I feel like it's at a pretty good stage!

Note to self: The following is really useful for debugging double-frees and such:

DYLD_INSERT_LIBRARIES=/usr/lib/libgmalloc.dylib MallocStackLogging=YES NSZombieEnabled=YES MallocGuardEdges=YES MallocScribble=YES ./target/debug/examples/my_example

[ryanmcgrath]

Merged. :)

Thank you again for the sheer amount of work here, as well as for the wider ObjC work you're doing in the Rust community. It's really great that someone's picked up the torch and focused on making it all safer and more reliable overall.

I'll let this sit in trunk for a bit to see how people find it - whether we need to fix any bugs or anything - but then if all seems fine I'll eye pushing out the next (-beta, maybe) release that uses this.

[madsmtm]

Perfect!

I think before you can push a beta, we'll need to stop relying on the git dependencies that I had to add temporarily to make things work - I think I have that somewhere in a PR or a stash or something, will try to get it sorted out in the next few days.

[ryanmcgrath]

Yup, no rush!

[abhillman]

FYI: https://github.com/ryanmcgrath/cacao/issues/122