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

Tracking Issue for integer_sign_cast #125882

Open
1 of 3 tasks
Rua opened this issue Jun 2, 2024 · 10 comments
Open
1 of 3 tasks

Tracking Issue for integer_sign_cast #125882

Rua opened this issue Jun 2, 2024 · 10 comments
Labels
C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@Rua
Copy link
Contributor

Rua commented Jun 2, 2024

Feature gate: #![feature(integer_sign_cast)]

This is a tracking issue for explicit signedness casting methods for integer primitive types. Libs discussion: rust-lang/libs-team#359.

Public API

(for N in [8, 16, 32, 64, 128, size]):

impl uN {
    pub const fn cast_signed(self) -> iN {}
}

impl iN {
    pub const fn cast_unsigned(self) -> uN {}
}

Steps / History

Unresolved Questions

Mostly bikeshedding regarding naming, as mentioned in the Libs discussion. The current proposal follows the naming of cast_const and cast_mut for pointers.

Alternatively, these could be implemented as from_bits and to_bits methods for the signed types.

Footnotes

  1. https://std-dev-guide.rust-lang.org/feature-lifecycle/stabilization.html

@Rua Rua added C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Jun 2, 2024
workingjubilee added a commit to workingjubilee/rustc that referenced this issue Jun 2, 2024
…lacrum

Implement feature `integer_sign_cast`

Tracking issue: rust-lang#125882

Since this is my first time making a library addition I wasn't sure where to place the new code relative to existing code. I decided to place it near the top where there are already some other basic bitwise manipulation functions. If there is an official guideline for the ordering of functions, please let me know.
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Jun 3, 2024
Rollup merge of rust-lang#125884 - Rua:integer_sign_cast, r=Mark-Simulacrum

Implement feature `integer_sign_cast`

Tracking issue: rust-lang#125882

Since this is my first time making a library addition I wasn't sure where to place the new code relative to existing code. I decided to place it near the top where there are already some other basic bitwise manipulation functions. If there is an official guideline for the ordering of functions, please let me know.
@tbu-
Copy link
Contributor

tbu- commented Jun 3, 2024

Mostly bikeshedding regarding naming, as mentioned in the Libs discussion. The current proposal follows the naming of cast_const and cast_mut for pointers.

I think that cast_signed and cast_unsigned do not communicate intent on what to do on overflows. This is unlike cast_const and cast_mut where no such overflow can occur.

I think something like reinterpret_signed or the proposed from_bits/to_bits woul be more appropriate.

@electroCutie
Copy link

do not communicate intent on what to do on overflows

There wouldn't be any overflows, as I understand the term. The number of bits stays the same, which is expressly part of the design of the function.

@tbu-
Copy link
Contributor

tbu- commented Jun 4, 2024

It does not communicate what happens to values of unsigned integers that do not fit into the target integer values.

We're reinterpreting the bits as an unsigned integer, but cast does not say that. E.g. casting from integer to float does not reinterpret the bits, casting from i8 to i32 does not solely reinterpret the bits either, it sign-extends them. So "cast" doesn't tell me what happens.

@electroCutie
Copy link

electroCutie commented Jun 4, 2024

@tbu- ah now I see your meaning

Your suggestion of reinterpret_signed seems like a good way to phrase it

@Rua
Copy link
Contributor Author

Rua commented Jun 4, 2024

What about transmute_signed? That seems closer to Rust's typical jargon.

@aapoalas
Copy link

aapoalas commented Jun 8, 2024

IMO: A consistent API is most important, so from_bits / to_bits should be chosen.

@Rua
Copy link
Contributor Author

Rua commented Sep 3, 2024

After thinking about this for a while, and looking at how I'm actually using this function, I think I agree with the suggestion to use from_bits and to_bits instead. Do others agree? If so, can I just make a PR to change the name?

@jhpratt
Copy link
Member

jhpratt commented Sep 5, 2024

I personally find from_bits/to_bits to be too similar to the {from|to}_{n|b|l}e_bytes methods. However from_bits and to_bits` are present and have been stable for years, with the behavior equivalent to that of a transmute (i.e. the same as this proposal).

So as much as I don't care for the naming, it would be consistent with that. With that said, casting a sign and reinterpreting the type entirely feel like two different things that don't necessarily have precedential impact. An integer and a float share effectively zero in common when using from_bits/to_bits, while integers will share a value for the overlapping range.

@aapoalas
Copy link

aapoalas commented Sep 6, 2024

After thinking about this for a while, and looking at how I'm actually using this function, I think I agree with the suggestion to use from_bits and to_bits instead. Do others agree? If so, can I just make a PR to change the name?

Do you happen to have any examples of this usage in public repositories?

@jhpratt
Copy link
Member

jhpratt commented Sep 6, 2024

@aapoalas While not using this feature directly, the time-rs/time repository uses an API from num-conv that is identical to the current proposal.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

5 participants