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 num_as_ne_bytes feature #76976

Closed
1 of 3 tasks
hch12907 opened this issue Sep 20, 2020 · 17 comments · Fixed by #85679
Closed
1 of 3 tasks

Tracking Issue for num_as_ne_bytes feature #76976

hch12907 opened this issue Sep 20, 2020 · 17 comments · Fixed by #85679
Labels
C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC Libs-Tracked Libs issues that are tracked on the team's project board. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@hch12907
Copy link
Contributor

hch12907 commented Sep 20, 2020

The feature gate for the issue is #![feature(num_as_ne_bytes)].

pub mod core {
    pub mod f64 {
        impl f64 {
            pub fn as_ne_bytes(&self) -> &[u8; 8] {}
        }
    }

    pub mod f32 {
        impl f32 {
            pub fn as_ne_bytes(&self) -> &[u8; 4] {}
        }
    }
}

About tracking issues

Tracking issues are used to record the overall progress of implementation.
They are also used as hubs connecting to other relevant issues, e.g., bugs or open design questions.
A tracking issue is however not meant for large scale discussion, questions, or bug reports about a feature.
Instead, open a dedicated issue for the specific matter and add the relevant feature gate label.

Steps

@hch12907 hch12907 added the C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC label Sep 20, 2020
@SimonSapin
Copy link
Contributor

Ad discussed starting in #64464 (comment) I’m not convinced this feature carries its weight to be included in the standard library, given that to_ne_bytes also exists.

@SimonSapin SimonSapin added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Oct 11, 2020
@KodrAus KodrAus added the Libs-Tracked Libs issues that are tracked on the team's project board. label Nov 6, 2020
@mbartlett21
Copy link
Contributor

as_ne_bytes_mut may be more useful.

@SimonSapin
Copy link
Contributor

How so? Trying to go beyond "I want to borrow because I want to borrow"

@shepmaster
Copy link
Member

In a similar vein, I was curious if the opposite existed, converting from &(mut) [u8; 4] to &(mut) u32:

let mut a = [1, 2, 3, 4];
let v: &mut u32 = u32::from_ne_bytes_mut(&mut a);
// Then potentially used with
let x = AtomicU32::from_mut(v);

I do not have a specific usecase in mind. My thoughts were likewise around avoiding the copy (small as it may be).

@m-ou-se
Copy link
Member

m-ou-se commented Feb 2, 2021

@shepmaster Something kind of like that exists: [u8]::align_to::<u32>

It returns (&[u8], &[u32], &[u8]) because the byte slice might not be aligned properly for an u32, so it returns the unaligned parts separately.

It's also unsafe because it can transmute to any type. A safe version for integers could be possible, but that won't solve the alignment problem.

@shepmaster
Copy link
Member

A good point! The original case had a repr(align), which I neglected to show here. You get that "for free" if you store a u32 and have something like the suggested as_ne_bytes_mut.

@hch12907
Copy link
Contributor Author

hch12907 commented May 8, 2021

Sorry for replying really late here (I accidentally unsubscribed from the issue.) but how should I proceed from here?

  • Remove the function from std as per Simon's suggestion
  • Add another function as_ne_bytes_mut

Though, the AtomicU* one does sound interesting to me although it is unrelated my original use case.

@sffc
Copy link

sffc commented May 11, 2021

This feature would be useful to make this be a safe conversion. Currently this requires unsafe code. We use this type of abstraction in TinyStr; it would also be handy in some serialization-type libraries.

@sffc
Copy link

sffc commented May 11, 2021

One other suggestion. It would be useful if this method returned a type like Aligned<[u8; 4], 4> (pseudo-code for a newtype that guarantees that the byte array is 4-byte aligned). This would allow for safe two-way &u32 <-> &Aligned<[u8; 4], 4> conversion.

In other words, returning &[u8; 4] is nice in that it remembers the length, but it throws away the alignment. It would be nice if we could also retain the alignment information.

@SimonSapin
Copy link
Contributor

to make this be a safe conversion

Which "this" are you referring to? Why does to_ne_bytes not work for you?

@sffc
Copy link

sffc commented May 12, 2021

to make this be a safe conversion

Which "this" are you referring to? Why does to_ne_bytes not work for you?

I meant that as_ne_bytes() allows the conversion from &u32 to &[u8] to be safe. Right now it requires unsafe.

@sffc
Copy link

sffc commented May 12, 2021

Also: in conjunction with #74985 (comment), this feature would also enable safe conversion from &[u32] to &[u8].

@hch12907
Copy link
Contributor Author

Regarding Aligned<[u8; 4], 4>: I believe that would be out of scope for this feature?

At least, I am not aware of any machinery in std that allows one to say "the returned value is N-byte aligned", so doing that would require a new feature that mimics the aligned crate in std.

@scottmcm
Copy link
Member

I think I'm with #76976 (comment) on this one. Given that this can't (portably) have .as_le_bytes() and .as_be_bytes(), I'm not sure it's worth having as a distinct thing, assuming that the Safe Transmute project lands -- as one of its top two use cases is offering &[u8] native-endian views into everything where it's safe to do so, including f32 and u64 and such.

@BurntSushi
Copy link
Member

@scottmcm Agreed. I think it would be great to wait and see what the Safe Transmute project gives us here.

@workingjubilee
Copy link
Member

workingjubilee commented May 24, 2021

The use case is valid but a more general mechanism is direly needed here (something like this came up in Portable SIMD discussions).

@hch12907
Copy link
Contributor Author

Opened #85679 which removes this feature.

@bors bors closed this as completed in f3b10dd May 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC Libs-Tracked Libs issues that are tracked on the team's project board. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants