rust-lang / libs-team

The home of the library team
Apache License 2.0
110 stars 18 forks source link

Explicit signedness casts for integers #359

Closed Rua closed 3 months ago

Rua commented 3 months ago

Proposal

Problem statement

Casting using the as operator has some issues, in that it does not necessarily reflect the intent of the cast. Was the cast meant to be lossy or not? Was it meant to preserve the size? Constness? Signedness?

Some of the motivations for pointer constness casts raised at https://internals.rust-lang.org/t/casting-constness-can-be-risky-heres-a-simple-fix/15933 would apply here too. When refactoring, the sizes of types may change, and silently introduce bugs due to casts not being updated.

Clippy has the cast_possible_wrap and cast_sign_loss lints, which tag sign-changing casts, but since there is no alternative to using as, it cannot propose a fix.

Motivating examples or use cases

Rust has been slowly introducing alternatives to direct casts, to more clearly signal intent, catch errors, and allow casting only one aspect of a type instead of everything in one go. From is used when the cast is lossless, TryFrom for checked casts, ptr::cast_const and ptr::cast_mut change the mutability and nothing else, ptr::cast changes pointed type and nothing else.

Solution sketch

Following the example of ptr::cast_const and ptr::cast_mut, I propose adding two new methods to integer types, for all X including size:

These do the same as a regular as cast, but crucially they only cast to another integer of the same size and opposite signedness. They don't ever change the size, so i32u64 and u64i32 are not implemented.

Alternatives

Other names for the two proposed methods would be possible, such as:

It would also be possible to implement this as a trait, like the explicit_cast crate does, but since it's a closed class of types, that seems overkill. There have also been proposals to include this under a more general "wrapping cast" or "lossy cast" function/trait of some sort, but I feel that a i32u32 cast isn't in the same class of casts as i32 -> i16, since the former is fully reversible and the latter is actually lossy.

Links and related work

aapoalas commented 3 months ago

Bikeshed: These should definitely follow fX::from_bits as they're the exact same bitwise "conversion"/reinterpretation operation on just a different numeric type.

Rua commented 3 months ago

Bikeshed: These should definitely follow fX::from_bits as they're the exact same bitwise "conversion"/reinterpretation operation on just a different numeric type.

The downside is that it makes method chaining a bit uglier with from_bits than it would be with cast_signed.

BurntSushi commented 3 months ago

What is the intended behavior of (-1i32).cast_unsigned()?

GrigorenkoPV commented 3 months ago

What is the intended behavior of (-1i32).cast_unsigned()?

u32::MAX I suppose. From what I can tell, this is meant to be a bitwise-cast, and Rust guarantees two's complement.

Rua commented 3 months ago

What is the intended behavior of (-1i32).cast_unsigned()?

u32::MAX I suppose. From what I can tell, this is meant to be a bitwise-cast, and Rust guarantees two's complement.

Yes. It's exactly the same operation as -1i32 as u32, but made more explicit.

aapoalas commented 3 months ago

Re: bikeshedding on from_bits

It'd be bad if the answer to the question of "how do I reinterpret 64 bits as a numeric type X?" depended on X. It has a good chance of spawning a mnemonic: "from for floats, reInt for ints" or something similar.

Amanieu commented 3 months ago

We discussed this in the libs-api meeting today. We're happy to add these methods.

Feel free to open a tracking issue and open a PR to rust-lang/rust to add it as an unstable feature.

jhpratt commented 3 months ago

Trait methods take precedence over inherent methods nowadays, correct? Trying to determine if there is any future breakage for num-conv, which has an identical API using extension traits. To be clear, I have no issue whatsoever with breakage and would love to see the API.

cuviper commented 3 months ago

Trait methods take precedence over inherent methods nowadays, correct?

Only while the inherent method is unstable, otherwise it takes precedence.

jhpratt commented 3 months ago

Good enough for num-conv, given that the signature and behavior are identical.

ChayimFriedman2 commented 2 months ago

I'm worried about the fact that the names cast_(un)signed() doesn't express the fact that they don't check for overflow. This makes them almost as bad as as IMHO. Maybe cast_(un)signed_wrapping()?

GrigorenkoPV commented 2 months ago

This makes them almost as bad as as IMHO.

Not really. The main problem with as (at least as I see it) is that it does too many things. References and pointers aside, just with numbers it can do

Rua commented 1 month ago

We discussed this in the libs-api meeting today. We're happy to add these methods.

Feel free to open a tracking issue and open a PR to rust-lang/rust to add it as an unstable feature.

Does this need an RFC first, or can I just link to this issue?

jhpratt commented 1 month ago

Linking to this issue is sufficient.