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

ACP: Additional NonZero conversions #145

Closed
clarfonthey opened this issue Dec 7, 2022 · 3 comments
Closed

ACP: Additional NonZero conversions #145

clarfonthey opened this issue Dec 7, 2022 · 3 comments
Labels
ACP-accepted API Change Proposal is accepted (seconded with no objections) api-change-proposal A proposal to add or alter unstable APIs in the standard libraries T-libs-api

Comments

@clarfonthey
Copy link

clarfonthey commented Dec 7, 2022

Proposal

Problem statement

It was discussed in #135 that using NonZeroU8 instead of simply u8 would be more ergonomic for CStr, but right now the ergonomics of NonZero slices aren't great.

Motivation, use-cases

  • Conversions between CStr and slices could be kept safe with less checks if we leveraged [NonZeroU8] as a type.
  • (probably more, will add more later)

Solution sketches

I think that we definitely want the following impls, where X is some primitive integer type:

  • From<&NonZeroX> for &X
  • From<&[NonZeroX]> for &[X]
  • From<[NonZeroX; N]> for [X; N]
  • TryFrom<&X> for &NonZeroX
  • TryFrom<&[X]> for &[NonZeroX]
  • TryFrom<[X; N]> for [NonZeroX; N]

Note that mutable references are not supported explicitly since they could be unsound: converting from &mut NonZeroX to &mut X could allow writing a zero and later observing it as a NonZeroX. I don't believe that there's the same problem converting from &mut X to &mut NonZeroX (since the value could not be zero for the duration of the reborrow), but I'm not including it for now in case that's still unsound for some reason.

Depending on how this is implemented, we could add methods directly on NonZeroX like:

  • fn new_ref(val: &X) -> Option<&NonZeroX>
  • fn new_slice(val: &[X]) -> Option<&[NonZeroX]>
  • fn new_array<const N: usize>(val: [X; N]) -> Option<[NonZeroX; N]>

This would allow the addition of newer unsafe methods:

  • unsafe fn new_ref_unchecked(val: &X) -> &NonZeroX
  • unsafe fn new_slice_unchecked(val: &[X]) -> &[NonZeroX]
  • unsafe fn new_array_unchecked<const N: usize>(val: [X; N]) -> [NonZeroX; N]

Note that all of the unsafe methods would be equivalent to transmutes, and thus not gain any new functionality (whereas the safe methods would be impossible with safe code), although they would be more explicit than simple transmutes. I'm a fan of adding additional named methods over simple From and TryFrom impls, since they can make clearer code, but discussing this is what the ACP is for.

Links and related work

None known currently.

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.

@clarfonthey clarfonthey added api-change-proposal A proposal to add or alter unstable APIs in the standard libraries T-libs-api labels Dec 7, 2022
@scottmcm
Copy link
Member

scottmcm commented Dec 7, 2022

While I agree this is stuff safe-transmute will be able to do, I think it's reasonable to add Froms for this since it clarifies that it's normal From-like meaningful, not safe-but-maybe-weird transmute stuff like &[i32]&[f32].

@m-ou-se
Copy link
Member

m-ou-se commented Aug 16, 2023

I think this falls in the same category as things like &[u8]&[MaybeUninit<u8>], &[NonNull<T>]&[*const T], &u32&Wrapping<u32>, &String&Reverse<String>, and so on.

I don't know what the solution for those things is, but we keep running into these cases. (E.g. the ergonomics of [NonZeroU8] for CStr, [MaybeUninit<u8>] for Read, and so on.) They're all cases of repr(transparent) with some additional restriction/guarantee. Do we need a special trait or even special language support for those?

Ideally, we'd also support all the valid combinations, like &[Reverse<NonZeroU8>]&[MaybeUninit<u8>], and &[&[&NonZeroU8; 2]]&[&[&u8; 2]], and so on.

I agree with @scottmcm above: "safe transmute" might be able to handle all these things, but that is something significantly different. These are about changing the interface to the same data (Wrapping or Reverse) or dropping an unused guarantee (e.g. &NonZeroI32 to &i32), not about reinterpreting the bits as an entirely new value (e.g. &i32 to &f32 or &[u8; 4]).

@joshtriplett
Copy link
Member

joshtriplett commented Aug 6, 2024

We discussed the proposed From and TryFrom impls in today's @rust-lang/libs-api meeting, and approved them:

impl From<&NonZeroX> for &X
impl From<&[NonZeroX]> for &[X]
impl From<[NonZeroX; N]> for [X; N]
impl TryFrom<&X> for &NonZeroX
impl TryFrom<&[X]> for &[NonZeroX]
impl TryFrom<[X; N]> for [NonZeroX; N]

@joshtriplett joshtriplett added the ACP-accepted API Change Proposal is accepted (seconded with no objections) label Aug 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ACP-accepted API Change Proposal is accepted (seconded with no objections) 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