Add non-portable infallible conversion traits for usize/isize

[SimonSapin]

This follows the precedent of the std::os module for handling (non-)portablity.
See the doc-comment of the new module for what problem this is solving and how.

[rust-highfive]

r? @shepmaster

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

[SimonSapin]

This is an alternative to #70460 following the precedent of std::os, without a portability lint.
@rust-lang/libs, what do you think of this approach at a high level?

Details to be resolved:

• Module placement. Inside std::os is not appropriate since there is nothing OS-specific here. target is chosen after #[cfg(target_pointer_width)].
• Trait names
• Method names. I was hoping to use from and into and have them not collide with From and Into since there is no overlap in (source type, target type) pairs. However I still ran into error[E0034]: multiple applicable items in scope.
• Have rustdoc show a "This is only supported on […]" badge like it does for modules in std::os. rustdoc has code for parsing the #[cfg] attributes that are used, but it does not appear to be used here.
• Tracking issue number in #[unstable] the attribute

Rendered copy of the doc-comment:

Target-specific functionality

Background: From and Into are portable

The From and Into traits are in the prelude, so they don’t need to be imported with use. They provide conversions that are infallible. For example, From<u32> for u64 is implemented:

assert_eq!(u64::from(7_u32), 7_u64);

… but From<u64> for u32 is not, because larger values cannot be represented:

let x = 7_u64;
// error: `std::convert::From<u32>` is not implemented for `u16`
let _ = u32::from(x); // (What if `x` was `7_000_000_000_000_u64` ?)

Additionally, From and Into impls are portable: they only exist when they are infallible regardless of the target. For example, converting u64 to usize would be infallible if the target happens to have 64-bit pointers, but the From trait still doesn’t allow it:

let x = 7_u64;
// error: `std::convert::From<u64>` is not implemented for `usize`
let _ = usize::from(x);

This conversion is possible with the TryFrom trait:

use std::convert::TryFrom;

assert_eq!(usize::try_from(7_u64).unwrap(), 7_usize);

However, because try_from is fallible, this may require using .unwrap(). In a less trivial case, it may not be obvious to readers of the code that this can never panic (on 64-bit platforms).

Non-portable conversion traits

This module provides integer conversion traits that are only available for some targets. They provide the "missing" integer conversions for code that only cares about portability to some platforms. For example, an application that only runs on 64-bit servers could use:

#![feature(non_portable_conversion)]

# // Make this test a no-op on non-64-bit platforms
# #[cfg(target_pointer_width = "64")]
use std::target::PointerWidthGe64From;

# #[cfg(target_pointer_width = "64")]
assert_eq!(usize::target_from(7_u64), 7_usize);

Here the code does not have a panic branch at all. In return, it does not compile on some targets.

#![feature(non_portable_conversion)]

// These two never exist at the same time:
use std::target::PointerWidthGe64From;
use std::target::PointerWidthLe32From;
// error[E0432]: unresolved import

The mandatory import std::target is an indication to writers and readers of non-portable, target-specific code. This is similar to the std::os module.

The trait names contain Ge or Le which mean “greater then or equal” and “less than or equal” respectively, like in the ge or le methods of the PartialOrd trait.

[SimonSapin]

@rust-lang/rustdoc In what cases does rustdoc render #[cfg] attributes as a blue "This is supported on […] only" badge like https://doc.rust-lang.org/std/os/windows/index.html ?

[cuviper]

Module placement. Inside std::os is not appropriate since there is nothing OS-specific here. target is chosen after #[cfg(target_pointer_width)].

How about core/std::arch? That wouldn't just be intrinsics anymore, but it seems appropriate.

[SimonSapin]

This functionality does not fit the description in the current doc-comment for core::arch, but we could potentially rephrase that. Still, I feel that the module would have two rather distinct "categories" of things in it.

[ollie27]

In what cases does rustdoc render #[cfg] attributes as a blue

You'll need to also add a matching #[doc(cfg(...))] attribute because that's what rustdoc is actually rendering.

