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

More Encode and RefEncode implementations #52

Merged
merged 4 commits into from
Oct 30, 2021
Merged

More Encode and RefEncode implementations #52

merged 4 commits into from
Oct 30, 2021

Conversation

madsmtm
Copy link
Owner

@madsmtm madsmtm commented Oct 30, 2021

I chose not to implement RefEncode for Box because of the hazzle with no_std stuff, and because I don't think it'll be very useful.

RefEncode being Encode::String on i8 and u8 makes RefEncode on BOOL wrong (apparently BOOL is more special than just a type-alias over char?), but this was a problem before (just limited to raw pointers to BOOL; now it's also a problem with references to BOOL).

However, the newtype Bool doesn't suffer from this issue, and using pointers to BOOL is very rare in practice; so I think proper documentation on BOOL (which this PR adds) should solve this issue well enough.

This was not done previously because of uncertainty with the `char*` being the "String" encoding.

But investigation shows that:
- @encode(signed char) -> "c"
- @encode(unsigned char) -> "C"
- @encode(char*) -> "*"
- @encode(char**) -> "^*"

And here it doesn't matter if we replace `char` with e.g. `uint8_t`, the encoding scheme still works like this.

Luckily `RefEncode` is designed in just the right way that it can actually handle this really nicely!
Allows `*const *const c_void` and such.
@madsmtm madsmtm mentioned this pull request Oct 30, 2021
80 tasks
/// You should not rely on this encoding to exist for any other purpose (since
/// `()` is not FFI-safe)!
///
/// TODO: Figure out a way to remove this.
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Calling attention to this. Possible solution would be to change encode_fn_pointer_impl! and similar macros to handle both the case where R: Encode and where R = ()

@madsmtm madsmtm merged commit 1ddd898 into master Oct 30, 2021
@madsmtm madsmtm deleted the more-encode branch October 30, 2021 17:59
@madsmtm madsmtm added this to the objc2 v0.3 milestone Apr 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement Encode for more types
1 participant