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

fix: NSObject offset size in SRObject being too small #60

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

hampustagerud
Copy link

Hi!

Thank you for this library, it have made mixing Rust and Swift a breeze!

However, I encountered a bug when I tried to implement a function which returned a class with 4 ints. The first member of the class always got the same weird number. It seems like the memory offset used in SRObject is not big enough, only taking up one byte whereas a pointer (on 64-bit systems) is 8 bytes.

Replacing the u8 offset with *const c_void seems to solve this issue as Rust now allocates the size for a pointer instead of just a byte. The test I also added replicates my issue and fails with the u8 type but passes after the change 🙂

@Brendonovich
Copy link
Owner

From some reading it appears you're correct, each NSObject contains a pointer.
I'm curious why existing examples continued to work without this though, also I now realise that I haven't accounted for alignment of individual fields at all 😅

@hampustagerud
Copy link
Author

Why it worked before is a mystery to me too, I have no idea really 😅

I used the Swift REPL which printed the layout of the object, not too sure how to get it otherwise outside of xcode 🙂

repl class

It looks the same for all classes I define so it really shouldn't work. Maybe there is some compiler magic at work that aligns the structure differently depending on what fields are defined in the class and by pure luck it has worked? 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants