Rust: Tracking issue for platform-dependent-API TryFrom impls involving usize/isize

[SimonSapin]

Background

For converting between different primitive integer types we have impls of the From trait (and so of TryFrom<Error=!> through the generic impl<T, U> TryFrom<T> for U where U: From<T>) for conversions that always succeed (for example u8 to u32), and TryFrom<Error=TryFromIntError> for other conversions (for example u32 to u8, which return Err(_) on values that would overflow).

When usize or isize is involved however, some conversions can be fallible or infallible depending on the target platform’s pointer width. (For example u64 to usize on 64-bit v.s. 32-bit platforms.)

There is desire to make such impls infallible with From whenever possible. Since APIs would be different on the target platform, their usage should be gated on a portability lint that is not implemented yet. (The lint would additionally need to be able to "see" that a non-portable impl is used indirectly through a generic impl like impl<T, U> TryFrom<T> for U where U: From<T> or impl<T, U> Into<U> for T where U: From<T>.)

A downside of this is that even code that would be portable because it doesn’t care about the error type (because it only tests for the presence of an error, or works in both case though e.g. the From conversion inside the ? operator) would still need top opt into "non-portability" in order to silence the lint.

In order to be able to stabilize the TryFrom trait without waiting for the portability lint to be implemented, https://github.com/rust-lang/rust/pull/49305 removed the affected impls. They are those marked NP in the table in https://github.com/rust-lang/rust/pull/49305#issuecomment-376293243:

• TryFrom<usize> for u16, u32, u64, u128, i32, i64, i128
• TryFrom<isize> for i16, i32, i64, i128
• TryFrom<_> for usize: u32, u64, u128
• TryFrom<_> for isize: u16, u32, u64, u128, i32, i64, i128

Alternatives

We want to have these impls eventually, but need to chose between:

• Different APIs based on the target platform, with a portability lint
• Give up on static-infallibility-on-some-platforms and make these impls apparently-fallible with TryFrom<Error=TryFromIntError> on all platforms.

[clarfonthey]

More discussion on this is also in #48593.

[SimonSapin]

Also:

• https://github.com/rust-lang/rfcs/issues/1748
• https://internals.rust-lang.org/t/lang-team-minutes-integer-conversion-and-portability-writ-large/4170

[SimonSapin]

Not having these impls at all is a "hole" in the functionality of this trait: https://github.com/rust-lang/rust/pull/49305#issuecomment-376881109

[Kixunil]

An idea:

impl TryFrom<u64> for usize {
    type Error = SizeConversionError<u64>;

    // fn try_from ...
}

impl<T> From<SizeConverstionError<T>> for TryFromIntError {
// fn from ...
}

// Mark use of this trait with portability lint
// The philosophy is similar to e.g. AsRawFd trait
trait SizeConversionExt {
    type Value;

    fn conversion_never_fails(self) -> Self::Value;
}

// pseudo attribute - don't know the real one
#[cfg(pointer_size = 64)]
impl SizeConversionExt for Result<u64, SizeConverstionError<u64>> {
    type Value = u64;

    fn conversion_never_fails(self) -> Self::Value {
        match self {
            Ok(val) => val,
            Err(e) => e.uninhabited,
        }
    }
}

// Portable code:
fn portable<T: TryInto<usize>>(val: T) {
    match val.try_into() {
        // ...
    }
}

// For X86-64
fn non_portable(val: u64) {
    #[allow(non-portable)]
    use /*something::something*/SizeConversionExt;
    let val: usize = val.try_into().conversion_never_fails();
    // Do whatever with val
}

[clarfonthey]

I think that making all of these conversions go through TryFrom is the best route. Infallible conversions can be marked as Error = ! and then any matching without an Err branch would trigger the portability lint. We still get the same performance gains, but people know by nature of saying try_from instead of from that it won't always work.

[SimonSapin]

@clarcharr I’m not sure what you mean. Note that though a generic impl, any From conversion also has a corresponding TryFrom<Error=!> conversion. Are you suggesting that the missing impls should not have From, only TryFrom, but with an error type that varies based on the platform? That’s less of a portability hazard than an impl that may or may not be there, but only slightly: if you do anything with the error value it still has a type that varies across platforms. So this would still need a portability lint that does not exit yet.

I also don’t know what you mean by performance: whatever the error type, with sufficiently advanced inlining the optimizer can see that a given impl only ever returns Ok and compile .unwrap() (or whatever) into a no-op. The reason people have been asking for platform-specific impls so to be able to write code that is statically apparent to never fail on the platforms they care about, so that .unwrap() is not needed in the first place.

[ltratt]

Bitter experience over many years of porting code that performs integer conversions suggests to me that people will use infallible conversions on code that will later be run on platforms where the conversion does indeed fail, creating a portability headache. Even in the presence of a lint, this stuff often leaches deeply into the design of a program, including things which a lint won't pick up, where it can become extremely difficult to unpick.

As @SimonSapin says, with sufficiently good optimisations -- which, IIRC the last time I looked at this is already the case -- both the From and TryFrom case have identical performance for infallible conversions. For the few cases where one wants to make clear that a conversion is infallible, I think it's probably even arguable that using a completely different API (e.g. from a crate for this purpose) is a good idea.

[Kixunil]

I agree with @ltratt. I've recently participated on porting some code from 32 to 64 bit and it wasn't fun.

That being said, performance is fine in simple cases, but I guess it would get worse with more complex code where errors are propagated and handled by the caller. That's why I'd prefer what I wrote above: have a special error type with a private field of type ! or () depending on conversion and platform.

[kevincox]

Is there any evidence of a performance degradation for having TryFromIntError instead of !? It seems like the should be very simple for the compiler to optimize and it would be nice to have the consistency for all target-sized types.

