Make transmuting from fn item types to pointer-sized types a hard error. #34198

eddyb · 2016-06-10T09:53:12Z

Closes #19925 by removing the future compatibility lint and the associated workarounds.
This is a [breaking-change] if you transmute from a function item without casting first.
For more information on how to fix your code, see #19925.

rust-highfive · 2016-06-10T09:53:25Z

r? @nrc

(rust_highfive has picked a reviewer for you, use r? to override)

eddyb · 2016-06-10T09:54:11Z

r? @nikomatsakis

I'll start a crater run to see if there's any measurable impact.

eddyb · 2016-06-10T14:34:05Z

Crater report: https://gist.github.com/eddyb/c9a976482be44529e3cdd79ca97db0ce.

This looks pretty bad, much worse than what I remember. It seems a bunch of crates haven't fixed their code, although the warning only now got into beta, so nobody staying on stable has ever seen it.

eddyb · 2016-06-10T15:04:08Z

Looks like nix was fixed in https://github.com/nix-rust/nix, but crates.io still has a 3-month old version.
cc @carllerche

nikomatsakis · 2016-06-10T15:13:08Z

I've been meaning to alter the future compatibility warnings so that they
always (at least) warn, even if the user (or cap lints) sets them to allow.
That might also help.
On Jun 10, 2016 11:04 AM, "Eduard-Mihai Burtescu" notifications@github.com
wrote:

Looks like nix was fixed in https://github.com/nix-rust/nix, but crates.io
still has a 3-month old version.
cc @carllerche https://github.com/carllerche

—
You are receiving this because you were assigned.
Reply to this email directly, view it on GitHub
#34198 (comment), or mute
the thread
https://github.com/notifications/unsubscribe/AAJeZsyi-DJ3AF-sLviSdqoLNAO-aHiFks5qKX0PgaJpZM4Iyy6A
.

bluss · 2016-06-10T15:58:54Z

@nikomatsakis That's great, because it alerts users that depend on outdated (unfixed) versions.

kamalmarhubi · 2016-06-10T16:14:00Z

I just got the ability to publish and published nix v0.6.0 which builds on nightly.

eddyb · 2016-06-10T20:13:14Z

Crater report 2: https://gist.github.com/eddyb/defd09acfd0c1d6036334d147d7fc040.

@kamalmarhubi As I feared, releasing 0.6.0 wasn't enough (e.g. notify still uses 0.5.0).
Could you release 0.5.1 with 0.5.0's code + the transmute fix?

EDIT: Even worse, 0.4.2 is also still in use (e.g. signal), so that would need a new release (0.4.3) too.
Also 0.3.9 (only in utp). You can see all of the versions used by other crates.io crates at https://crates.io/crates/nix/reverse_dependencies.
AFAICT, you wouldn't need more than 0.5.1, 0.4.3 and 0.3.10 to cover everyone.

eddyb · 2016-06-10T21:21:54Z

@SSheldon objc-0.1.8 is still broken, all but the last 2 transmute calls (the ones in the macro) in src/message.rs need casts on their arguments.
You would have to release 0.1.9 to solve this, as some crates are still using 0.1.8.

kamalmarhubi · 2016-06-11T09:13:53Z

@eddyb I just published a v0.5.1, which builds on nightly. Publishing things in earlier release series is harder as we don't have git tags for those releases. I can try to narrow it down, but it will probably not be today.

eddyb · 2016-06-11T09:38:14Z

