rust-lang / libs-team

The home of the library team
Apache License 2.0
110 stars 18 forks source link

Add `array::repeat` for making an array from a non-`Copy` non-`const` value #310

Closed scottmcm closed 1 month ago

scottmcm commented 7 months ago

Proposal

Problem statement

For a type that's copy, [x; N] works great for getting an array. For a constructor that's const, [const { Foo::new() }; N] works great (or hopefully will soon) for getting an array.

However, if you have a general String value that you want to repeat, there's no great way right now.

We should add one.

Motivating examples or use cases

Like how vec![x; N] supports non-Copy values, we should have a way to do that for normal arrays too.

See zulip thread https://rust-lang.zulipchat.com/#narrow/stream/122651-general/topic/syntactical.20parallelism.20for.20multiple.20clones/near/403589163

As an interesting bonus, this would also let array::repeat(x) work inferring the length in many cases, which could actually be a nice thing to use even with Copy types, until [x; _] (or however we spell that) ends up happening.

Solution sketch

This is trivial to implement in core with the existing internal methods:

// in core::array

pub fn repeat<T: Clone, const N: usize>(x: T) -> [T; N] {
    from_trusted_iterator(iter::repeat_n(x, N))
}

Alternatives

The simplest option is array::from_fn(|_| x.clone()), but that's sub-optimal in that it never re-uses the original value, likely losing capacity -- since the whole reason to not be using [x; N] in the first place is that the type isn't Copy, and thus plausibly has a non-trivial Drop.

Fixing that takes something like this (from Kevin Reid):

fn dup<T: Clone, const N: usize>(v: T) -> [T; N] {
    let mut buf = Some(v);
    core::array::from_fn(|i| if i == N - 1 { buf.take() } else { buf.clone() }.unwrap())
}

But that's far more complicated, and doesn't necessarily optimize well either. It could be done with unsafe, but once that's happening it's probably good to have it in core again to encapsulate the unsafe in a known-safe interface.

What happens now?

This issue contains an API change proposal (or ACP) and is part of the libs-api team feature lifecycle. Once this issue is filed, the libs-api team will review open proposals as capability becomes available. Current response times do not have a clear estimate, but may be up to several months.

Possible responses

The libs team may respond in various different ways. First, the team will consider the problem (this doesn't require any concrete solution or alternatives to have been proposed):

Second, if there's a concrete solution:

scottmcm commented 7 months ago

As I was typing this out, @the8472 pointed out that some types -- notably Arc -- could do something faster here if they know it's happening.

So another alternative here might be something like

trait Clone {
    // ... existing methods ...
    fn multi_clone<const N: usize>(&self) -> [Self; N] where Self: Sized { array::from_fn(|_| self.clone()) }
    fn multi_clone_consuming<const N: usize>(self) -> [Self; N] where Self: Sized { array::from_trusted_iterator(iter::repeat_n(self, N)) }
}

Which could be overridden for the few types that care.

ChayimFriedman2 commented 7 months ago

Perhaps instead of adding more and more iterator-like function to arrays, we should just have a collect_array() method on iterators?

scottmcm commented 7 months ago

Something very much like that exists in nightly already, @ChayimFriedman2 -- https://doc.rust-lang.org/nightly/std/iter/trait.Iterator.html#method.next_chunk

The problem is that even if that's stable, it's fallible because there's no type-based minimum length on iterators -- and there can't be because next exists -- other then "it's infinite". And while iter::repeat could be InfiniteIterator, repeat(x).next_chunk_infallible() doesn't give the "keep the capacity from x" guarantee.

So even if I could write iter::repeat_n(x, N).next_chunk::<_, N>().unwrap(), because everything in there was already stable, I'd still want array::repeat(x) because it's convenient.

Amanieu commented 7 months ago

It seems like there are 2 separate motivations here, which each have different optimal solutions.

If the primary concern is the syntax, then array::repeat is a good solution: it's simple an easy to use.

If the primary concern is performance due to multiple refcount increments, then a more general solution would be a way to increment an Arc multiple times at once and then return the new Arcs in an iterator:

impl Arc<T> {
    fn increment_multiple(self: Self, n: usize) -> impl Iterator<Item = Arc<T>>;
}
scottmcm commented 7 months ago

My primary intent here is that there's no safe-and-efficient way to do [x; N] for non-Copy things today. I think we should have an obvious way to point to.

If it does other things too, that's great, but that wasn't my goal with this ACP.

scottmcm commented 6 months ago

cc https://github.com/rust-lang/rust/issues/119530#issuecomment-1878583102, for another motivation example

scottmcm commented 4 months ago

I was reminded of this again today in Discord: https://discord.com/channels/273534239310479360/1120175689124036669/1219021342003691520

image

Being able to say array::repeat(foo) as the answer would be great there, and if that doesn't address the Arc cases that's fine.

the8472 commented 4 months ago

Hrm, that reminds me of the java feature where the method-reference syntax can create instance-bound ~closures.

I.e. one would just write repeat(clonable) as from_fn(clonable::clone)

scottmcm commented 1 month ago

I responded to the above question (https://github.com/rust-lang/libs-team/issues/310#issuecomment-1863384069), so nominating to get eyes on it again @rustbot label +i-libs-api-nominated

Amanieu commented 1 month ago

We discussed this in the @rust-lang/libs-api meeting and are happy to accept it!