[GuillaumeGomez]

Sorry, wanted to hit the "comment" button and failed badly... About the doc thing, just like @ollie27 said.

[nikomatsakis]

One downside, or perhaps positive side, of this approach is that -- because it introduces a different trait -- it means that generic code based on Into and so forth won't be able to benefit from it.

This means that if I have some function like

fn foo<T: Into<u64>>() { .. }

or whatever, I can't use it with foo::<usize>, even if I am happy with the portability limitations. Basically, it solves the narrow problem of doing the conversion but limits "composability".

On the other hand, it may be that in this particular case, composability probably comes up less often (though I'm not sure). Further, I imagine that there are cases where the conversion from usize to u64 might be "hidden" in a series of generic functions and hence the portability lint would either not fire (despite the actual dependency) or else fire at a relatively obscure place, relatively high up in the set of functions, which wouldn't be a problem here.

[Amanieu]

Having to manually import a (long) trait and then use target_into feels like enough work that most people will probably opt to just use .try_into().unwrap() instead (especially if we add TryInto to the prelude for 2021 edition).

I can see several ways to ease this burden:

• We could simply provide From impls for all conversions support by the current target. These impls would be specially marked to produce a portability lint when used. This lint can be disabled with #[allow] to indicate that the code is not portable to different pointer sizes.
• We could add the PointerWidth* traits to the prelude: simply using target_into() should be enough of an indicator that the code is non-portable.

[petrochenkov]

Separate traits mostly defeat the purpose in this case, IMO.
The "mostly portable" conversions like u32 -> usize are supposed to work with any existing code expecting From/Into and are not required to have any special indication like separate method names.

[SimonSapin]

The name could be shorter, for example PtrGe32::to.

If .try_into().unwrap() is acceptable, is nothing needed over the status quo?

Having these traits in the prelude would defeat the point of making import from std::target visible, thus following the precedent of std::os.

The "mostly portable" conversions are supposed to […] not required to have any special indication

I don’t think there is consensus or precedent for the "mostly" part.

[nikomatsakis]

@SimonSapin

I don’t think there is consensus or precedent for the "mostly" part.

I'm somewhat surprised by this. Wasn't that indeed the point of the portability RFC? I guess you're saying that, in the time since, consensus has eroded, or circumstances have changed?

I personally still think it makes sense to have a notion of 'mostly portable' and to make code targeting 'common platforms' ergonomic and easy. I think that trying to encourage "too much" portability is frustrating and likely to lead people to stop carrying altogether: but making it easy and painless to be portable amongst 32-bit/64-bit and win/linux/mac etc is something most people are happy to do.

[shepmaster]

In return, it does not compile on some targets.

To me, this is the biggest win. Just this last week, I wrote this code:

let expected_len_us = usize::try_from(expected_len)
    .expect("Only designed to run on 32-bit systems or higher");

I'd much rather that the person compiling the code for a 16-bit platform get a compilation error than some poor end user of the server get a runtime panic.

[Dylan-DPC-zz]

moving this to waiting-on-team as team consensus seems to be needed here in order to proceed

[LukasKalbertodt]

I am not particularly fond of this solution. As @Amanieu said, using these traits is quite a hassle and most users will just use try_into().unwrap() or even as instead. I am still slightly confused how I would use this module in practice.

Without having read all the discussion about this topic, the initial PR seemed like quite a nice solution to me. It works with the standard Into/From traits and I can easily specify what point-widths I am targeting at the root of my crate by #![allow(_)]ing or #![deny(_)]ing the corresponding lints. Then, too, it would not compile on the targets my code does not support.

Maybe I missed it, but have you explained what you think is the advantage of your proposal over the original PR?

[Dylan-DPC-zz]

@SimonSapin any updates on this?

[SimonSapin]

I’m closing this PR since it’s not clear to me what next step could be to go toward consensus, and I don’t have bandwidth to pursue this at the moment. Anyone should feel free to pick it up again (or propose something different).