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

Add separate macro for msg_send! that does not verify message types #121

Closed
faern opened this issue Aug 8, 2023 · 3 comments
Closed

Add separate macro for msg_send! that does not verify message types #121

faern opened this issue Aug 8, 2023 · 3 comments

Comments

@faern
Copy link

faern commented Aug 8, 2023

The verify_message feature is awesome! Sadly when you enable it all messages must pass the verification. This also adds the requirement that all messages implement Encode. I have a case where I want the message types verified for me at compile time, but a few msg_send! calls include types that do not implement Encode, as a result I must opt out of all message verification.

I can manually verify messages. But that is cumbersome, and eventually a developer will forget to do it, skipping the safety checks.

Can we add a separate msg_send_unverified! that is always identical to what msg_send! does when verify_message is disabled?

@madsmtm
Copy link

madsmtm commented Aug 8, 2023

I'd argue no, for several reasons:

  • Ideally, the macro should always take Encode types, since it helps a lot against passing types that are not valid C types (e.g. accidentally passing Vec<u8> or &str).
  • The message sending ABI arguably requires it, e.g. the current way of doing it with comparing type IDs doesn't allow otherwise valid newtypes.
  • It can be really nice for the verification to automatically be enabled when debug_assertions are, instead of requiring the user to manually enable the feature.

(I've made all of these changes in my fork objc2, so a bit of my argument is also from there, might not apply directly here).

Instead, when calling methods with arguments that don't implement Encode, you should either:

  • Implement Encode for the type (or a #[repr(transparent)] wrapper type).
  • Convert it to an equivalent type that does implement Encode.

@madsmtm
Copy link

madsmtm commented Aug 8, 2023

(In your case, that would be implementing Encode for NSOperatingSystemVersion)

@faern
Copy link
Author

faern commented Aug 8, 2023

You are completely right! This was an oversight from me. I did a quick code cleanup and did not realize NSOperatingSystemVersion was our own struct. I did not realize we could implement Encode for it ourselves. Well then, problem solved :) Thank you for the input

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

No branches or pull requests

2 participants