Just to be clear. I'm in favour of having all usize and isize TryFrom implementations return an Error type even if it isn't possible on the platform.

[SimonSapin]

This issue is not at all about performance. It’s about choosing (or not) to have APIs vary across targets.

[kevincox]

I must have missed some argument. What is the downside with having TryFromIntError on all targets?

[SimonSapin]

@kevincox I personally think that’s what we should do anyway, but as explained in “Background” of the original message of this thread it prevents having non-portable portability-lint-gated From impls in the future: https://github.com/rust-lang/rust/pull/37423, https://github.com/rust-lang/rust/pull/49305#issuecomment-376231122, since through the generic TryFrom where From impl they would overlap.

[kevincox]

Thanks for explaining. One solution would be https://github.com/rust-lang/rfcs/blob/master/text/1210-impl-specialization.md. I believe this would allow the blanket impl TryFrom for From to exist while still having concrete impls for usize and similar. I would need to carefully re-read the specificity rules to ensure this is possible.

However until then (or if that never comes to fruition) I think I would still prefer having the TryFrom and the blanket impl for From as opposed to having the non-portable From impls available.

[SimonSapin]

The TryFrom<T> for U where U: From<T> impl is not under discussion here. The two options are, for example for converting u64 to usize:

• usize: TryFrom<u64, Error=TryFromIntError> on all platforms (possible now, my preference), or
• usize: From<u64> (and usize: TryFrom<u64, Error=!> through the generic impl) on 64-bit platforms where the conversion is infallible and usize: TryFrom<u64, Error=TryFromIntError> on platforms where usize is smaller (so the conversion can overflow). Using this conversion (whatever the platform, even if you don’t care about the TryFrom error type and your could would in fact be portable) causes a portability warning (lint) that can be silenced by explicitly marking the code as known to be non-portable. This portability lint is not implemented yet, and the details of how it would work exactly are a bit up in the air.

[ltratt]

Has there been any progress towards making a decision on this? Selfishly, I would like not to be stuck using a nightly from March (because the removed usize conversions mean my code doesn't compile with anything newer)...

[tobz1000]

I would like this to be finalised too. To me it seems that first option (use Error=TryFromIntError regardless of platform) is the more popular one, and more forward-compatible. I would rather see the performance issue dealt with through potential future compiler optimisation.

[ollie27]

Prior to #49305 the error types for TryFrom conversions involving usize and isize depended on the platform. Given that these impls were used in the wild, is anyone aware of any problems that caused?

[Kixunil]

I would prefer the error type to carry information about from which integer the conversion was attempted as without it compiler optimizations will be probably impossible without whole-program analysis.

That being said, I prefer cross-platform solution. (Note: AVR has 16 bit addressing.)

[clarfonthey]

Wouldn't it be possible to make these all implement TryFrom with an unstable error type, so that we at least have TryFrom until this is resolved?

[SimonSapin]

@Kixunil Do you mean a single error type that would contain some kind of enum? What would you do with that info? What would be the public APIs? Or if you mean separate error types, that’s potentially a lot of types for all combinations of “infallible if usize is at {least,most} {16,32,64,128} bits”.

@clarcharr Is it meaningful for a type to be unstable if stable APIs can still produce a value of that type?

[clarfonthey]

@SimonSapin The only way to produce that type would be from <T as TryFrom<usize>>::Error, which can be replaced later as either TryFromIntError or ! without any breaking changes.

It is a sort of hack, but it would allow us to at least add TryFrom impls without choosing whether some of the errors are !.

[SimonSapin]

By produce a value of that type I meant something like T::try_from(x).unwrap_err() where x is a value such that the conversion fails.

[Kixunil]

To provide more context about my idea, I wrote a proof-of-concept.

[SimonSapin]

@Kixunil This would probably needs two type parameters to support conversions in both directions. But more importantly, TryFromIntError is public so traits in its where clauses need to be public as well. And we’re back to having a public (associated) type TryFromIntMayFail::MayFail which varies across platforms, same as with conditionally defining TryFrom<Error=!> directly. So this doesn’t solve the original problem.

[Kixunil]

@SimonSapin Good point. Hypothetically, if compatibility lint was easier to implement on methods, it could still be helpful. The associated type could be hidden by sealing the trait.

Anyway, I prefer compatibility-first approach.

Edit: I just realized, we don't need that trait at all. We could just conditionally compile never method.

[SimonSapin]

@Kixunil I only vaguely guess what you mean by sealing the trait, but I believe this also doesn’t solve the problem.

[Kixunil]

@SimonSapin similar to what byteorder::Byteorder did, but the other way around - not defining the type on the public trait but on private trait.

[snim2]

@SimonSapin Hi Simon, thanks for pushing this forward. Unfortunately I'm really blocked on this issue (in particular it's the TryFrom<_> for usize: u32, u64, u128 conversion I need, but I guess it doesn't make a huge difference). I wonder whether it would make sense to feature-gate the impls that were removed, until there's a stable solution to this issue that the Rust team are happy with?

[SimonSapin]

Unfortunately, #[unstable] attributes on impl {} items does not actually work. If the trait and type are both stable, the impl will be usable. (Though we’ve reverted the stabilization of the trait, but that’ll hopefully change back again soon.)

[snim2]

Ok, many thanks. I just noticed that you have a revert commit for this, is that likely to be merged into master?

[SimonSapin]

This would be my preferred outcome. I’m gonna talk to people this week about what’s going on with the portability lint and what to do process-wise.

[snim2]

Many thanks.

[SimonSapin]

:bell: :bell: Proposed resolution:

PR https://github.com/rust-lang/rust/pull/51564 restores TryFrom impls that are infallible on some platforms only as always fallible, on all platforms, at the type system level (with Error=TryFromIntError).