Implement conversion traits for usize/isize together with a portability lint

[petrochenkov]

An implementation for https://internals.rust-lang.org/t/lang-team-minutes-integer-conversion-and-portability-writ-large/4170
All lossless From/Into conversions are implemented, but conversions not portable between 64-bit and 32-bit platforms are reported by a special warn-by-default lint. Conversions not portable between more exotic platform pairs can also be reported by two other allow-by-default lints.

cc @nikomatsakis
r? @aturon

[petrochenkov]

nonportable_32_64 lint covers most of needs, but it's not a complete solution.
I'll also add allow-by-default lints nonportable_16_32 for people targeting specialized hardwire and nonportable_64_128 for especially paranoid developers.

A lint nonportable_x1_x2_..._xN is triggered when the conversion can be performed on at least one of xi-bit platforms, but not on all of them. Combinations like "nonportable_16_32_64" can be obtained by using two or more lints: nonportable_16_32 + nonportable_32_64.

The lints can also be improved to catch some generic cases like T: Into<usize>, T = u64.

[petrochenkov]

@aturon Could you give some early feedback, what is the current general sentiment of libs team regarding these conversions on the scale "We want this now!" -> "We'll merge this with modifications" -> "We won't merge it at this time no matter what."?

[aturon]

Oy, this has disappeared in my inbox during Rust Belt Rust -- I'm very sorry for the delay! I'll get you feedback ASAP.

[ollie27]

Would it not make more sense for the lints to only cover one pointer size each? We could have nonportable_16, nonportable_32 and nonportable_64 and they would trigger if you used a conversion not compatible with that pointer size.

[petrochenkov]

@ollie27
Yes, that may be simpler and better.
I implemented the current scheme because I originally planned a single lint - 32_64, but since we need several lints anyway the lints can be changed to cover one pointer size each.

[nikomatsakis]

Huh, this disappeared out of my inbox too somehow. =) Very cool! I'll take a look.

[bors]

