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

msg_send! still compiles when the return type cannot be inferred (relying on behavior that will change in future versions of Rust) #62

Closed
mrmekon opened this issue Mar 30, 2018 · 5 comments

Comments

@mrmekon
Copy link

mrmekon commented Mar 30, 2018

This is probably not a bug that needs to be fixed in rust-objc, but a warning to developers since it causes runtime crashes with unclear errors.

Previously, both of these worked, assuming setThing: returned void:

let _ = msg_send![obj, setThing: 1];
msg_send![obj, setThing: 0];

They worked because Rust inferred unknown types as (). As of recent Rust nightly builds, unknown types are inferred as a new '!' type (see: rust-lang/rust#48950).

Unfortunately, Rust will build it and the crashes occur at runtime. Depending on which features you have compiled rust-objc with, you either get a panic at the calling line, a panic in objc_exception, or a segfault.

The error messages are entirely unhelpful unless you build rust-objc with the verify_message feature, which catches it at compile-time.

Fix by forcing the type back to ():

let _: () = msg_send![obj, setThing: 1];
@kvark
Copy link

kvark commented Apr 13, 2018

Any plans for a proper fix? This affects us greatly in metal-rs.

@alecmocatta
Copy link

@kvark I believe the fix should be either in metal-rs: adding let () = before relevant msg_sends, or rustc changing what type inference defaults to on unconstrained types. Either way, looks like rustc will revert and avoid stabilising till ramifications are clearer rust-lang/rust#49691

@SSheldon
Copy link
Owner

I didn't even know it would compile if you omit the let _: () = when using msg_send!! Sorry to hear this caused folks trouble. Unfortunately I'm not aware of anything that we can do in the objc crate to avoid this...

If the rustc change gets shipped again, it seems like the only way to fix would be to add the type annotations. I'll close this out because I'm not aware of any action objc can take, but if anyone has an idea let me know!

Aaron1011 referenced this issue in alacritty/alacritty Jun 14, 2018
@jClaireCodesStuff
Copy link

I have an idea!

I don't use MacOS, but I came across this issue when researching the stabilization of the never type !. There's a line of thought right now saying "oh, we should be cautious about ! because some unsafe code becomes unsound". So I'm digging into the examples and seeing if there's a reasonably straightforward fix for them. I think there may be a fix here.

Undefined behavior can be non-local, so it's hard to blame one thing specifically, but I would first look to fn msg_send_fn and fn msg_send_super_fn in the platform modules. (link to x64_64, but the same reasoning should apply to all platforms)

pub fn msg_send_fn<R>() -> Imp promises, in plain English: "if you name any type R, as long as it is sized, I will tell you which external fn obeys the type rules for returning R." The problem is that "returning !" means "never returning", so this promise is broken for !.

If R is constrained, R: Encode or similar, then ! would be excluded. (Encode won't be automatically implemented for `!`` because it's an unsafe trait.) If I understand correctly it should cover most/all of the actual FFI calls. It might be necessary or desirable to make a new marker trait for this purpose.

The immediate effect will be that R: Encode bound bubbling up to functions that call msg_send_fn within the crate. One of these functions implements the expansion of send_msg!, so the compiler will also enforce let _: () = (or let () = - valid destructuring match for ()) for void-returning calls. Otherwise it should give an error about inferring R: objc::Encode.

It is a breaking change; some users have been omitting let _: () = (or the equivalent let () =) and that worked by accident because Rust has set unconstrained types to (). I don't think it's possible to detect the never_type feature gate, but if so that could be used to only reject code that would crash anyway. A more difficult but reasonable solution might be to detect compiler version.

SimonSapin added a commit to SimonSapin/rust-objc that referenced this issue Oct 15, 2019
This is a breaking change, to prevent misuses such as
SSheldon#62
@SSheldon
Copy link
Owner

I haven't fully understood this issue. I thought this was unavoidable, but after @SimonSapin's comment rust-lang/rust#65355 (comment), it turns out that the un-annotated usage only compiles because of a quirk in objc's macro and the type resolution.

For example, the following code will not compile:

mem::zeroed();

You get an error stating "type annotations needed". I would expect that a bare msg_send! like the following behaves the same way and would fail to compile:

msg_send![obj, release];

But as described above, that code is accepted! It infers a () return type from msg_send!. Why is this? Well, it turns out that the following code will compile:

if false {
    panic!("panic")
} else {
    mem::zeroed()
};

For some reason, the compiler sees that one branch is ! and the other is _ and decides that the type must be (). There is a comparable code structure inside the msg_send! macro which results in the same behavior. With the change to stabilize !, the compiler will instead infer that the type here is !, and undefined behavior ensues.

With that understanding, there is in fact something we can do here: change the structure of the code inside the macro so that bare msg_send! is not accepted, and correctly errors requiring an explicit return type. This will prevent users from relying on this quirk in type resolution while unblocking the Rust devs on stabilizing !.

@SSheldon SSheldon reopened this Oct 16, 2019
@SSheldon SSheldon changed the title Type inference of msg_send! with void returns does not work with nightly msg_send! still compiles when the return type cannot be inferred (relying on behavior that will change in future versions of Rust) Oct 16, 2019
bors-servo pushed a commit to servo/core-foundation-rs that referenced this issue Oct 16, 2019
Fixes so the compiler can infer msg_send! return types

Currently, due to a quirk in Rust's type inference interacting with the structure of the `msg_send!` macro, a `()` return type will be inferred when the compiler cannot otherwise determine the return type. This behavior is expected to change, and in the future could resolve to a `!` return type, which results in undefined behavior.

Linting has previously been added for this in rust-lang/rust#39216, but it did not catch these cases due to SSheldon/rust-objc#62. An upcoming version of objc will be fixed to stop hiding these errors, at which point they will become compile errors.

This change fixes these errors and allows cocoa to compile with the fixed version of objc.
bors-servo pushed a commit to servo/core-foundation-rs that referenced this issue Oct 17, 2019
Fixes so the compiler can infer msg_send! return types (backport to 0.18)

This is a backport of #340 onto the 0.18 release.

Currently, due to a quirk in Rust's type inference interacting with the structure of the `msg_send!` macro, a `()` return type will be inferred when the compiler cannot otherwise determine the return type. This behavior is expected to change, and in the future could resolve to a `!` return type, which results in undefined behavior.

Linting has previously been added for this in rust-lang/rust#39216, but it did not catch these cases due to SSheldon/rust-objc#62. An upcoming version of objc will be fixed to stop hiding these errors, at which point they will become compile errors.

This change fixes these errors and allows cocoa to compile with the fixed version of objc.
Osspial pushed a commit to rust-windowing/winit that referenced this issue Oct 18, 2019
* Fix so the compiler can infer msg_send! return type

Currently, due to a quirk in Rust's type inference interacting with the
structure of the msg_send! macro, a () return type will be inferred when
the compiler cannot otherwise determine the return type. This behavior
is expected to change, and in the future could resolve to a ! return
type, which results in undefined behavior.

Linting has previously been added for this in rust-lang/rust#39216, but
it did not catch these cases due to SSheldon/rust-objc#62. An upcoming
version of objc will be fixed to stop hiding these errors, at which
point they will become compile errors.

This change fixes these errors and allows winit to compile with the
fixed version of objc.

* Bump cocoa to 0.19.1
jdm pushed a commit to servo/surfman that referenced this issue Oct 19, 2019
Currently, due to a quirk in Rust's type inference interacting with the
structure of the msg_send! macro, a () return type will be inferred when
the compiler cannot otherwise determine the return type. This behavior
is expected to change, and in the future could resolve to a ! return
type, which results in undefined behavior.

Linting has previously been added for this in rust-lang/rust#39216, but
it did not catch these cases due to SSheldon/rust-objc#62. objc has been
fixed to stop hiding these errors as of version 0.2.7, and they are now
compile errors.

This change fixes these errors and allows compiling with the
latest version of objc.
hecrj pushed a commit to hecrj/winit that referenced this issue Oct 20, 2019
…1227)