@kamalmarhubi Thanks for the help with this!
For releases without tags, it might be easier to get the package from crates.io (I don't know how to do that without adding it as a dependency of something and then looking in ~/.cargo/registry/src/), modify it and republish that.
It might be possible to of course bisect git to find out what commit matches in the source (modulo metadata/files that were not included in crates.io?).

eddyb · 2016-06-11T09:43:25Z

@GuillaumeGomez @gkoz Reminder that glib-0.0.8 is still broken, gtk-rs/glib#116 needs to be released ~~in 0.0.9 (not 0.1.*)~~ (EDIT: see below, releasing 0.0.9 won't do any good anyway 😞).

bluss · 2016-06-11T09:50:44Z

0.0.x versions don't upgrade automatically to anything, so nothing can be done.

eddyb · 2016-06-11T09:56:16Z

@bluss I just checked and indeed that's the case - why do we even allow depending on such an "unfixable" version?!

eddyb · 2016-06-11T11:00:44Z

@nikomatsakis I've went through all of the 50 root regressions and here's what I found:

Partially fixed and/or yet unpublished fixes

nix, glib and objc discussed above.
@tomaka Fix various warnings from rustc tomaka/hlua#73 needs to be published as 0.1.9.
@Ameliorate hlua_master has the same problem.
@tickbh td_rlua has a similar problem in 0.1.3 (fix would have to go in 0.1.4).

Yet unfixed

@arturoc gstreamer-0.4.0 at https://github.com/arturoc/gstreamer1.0-rs.
@swizard0 simple-signal-1.0.5 with just one problematic transmute.
@Ginurx zmq-rs-0.1.7 with also just one problematic transmute.

GuillaumeGomez · 2016-06-11T11:16:05Z

@eddyb: Fine fine!

Ameliorate · 2016-06-11T16:11:05Z

@eddyb I've yanked hlua_master, so you can check that one off. It wasn't really meant to be used by anyone. I just made it because at the time hlua's github master branch didn't match what was on crates.io.

SSheldon · 2016-06-11T18:33:01Z

@eddyb, if the crates relying on objc 0.1 were upgraded to use 0.2, would that be satisfactory? Or must there be a fix for 0.1? The fix for 0.1 would be a lot of copy-pasting and I'm not super keen on it 😅

Looking at the crates that require 0.1:

clipboard: I could go send a PR to update this
metl: it actually has already updated to 0.2 but not published a new version (any plans to publish a new version @burtonageo?)
glutin_cocoa: this is a defunct fork of cocoa
uikit: this is mine, I'll go update it

If it's amenable to you I might just take this as an opportunity to get clipboard and uikit updated.

eddyb · 2016-06-11T18:36:35Z

@SSheldon That sounds great, thank you! Having seen the code, I agree that not fixing the transmute-laden version is better.

eddyb · 2016-06-11T18:53:13Z

@tomaka Is glutin_cocoa completely unused anymore?
@alexcrichton Is yanking all the versions of such a package an acceptable policy?

alexcrichton · 2016-06-11T18:55:55Z

Yeah it'll work in terms of preventing new dependencies on it, but it's the same standard "lockfiles continue to work" semantics which means it's not 100% fixed if someone's using it.

If no one's using it though then shouldn't matter too much either way.

eddyb · 2016-06-11T18:58:24Z

@alexcrichton It has... downloads. I wonder if that's all from crater though, heh.

nikomatsakis · 2017-02-17T22:50:40Z

@eddyb see also #19925 (comment), which I think is a nifty idea, though orthogonal-ish from this PR

eddyb · 2017-02-28T15:41:52Z

Latest crater run shows the same regressions as before - see #34198 (comment).

eddyb · 2017-02-28T15:42:48Z

@nikomatsakis Oh I almost forgot about the Option thing, I guess I'll do that now.

EDIT: done!

nikomatsakis · 2017-02-28T21:44:23Z

src/librustc/diagnostics.rs

+of `foo` tells us precisely what function is being called.
+
+As noted above, coercions mean that most code continues to work just fine
+before and after this behavior was implemented. However, you can tell the


probably we don't want to say "this behavior was implemented" here...I would just say "Coercions mean that most code doesn't have to be concerned with this distinction."

Ah that's a better phrasing. I did try to remove temporal indicators, this one was trickiest.

nikomatsakis

r=me modulo the extended error description change I suggested.

I still think we may want to consider making extern "C" fn types be pointer-sized, but that's a separate change that ought to go through the RFC process IMO.

eddyb · 2017-02-28T21:48:12Z

@bors r=nikomatsakis

bors · 2017-02-28T21:48:13Z

📌 Commit 7650afc has been approved by nikomatsakis

bors · 2017-02-28T22:17:20Z

⌛ Testing commit 7650afc with merge a6e96ad...

bors · 2017-02-28T22:19:37Z

💔 Test failed - status-appveyor

eddyb · 2017-02-28T22:22:39Z

@bors retry

chocolatey network error

bors · 2017-02-28T22:23:12Z

⌛ Testing commit 7650afc with merge d5d59ca...

bors · 2017-02-28T22:24:45Z

💔 Test failed - status-appveyor

alexcrichton · 2017-02-28T22:28:26Z

@bors: retry * chocolatey error

…

On Tue, Feb 28, 2017 at 4:24 PM, bors ***@***.***> wrote: 💔 Test failed - status-appveyor <https://ci.appveyor.com/project/rust-lang/rust/build/1.0.2170> — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#34198 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAD95Nz5DBy-GXPj64lncYliSbJ8W4Uhks5rhJ6vgaJpZM4Iyy6A> .

bors · 2017-03-01T03:49:29Z

⌛ Testing commit 7650afc with merge 5bddaa4...

bors · 2017-03-01T05:28:04Z

💔 Test failed - status-travis

eddyb · 2017-03-01T08:48:43Z

@bors retry

I can't see the log... because S3?

bors · 2017-03-01T10:03:48Z

⌛ Testing commit 7650afc with merge 691eba1...

…el-bad, r=nikomatsakis Make transmuting from fn item types to pointer-sized types a hard error. Closes #19925 by removing the future compatibility lint and the associated workarounds. This is a `[breaking-change]` if you `transmute` from a function item without casting first. For more information on how to fix your code, see #19925.

bors · 2017-03-01T11:59:50Z

☀️ Test successful - status-appveyor, status-travis
Approved by: nikomatsakis
Pushing 691eba1 to master...

rust-highfive assigned nrc Jun 10, 2016

rust-highfive assigned nikomatsakis and unassigned nrc Jun 10, 2016

eddyb force-pushed the you're-a-bad-transmute-and-you-should-feel-bad branch 2 times, most recently from 41ffc25 to ddf2469 Compare June 10, 2016 10:00

kamalmarhubi mentioned this pull request Jun 10, 2016

Release procedure and policy nix-rust/nix#363

Closed

tomaka mentioned this pull request Jun 11, 2016

Publish 0.1.9 tomaka/hlua#80

Merged

SSheldon mentioned this pull request Jun 11, 2016

Publish 0.1.2 to crates.io aweinstock314/rust-clipboard#32

Closed

eddyb force-pushed the you're-a-bad-transmute-and-you-should-feel-bad branch from fb4663b to e1c3a6f Compare February 28, 2017 11:30

eddyb added the relnotes Marks issues that should be documented in the release notes of the next release. label Feb 28, 2017

eddyb force-pushed the you're-a-bad-transmute-and-you-should-feel-bad branch from e1c3a6f to 6b99df6 Compare February 28, 2017 16:20

eddyb requested a review from nikomatsakis February 28, 2017 16:32

nikomatsakis reviewed Feb 28, 2017

View reviewed changes

nikomatsakis requested changes Feb 28, 2017

View reviewed changes

Make transmuting from fn item types to pointer-sized types a hard error.

7650afc

eddyb force-pushed the you're-a-bad-transmute-and-you-should-feel-bad branch from 6b99df6 to 7650afc Compare February 28, 2017 21:48

bors merged commit 7650afc into rust-lang:master Mar 1, 2017

eddyb deleted the you're-a-bad-transmute-and-you-should-feel-bad branch March 1, 2017 13:41

Make transmuting from fn item types to pointer-sized types a hard error. #34198

Make transmuting from fn item types to pointer-sized types a hard error. #34198

Conversation

eddyb commented Jun 10, 2016 • edited Loading

rust-highfive commented Jun 10, 2016

eddyb commented Jun 10, 2016

eddyb commented Jun 10, 2016

eddyb commented Jun 10, 2016

nikomatsakis commented Jun 10, 2016

bluss commented Jun 10, 2016

kamalmarhubi commented Jun 10, 2016

eddyb commented Jun 10, 2016 • edited Loading

eddyb commented Jun 10, 2016

kamalmarhubi commented Jun 11, 2016

eddyb commented Jun 11, 2016 • edited Loading

eddyb commented Jun 11, 2016 • edited Loading

bluss commented Jun 11, 2016

eddyb commented Jun 11, 2016

eddyb commented Jun 11, 2016 • edited Loading

Partially fixed and/or yet unpublished fixes

Yet unfixed

GuillaumeGomez commented Jun 11, 2016

Ameliorate commented Jun 11, 2016 • edited Loading

SSheldon commented Jun 11, 2016

eddyb commented Jun 11, 2016

eddyb commented Jun 11, 2016

alexcrichton commented Jun 11, 2016

eddyb commented Jun 11, 2016

nikomatsakis commented Feb 17, 2017

eddyb commented Feb 28, 2017

eddyb commented Feb 28, 2017 • edited Loading

nikomatsakis Feb 28, 2017

Choose a reason for hiding this comment

eddyb Feb 28, 2017

Choose a reason for hiding this comment

nikomatsakis left a comment

Choose a reason for hiding this comment

eddyb commented Feb 28, 2017

bors commented Feb 28, 2017

bors commented Feb 28, 2017

bors commented Feb 28, 2017

eddyb commented Feb 28, 2017

bors commented Feb 28, 2017

bors commented Feb 28, 2017

alexcrichton commented Feb 28, 2017 via email

bors commented Mar 1, 2017

bors commented Mar 1, 2017

eddyb commented Mar 1, 2017

bors commented Mar 1, 2017

bors commented Mar 1, 2017

eddyb commented Jun 10, 2016 •

edited

Loading

eddyb commented Jun 10, 2016 •

edited

Loading

eddyb commented Jun 11, 2016 •

edited

Loading

eddyb commented Jun 11, 2016 •

edited

Loading

eddyb commented Jun 11, 2016 •

edited

Loading

Ameliorate commented Jun 11, 2016 •

edited

Loading

eddyb commented Feb 28, 2017 •

edited

Loading