☔ The latest upstream changes (presumably #37597) made this pull request unmergeable. Please resolve the merge conflicts.

[aturon]

cc @rust-lang/libs @rust-lang/lang

@petrochenkov Again, sorry for the delay from Rust Belt Rust. I've had a chance to take a look, and I'm personally very excited about this. In general, I think lints (or lint-like systems) are a great way to handle these various portability tradeoffs, and potentially also to approach things like implicit widening. This PR also closely aligns with the ongoing libs team discussion about scenarios. I'm CCing the relevant teams, but I suspect given previous discussions that people are going to be on board with something very much like this PR.

I do agree with @ollie27's suggestion; I found the mention of two sized in the lints a bit confusing.

[alexcrichton]

I'd personally prefer to hold off on this until we figure out where scenarios are going. In that world this lint would actually default to a hard error and also cover cases like:

fn foo<T: Into<u32>>(t: T) { /* ... */ }

foo(1usize); // not always portable

In general though sounds like a great case for scenarios!

[alexcrichton]

@petrochenkov do you have thoughts about the case I mentioned above where it looks like we wouldn't emit a warning?

[petrochenkov]

@alexcrichton

do you have thoughts about the case I mentioned above where it looks like we wouldn't emit a warning?

I mentioned it in a message above.
I'll implement this improvement (and also some necessary compiletest changes) if this is going to be merged. (I don't have much free time now, so I wouldn't like to spend it on a wasted effort).

[petrochenkov]

@alexcrichton
By the way, do you prefer this to be solved by scenarios?
Solely for uniformity (no special lints, everything is solved by scenarios) or due to ability to cfg out these impl early?

I'm certainly sympathetic to both these aspects.
(uniformity is good, and cfg-like preprocessing is completely precise, while I'm not so sure post-factum impl detection can be done precisely, at least not without effort).

I imagine with scenarios the user code will look something like this.
32-bit and 64-bit platforms are targeted

// No attributes, default scenarios are in action

let a: usize = 0u32.into();
let b: usize = 0u64.into(); // ERROR no such impl

Only 64-bit platforms are targeted

#![scenario(32bit_targets = false)]

let a: usize = 0u32.into();
let b: usize = 0u64.into(); // OK

Only 16-bit and 32-bit platforms are targeted

#![scenario(64bit_targets = false)]
#![scenario(16bit_targets = true)]

let a: u32 = 0usize.into(); // OK

My main concern with scenarios is schedule. They look like a huge work (especially in requirements gathering department) which is very unlikely to be done by "community" and therefore done at all unless specifically planned.

[alexcrichton]

@petrochenkov ah ok, sorry I missed that!

But yeah I personally prefer this to be done with scenarios. I'd prefer to not have this as a lint as those can be opted out of (and they're ignored in dependencies), whereas scenarios are intended to provide more static guarantees that can't be opted out of.

It's true yeah the scenarios are still aways out, but this doesn't seem particularly too pressing to justify a second system perhaps (at least to me personally)

[alexcrichton]

@petrochenkov oh also to clarify, I think what you posted for scenario organization is roughly what I was thinking (I don't have false/true in my mind, though). I would imagine:

• We have a number of scenarios, usize_at_least_{16,32,64}_bits and tag all impls appropriately.
• The usize_at_least_32_bits scenario would be on by default, but could be disabled to stop inclusion of these impls

That way most uses of .into() and/or casts would work by default, but we'd statically know what crates require that and which don't.

[alexcrichton]

(either that or we'd have nothing on by default but you could opt-in as needed)

[petrochenkov]

Another problem with scenarios is that conversion impls are in libcore, so libcore needs to be recompiled on scenario changes to apply new cfgs.

[sfackler]

Scenario changes would not involve recompiling standard libraries.

[petrochenkov]

Huh? I was under impression that scenarios is simply a "preprocessor" feature.
I've just reread the internals post about scenarios and it didn't mentioned that scenario checking is done lately, only deeply in the comments it suddenly turns out that "yeah, scenarios are checked after type checking".
But that's the most important aspect of the feature, it changes everything!

How are you going to check scenarios for trait impls? It'll require deep integration with type checker. Basically, every time the knowledge that Type: Trait is used, the relevant impl Trait for Type need to be scenario-checked, e.g. scenarios become part of type system. This is not what I expected from a configuration attribute.
Checking stability on impls requires the same thing, btw, and nobody was interested in implementing this so far.
The lint from this PR tries to do roughly the same thing, but for a single easily identifiable impl, and lints only need to be good enough, but not necessarily precise.

[alexcrichton]

Ah sorry for the misunderstanding!

How are you going to check scenarios for trait impls?

My personal assumption has been something like:

• Every item that can be loaded from metadata is tagged with scenarios
• When loading items from metadata, only configured scenarios are loaded
• When generating errors, however, all items are loaded (to indicate if scenarios need to be activated)

I was hoping that was easy enough to implement while also being 100% robust in theory.

[eddyb]

As long as you don't need to make choices inside types, then you can be clever like that with definitions, and effectively have them "feature-gated" and potentially unusable (i.e. ungating requiring rebuilding the dependency, but this should be avoided where possible).

[petrochenkov]

It's true yeah the scenarios are still aways out, but this doesn't seem particularly too pressing to justify a second system perhaps (at least to me personally)

This is where I disagree. Lack of these statically guaranteed lossless conversions is a pressing issue, it's not good that they are blocked by a feature in hand-waving stage of development, especially that they are trivially implementable themselves.
cc @briansmith @pornel @dhardy or who else was interested in this, I don't remember everyone

[brson]

I'm not excited about adding more things to std that are not available on some platforms before we understand where scenarios are going.

[alexcrichton]

The libs team discussed this a bit during triage today and some conclusions were:

• This actually mirrors exactly what we would have in terms of quantities of APIs with scenarios. That is, we'd have all these impls anyway, they'd just be behind a scenario.
• In that sense, scenarios are just trying to nudge you towards portability instead of allowing you to easily fall in a portability trap.
• We were concerned that adding scenarios to these apis might be a breaking change in the future. That is, let's say we decide to gate the u64 -> usize impl by default. This means that activation of that scenario would have to be tied to the lint level, which it may not otherwise necessarily need to.
• Unfortunately this implementation isn't bulletproof and may not provide the "nudge to being portable" that we quite want.
• The libs team wasn't entirely convinced that this was needed today, but @petrochenkov could you elaborate on your rationale for why we should fix this very soon?

[petrochenkov]

could you elaborate on your rationale for why we should fix this very soon?

Personally, I don't need this today, I needed this 1-2 years ago when I worked with integer heavy code.
Today I hoped the people pinged in #37423 (comment) to provide more motivational examples.
If the general sentiment is "won't merge", I'll close this for now, feel free to reuse the code if it'll be useful.

[kornelski]

I need this today as much as I needed it several months ago, i.e. I find Rust unpleasant to work with when non-usize types are involved, and when I use them combined with FFIs & as I don't feel like I'm getting memory safety that Rust promises.

If scenarios are coming in the near future I don't mind waiting. If they're a pie in the sky, a partial solution would be better than no solution (although I suspect type ascriptions might be required as well to make Into usable itself).

[dhardy]

I think I agree with brson to wait until scenarios are available. I don't personally need another solution today.