* Fix so the compiler can infer msg_send! return type

Currently, due to a quirk in Rust's type inference interacting with the
structure of the msg_send! macro, a () return type will be inferred when
the compiler cannot otherwise determine the return type. This behavior
is expected to change, and in the future could resolve to a ! return
type, which results in undefined behavior.

Linting has previously been added for this in rust-lang/rust#39216, but
it did not catch these cases due to SSheldon/rust-objc#62. An upcoming
version of objc will be fixed to stop hiding these errors, at which
point they will become compile errors.

This change fixes these errors and allows winit to compile with the
fixed version of objc.

* Bump cocoa to 0.19.1
cmyr added a commit to linebender/druid that referenced this issue Oct 22, 2019
As described in [rust-objc#62][], the msg_send macro could cause
UB in certain situations when built with a version of rustc that
includes the ! type.

rust-objc#62: SSheldon/rust-objc#62
cmyr added a commit to linebender/druid that referenced this issue Oct 22, 2019
As described in [rust-objc#62][], the msg_send macro could cause
UB in certain situations when built with a version of rustc that
includes the ! type.

[rust-objc#62]: SSheldon/rust-objc#62
cmyr added a commit to linebender/druid that referenced this issue Oct 22, 2019
As described in [rust-objc#62][], the msg_send macro could cause
UB in certain situations when built with a version of rustc that
includes the ! type.

[rust-objc#62]: SSheldon/rust-objc#62
cmyr added a commit to linebender/druid that referenced this issue Oct 23, 2019
As described in [rust-objc#62][], the msg_send macro could cause
UB in certain situations when built with a version of rustc that
includes the ! type.

[rust-objc#62]: SSheldon/rust-objc#62
cmyr added a commit to linebender/druid that referenced this issue Oct 23, 2019
As described in [rust-objc#62][], the msg_send macro could cause
UB in certain situations when built with a version of rustc that
includes the ! type.

[rust-objc#62]: SSheldon/rust-objc#62
cmyr added a commit to linebender/druid that referenced this issue Oct 23, 2019
As described in [rust-objc#62][], the msg_send macro could cause
UB in certain situations when built with a version of rustc that
includes the ! type.

[rust-objc#62]: SSheldon/rust-objc#62
cmyr added a commit to linebender/druid that referenced this issue Oct 23, 2019
As described in [rust-objc#62][], the msg_send macro could cause
UB in certain situations when built with a version of rustc that
includes the ! type.

[rust-objc#62]: SSheldon/rust-objc#62
cmyr added a commit to linebender/druid that referenced this issue Oct 23, 2019
As described in [rust-objc#62][], the msg_send macro could cause
UB in certain situations when built with a version of rustc that
includes the ! type.

[rust-objc#62]: SSheldon/rust-objc#62
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

5 participants