Open c410-f3r opened 2 years ago
I am not certain the currently implemented signature of try_from_fn
is the right one. The most similar thing to this currently in the standard library is https://doc.rust-lang.org/std/iter/trait.Iterator.html#method.try_for_each, where the return type is a generic R
based on a Try
trait impl, instead of hardcoded to Result
. Can try_from_fn
be done the same way?
@dtolnay I've submitted a PR to update array::try_from_fn
to be generic over Try
types: #91286
(As well as other related methods like Iterator::try_find
#63178 that hit similar questions.)
My 2¢ on the usize
parameter question:
array::from_fn(|_| value.clone())
.ι
being from_fn(identity)
is so much tidier than { let mut i = 0; from_fn(|| { let temp = i; i += 1; temp }) }
or whatever.iter::from_fn
, a usize
fundamentally can't overflow in array::from_fn
. And iter::from_fn
's closure must return an Option
, so its signature will never match array::from_fn
. (Though I guess array::try_from_fn
can take -> Option
closures, but with different semantics.)core
. It could be done with Default + Step
, but that's arguably a misuse of Default
, and it also allows things like array::from_fn::<i8, 200>
which feels wrong. I don't see core
importing num_traits::Unsigned
any time soon.array::from_fn(|_: usize| value.clone())
, for example. It could potentially allow from_fn(|()| value.clone())
, but that's non-obvious and makes the trait question even harder. (()..
isn't an iterator, and I hope that stays the case.)enumerate
is precedent for "sometimes we only offer the usize
version", so it's not unusual to only support usize
here.TryInto
in the prelude it's not that bad to get another type, and the range is obvious to LLVM in the monomorphization, so the checks easily optimize away.collect_into_array
anyway to know which things to drop. Not to mention that it has to write into the array, where if that's done with addressing modes like ptr [edx + 4*eax]
then the counter's already there too. (And things'll probably get inlined enough anyway that it'd be easy to remove if unused. The only way it wouldn't would be if the closure if complex, but then the cost of 1 LEA
is unnoticeable.)So personally I think array::from_fn
is right how it is, for all the reasons it ended up this way in the initial PR. And I'd like to see it move forward for stabilization. That's not up to me, though.
I don't think array::try_from_fn
is ready yet, though, so I don't know how procedurally libs-api would like to go, here. Usually it FCPs in the tracking issue, but there can only be one FCP per issue/PR, so maybe for a partial one it'd need a separate issue or a stabilization PR in which to make the decision?
I am not certain the currently implemented signature of try_from_fn is the right one. The most similar thing to this currently in the standard library is https://doc.rust-lang.org/std/iter/trait.Iterator.html#method.try_for_each [...]
Seconding this. I'm trying to collect an iterator into an array, for which I'd like to write:
fn parse() -> Option<()> {
let arr = array::try_from_fn(|_| self.iter.next())?;
// ...
}
But because try_from_fn
is hard-coded to return a Result
, this does not work. Having an R: Try
bound would solve this issue.
@yoshuawuyts I think my PR handles that scenario: https://github.com/rust-lang/rust/pull/91286/files#diff-534d866e6adad8a1f7577c2f1b3eed0b05d5f001c0d5032ac226393736940381R78-R82
I hate to re-bikeshed, but I stumbled on a possibility which I didn't see mentioned in #75644
I'm making a PR for array::repeat
, to handle simple things like @leonardo-m's repeat(vec![])
. But in doing that I noticed that there's not only iter::repeat
-- which I was copying for array::repeat
-- but there's also iter::repeat_with
.
So I went to make array::repeat_with
, but of course it's essentially identical to this tracking issue's array::from_fn
. It's a much better parallel than iter::from_fn
, as iter::repeat_with
returns an "infinite" iterator and thus the closure is just -> Item
(not from_fn
's -> Option<Item>
), like how the closure for creating an array is -> Element
.
So a potential new coat of paint for the constructors could be
array::repeat
for creating by cloning, like repeat(vec![1])
.array::repeat_with
for creating by calling a closure, like repeat_with(|| Vec::with_capacity(10))
.array::try_repeat_with
for creating by a fallible closure.I don't know if this makes me feel differently about the usize
argument. Allowing repeat_with(VecDeque::new)
does feel somewhat more expected to me than from_fn(VecDeque::new)
, but I also still agree with the arguments in my previous comment. Of course, having both is also a possibility -- just like there's both iter::from_fn
and iter::repeat_with
, despite them being rather similar. Perhaps for nightly it would be reasonable to have both, with notes on their tracking issues to think about the other one if considering for stabilization?
The two most important things about these functions are (1) Provide an unified way for creating custom arrays (it is common enough for std
) and (2) Take off the user burden of having to deal with unsafe
.
That said, I will personally support any proposal that will move any function or method towards stabilization as soon as possible. So if desired feel free to create repeat
-like variants, maintain both versions or even close this issue.
Although some recent development is happening in this field, sometimes I think that custom array constructors will take, if ever, as long as try_reserve
or more.
Since I've seen this, I've written [(); N].map(|_| ...)
dozens of times, some required the int and some didn't.
So I'd love to see this stabilized :)
Now that https://github.com/rust-lang/rust/pull/94119 has been merged to stabilize this, can we close this issue?
Now that #94119 has been merged to stabilize this, can we close this issue?
Only from_fn
is stabilized, try_from_fn
is still unstable. The latter is blocked on #84277.
Please, also add this functions:
fn concat_arrs<T, const N: usize, const M: usize>(a: &mut ([T; N], [T; M])) -> &mut [T; N + M]; // Without coping, same for 3-tuples, 4-tuples, etc
fn concat_arr_of_arrs<T, const N: usize, const M: usize>(a: &mut [[T; N]; M]) -> &mut [T; N * M];
Please, also add this functions:
fn concat_arrs<T, const N: usize, const M: usize>(a: &mut ([T; N], [T; M])) -> &mut [T; N + M]; // Without coping, same for 3-tuples, 4-tuples, etc fn concat_arr_of_arrs<T, const N: usize, const M: usize>(a: &mut [[T; N]; M]) -> &mut [T; N * M];
Can't you just use std::mem::transmute
for this? If you can do something without copying - it should be easy to just transmute it.
IMHO problem with from_fn
is that it is much harder to replicate that functionality and also it is a very widely used pattern.
I was wondering if these functions could be const in the future when the pending issues with const fn are dealt with.
These functions actually seem pretty useful for lookup tables.
The new signature would probably be something like:
const fn from_fn<F, T, const N: usize>(cb: F) -> [T; N]
where
F: ~const + FnMut(usize) -> T;
Filed https://github.com/rust-lang/rust/issues/89379 to try to get some extra traction on making the already-stabilized std::array::from_fn
itself const
.
As @scottmcm said, it is necessary to have stable constant closures in order to "constify" array::from_fn
in stable toolchains, which is probably going to happen in the next few years or so. See https://github.com/rust-lang/rust/issues/106003.
I think it is already possible to at least "constify" the hidden underlying machinery but I am not totally sure.
Is there any reason this isn't stable yet?
Hello,
let mut parts = "Mary had a small lamp".split_ascii_whitespace();
let parts: [&str; 5] = from_fn(|_i| parts.next().unwrap());
// produces: ["Mary", "had", "a", "small", "lamp"]
This works, but the documentation doesn't state that the callback parameter will grow upwards, so I'm not confident to use this code.
One day this could end up producing ["lamp", "small", "a", "had", "Mary"]
, if the implementation decided to "push" values from right to left, with a cb param going from 4 to zero; nothing says otherwise in the doc.
Is it possible that one day, we need the callback parameter to go in decreasing order? If not, I think it would be nice to add a word about this in the documentation.
Are there any open questions about this? If not, could someone start an FCP?
try_trait_v2
needs to be stabilized first and that probably won't happen in the near future. I am personally betting in 4 years or more (if ever).
Why does this depend on try trait necessarily? try_for_each
is already in the standard library.
Why does this depend on try trait necessarily?
Well, the original function returned Result
but you can see more context about the use of Try
in https://github.com/rust-lang/rust/pull/90505, https://github.com/rust-lang/rust/pull/91286 and
https://github.com/rust-lang/rust/pull/94119#issuecomment-1090685583.
try_for_each is already in the standard library.
EDIT: Oh I think it is because of concerns around the Residual
associated type and try_for_each
doesn't use that.
@c410-f3r is right -- the distinction is the use of the Residual
mechanism, which has no stable use in the library yet
See https://github.com/rust-lang/rust/pull/94119#discussion_r815334585 and https://github.com/rust-lang/rust/pull/94119#issuecomment-1090685583
Same questions as, for example, Iterator::try_find
(https://github.com/rust-lang/rust/issues/63178).
Provides the common use-case of creating custom fallible arrays in a reliable and unified way. Particularly useful for elements that don't implement Copy, Clone or Default.
Public API
Steps / History
Unresolved Questions
Should the callback return indices (https://github.com/rust-lang/rust/pull/75644#discussion_r689066495) ?from_fn
andtry_from_fn
are very similar torepeat_with
andtry_repeat_with
(#91613), just that they take unary instead of nullary functions. Anyone considering stabilizing these should think about those, and vice versa.