Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add uN::to_signed and iN::to_unsigned #183

Closed
Shadlock0133 opened this issue Feb 24, 2023 · 4 comments
Closed

Add uN::to_signed and iN::to_unsigned #183

Shadlock0133 opened this issue Feb 24, 2023 · 4 comments
Labels
api-change-proposal A proposal to add or alter unstable APIs in the standard libraries T-libs-api

Comments

@Shadlock0133
Copy link

Proposal

Problem statement

Currently, users wanting to change "signed-ness" of an integer while preserving it's bit representation (u8 to i8, or vice versa) would use as conversion. This has problems, as as doesn't restrict its input type, making it possible to introduce bugs during refactors or through code duplication.

Motivation, use-cases

For example, user might have started writing code using u8 type, and needs to use i8 (with wrapping behaviour) in a specific place:

fn foo(x: u8) {
    ...
    bar(x as i8)
    // or
    bar(x as _)
}
// potentially from another crate or from extern block
fn bar(x: i8) { ... }

Later, user decides to change the type to e.g. u16, so foo becomes fn foo(x: u16), but now the code will truncate the value before passing it to bar, potentially introducing subtle bugs. This could be dangerous for low-level and/or unsafe code, where this kind of conversions are not uncommon.

If user used bar(x.to_signed()), they would get an type error instead, making them aware of situation, and forcing them to resolve it in some way.

The proposed solution is also inspired by pointers methods like cast, cast_const, cast_mut, which have similar goals of making it safer to perform conversion without using as.

Solution sketches

My solution is to introduce set of methods on primitive integer type, to_signed and to_unsigned:

impl u8 {
    /// Converts to i8, wrapping if necessary.
    ///
    /// This is equivalent to `as i8`, but is more specific to enhance readability.
    ///
    /// # Example
    /// ```
    /// assert_eq!(255u8.to_signed(), -1i8);
    /// ```
    fn to_signed(self) -> i8 { self as i8 }
}
impl i8 {
    /// Converts to u8, wrapping if necessary.
    ///
    /// This is equivalent to `as u8`, but is more specific to enhance readability.
    ///
    /// # Example
    /// ```
    /// assert_eq!((-1i8).to_unsigned(), 255u8);
    /// ```
    fn to_unsigned(self) -> u8 { self as u8 }
}
// etc.

Other solution could be to use an "inverse" of this, from_signed/from_unsigned, e.g.:

impl u8 {
    fn from_signed(x: i8) -> Self { x as Self }
}

or to add both sets of methods: {to,from}_{signed,unsigned}, but the added value here seems negligible to author.

Links and related work

What happens now?

This issue is part of the libs-api team API change proposal process. Once this issue is filed the libs-api team will review open proposals in its weekly meeting. You should receive feedback within a week or two.

@Shadlock0133 Shadlock0133 added api-change-proposal A proposal to add or alter unstable APIs in the standard libraries T-libs-api labels Feb 24, 2023
@Noratrieb
Copy link
Member

This also has the benefit that methods bring with them of being able to be converted to a function pointer or being passed to generic functions like map without having to create a closure.
Additionally, it's also nicely chainable .

@scottmcm
Copy link
Member

scottmcm commented Feb 24, 2023

Makes sense to me, and I like it as a method on the source type so that .to_signed() does the right thing without me needing to get the type right -- so it'd be useful for that even if we got a num::WrappingFrom trait somewhere down the line, or safe-transmute that could do this too.

The one 🚲🏚 that comes to mind is whether it should have something in the name to acknowledge the "not exactly the same value"-ness, since to is generally more preserving. All of wrapping_to_signed, saturating_to_signed, and checked_to_signed seem entirely plausible here, but I feel like to_signed wouldn't want the usual "panic in debug, wrap in release" that that would imply from the normal pattern.

If it's described as "the same as an as cast", could it make sense to be as_unsigned() instead? I'm having trouble applying https://rust-lang.github.io/api-guidelines/naming.html#ad-hoc-conversions-follow-as_-to_-into_-conventions-c-conv here...

@pitaj
Copy link

pitaj commented Feb 24, 2023

I'm not so sure. to_bytes*

@scottmcm
Copy link
Member

I think these got approved as cast_(un)signed? #359 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-change-proposal A proposal to add or alter unstable APIs in the standard libraries T-libs-api
Projects
None yet
Development

No branches or pull requests

4 participants