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: Add try_from_slice constructor to core::array. #496

Closed
bjoernager opened this issue Nov 25, 2024 · 11 comments
Closed

ACP: Add try_from_slice constructor to core::array. #496

bjoernager opened this issue Nov 25, 2024 · 11 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

@bjoernager
Copy link

bjoernager commented Nov 25, 2024

Proposal

Problem statement

In Rust, you can convert a slice &[T] to an array [T; N] using <[T; N] as TryFrom<&[T]>>::try_from. The main issue with this is that it depends on a trait function, and trait functions are not allowed in constant expressions (e.g. const fn).

I propose adding a try_from_slice function to the core::array module for this scenario. The module in question already defines other constructors such as from_fn and from_ref.

Solution sketch

The following function should be added to the standard library:

// core::array

pub const fn try_from_slice<T, const N: usize>(slice: &[T]) -> Result<[T; N], TryFromSliceError>
where
    T: Copy;

Alternatives

The main alternative to adding this feature (with regard to const) would be to allow trait functions in constant expressions. This is already covered by const_trait_impl.

I still do believe adding this function can improve clarity in some code. Other areas of the standard library already define similar patterns, e.g. String implements Into<Box<str>> whilst also defining the into_boxed_str destructor.

Links and related work

Initial, unstable implementation: #133439

@bjoernager bjoernager added api-change-proposal A proposal to add or alter unstable APIs in the standard libraries T-libs-api labels Nov 25, 2024
@kennytm
Copy link
Member

kennytm commented Nov 25, 2024

If we are adding a dedicated conversion function I think flipping the order turning it into an inherent method of the slice is more convenient to the users

impl<T> [T] {
    pub const fn as_array_exact<const N: usize>(&self) -> Result<&[T; N], TryFromSliceError>;
}

I'm not sure why the ACP proposed &[T] -> [T; N] thus requiring T: Copy. Dereferencing a &[T; N] is allowed in const context.

@bjoernager
Copy link
Author

I see your point, although I feel it would be confusing to have the error type named TryFromSliceError when returned from a function with a slightly different intention and scope. Defining a new type AsArrayExactError would be clearer but also result in mostly duplicate code.

As for your second concern, I'm not quite sure I understand what the problem is. The semantics proposed in this ACP are basically the same as the preexisting; the logic has just been moved from TryFrom to a global function, using the same infrastructure. This approach is minimally different from status quo.

@programmerjake
Copy link
Member

programmerjake commented Nov 25, 2024

impl<T> [T] {
    pub const fn as_array_exact<const N: usize>(&self) -> Result<&[T; N], TryFromSliceError>;
}

I think as_array is clear enough, _exact isn't needed.
also, const fn supports &mut now, so there should also be an as_array_mut function.

@jdahlstrom
Copy link

jdahlstrom commented Nov 25, 2024

For what it's worth, in the meantime this works in a const context, though only on nightly for now:

if let ([a], []) = s.as_chunks() {
    Some(a)
} else {
    None
}

@scottmcm
Copy link
Member

scottmcm commented Nov 25, 2024

impl<T> [T] {
    pub const fn as_array<const N: usize>(&self) -> Option<&[T; N]>;
    pub const fn as_array_mut<const N: usize>(&mut self) -> Option<&mut [T; N]>;
}

seems pretty reasonable to me.

Would be convenient to have it in const, and the TryFrom impl can use it with a .ok_or(TryFromSliceError).

(I don't think it needs to return a Result, because the error can't say anything meaningful -- just like how NonZero::new returns an Option, rather than having a IsNotZeroError.)

@bjoernager
Copy link
Author

bjoernager commented Nov 25, 2024

I'm personally content with that solution.

@kennytm
Copy link
Member

kennytm commented Nov 26, 2024

As for your second concern, I'm not quite sure I understand what the problem is. The semantics proposed in this ACP are basically the same as the preexisting;

There are two pre-existing TryFrom impls between slice and array (up to mutability):

  1. TryFrom<&[T]> for &[T; N]
  2. TryFrom<&[T]> for [T; N] where T: Copy

@bjoernager
Copy link
Author

In any case, would this ACP have to be changed to instead propose the as_array and as_array_mut conversion methods, or would it have to be replaced entirely with a new ACP? (Assuming the general consensus wants the two methods).

@Amanieu
Copy link
Member

Amanieu commented Nov 26, 2024

We discussed this in the libs-api meeting and we are in favor of adding methods on slices, but also on raw pointers to slices and Box/Rc/Arc of slices:

impl<T> [T] {
    pub const fn as_array<const N: usize>(&self) -> Option<&[T; N]>;
    pub const fn as_array_mut<const N: usize>(&mut self) -> Option<&mut [T; N]>;
}
impl<T> *const [T] {
    pub const fn as_array<const N: usize>(self) -> Option<*const [T; N]>;
}
impl<T> *mut [T] {
    pub const fn as_array_mut<const N: usize>(self) -> Option<*mut [T; N]>;
}
impl<T> Box<[T]> {
    pub const fn into_array<const N: usize>(self) -> Option<Box<[T; N]>>;
}
impl<T> Rc<[T]> {
    pub const fn into_array<const N: usize>(self) -> Option<Rc<[T; N]>>;
}
impl<T> Arc<[T]> {
    pub const fn into_array<const N: usize>(self) -> Option<Arc<[T; N]>>;
}

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

@Amanieu Amanieu added the ACP-accepted API Change Proposal is accepted (seconded with no objections) label Nov 26, 2024
@Amanieu Amanieu closed this as completed Nov 26, 2024
@RustyYato
Copy link

For the owning versions, this should return a result so that the pointer can be reused on failure.

I.e.
Result<Box<[_; _]>, Box<[_]> instead of Option

@bjoernager
Copy link
Author

Is as_array_mut the desired name instead of as_mut_array (as with as_mut_ptr, as_mut_slice, etc.)?

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

7 participants