rust-lang / libs-team

The home of the library team
Apache License 2.0
127 stars 19 forks source link

Support `vec![const { expr }; N]` syntax for creating a `Vec` of non-`Clone` values #484

Open EFanZh opened 3 days ago

EFanZh commented 3 days ago

Proposal

Problem statement

For array types, I can use [const { expr }; N] syntax to create an array of expr, even if the type of expr does not implement Clone. But I can’t do the same thing with Vec: vec![const { expr }; N] currently does not compile.

Motivating examples or use cases

Imagining implementing a DFS algorithm with an array to save the traversal states, I can write something like:

enum State {
    NotVisited,
    Visiting,
    Visited,
}

let states = std::iter::repeat_with(|| State::NotVisited).take(n).collect::<Vec<_>>();

But it could be more simple if I could just write:

let states = vec![const { State::NotVisited }; n];

Deriving Clone might not always be possible because the value type could be from a third party crate.

Solution sketch

Reimplement vec! macro so it supports the const { ... } syntax above.

Alternatives

Links and related work

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:

dtolnay commented 3 days ago

Seems like a good idea to me. I expect the implementation you have in mind involves expanding to <[_]>::into_vec(Box::new([const { expr }; N])) in the const case. I think that's fine to do but it won't benefit from the good suggestion that array syntax provides where it tells you to wrap your expression in an inline const block. Compare:

error[E0277]: the trait bound `State: Copy` is not satisfied
 --> src/main.rs:9:14
  |
9 |     let _ = [State::NotVisited; N];
  |              ^^^^^^^^^^^^^^^^^ the trait `Copy` is not implemented for `State`
  |
  = note: the `Copy` trait is required because this value will be copied for each element of the array
help: consider annotating `State` with `#[derive(Copy)]`
  |
2 + #[derive(Copy)]
3 | enum State {
  |
help: create an inline `const` block
  |
9 |     let _ = [const { State::NotVisited }; N];
  |              ~~~~~~~~~~~~~~~~~~~~~~~~~~~

vs:

error[E0277]: the trait bound `State: Clone` is not satisfied
    --> src/main.rs:9:18
     |
9    |     let _ = vec![State::NotVisited; N];
     |             -----^^^^^^^^^^^^^^^^^----
     |             |    |
     |             |    the trait `Clone` is not implemented for `State`
     |             required by a bound introduced by this call
     |
note: required by a bound in `from_elem`
    --> $RUSTUP_HOME/toolchains/nightly-aarch64-unknown-linux-gnu/lib/rustlib/src/rust/library/alloc/src/vec/mod.rs:3172:21
     |
3172 | pub fn from_elem<T: Clone>(elem: T, n: usize) -> Vec<T> {
     |                     ^^^^^ required by this bound in `from_elem`
help: consider annotating `State` with `#[derive(Clone)]`
     |
2    + #[derive(Clone)]
3    | enum State {
     |

So while I don't think the implementation of this needs to be blocked on a better diagnostic, I would encourage you to think about what it would take to hint for the user to write vec![const { expr }; N] when they wrote vec![expr; N] on a const non-Clone expr.

EFanZh commented 3 days ago

I don’t think the array syntax can be used to implement the vec! macro, since the array syntax requires the length being a const value, while the length used in vec! can be a runtime value.

scottmcm commented 3 days ago

Should it maybe be Vec::repeat(n, || whatever) instead? (Or from_fn or something_with or whatever)

It feels like the const-ness here is mostly irrelevant to what this would do, and it'd be just as useful to have this for non-const computations.

cuviper commented 3 days ago

Other examples where Clone may not be wanted or intended:

dtolnay commented 3 days ago

I don’t think the array syntax can be used to implement the vec! macro, since the array syntax requires the length being a const value

Good call; I didn't consider non-const N — so this would only be serving as shorthand for iter::repeat_with(|| expr).take(N).collect::<Vec<_>>(). I agree with Scott that the const doesn't feel necessary or relevant in that case.

scottmcm commented 3 days ago

@cuviper Good point! A particularly subtle one is vec![HashMap::new(); n], since that keeps them all from getting different randomstates, whereas repeat_with(HashMap::new).take(n).collect() will get n different randomstates.

cuviper commented 3 days ago

We should also be careful about how this interacts with expr/expr_2021 changes: https://doc.rust-lang.org/nightly/edition-guide/rust-2024/macro-fragment-specifiers.html

... but maybe that just means we would have to manually match the const before the more general expr.

EFanZh commented 2 days ago

Vec::repeat(n, || ...) doesn’t have to replace the vec![const { ... }; n] syntax.

So, regardless of whether Vec::repeat(n, || ...) should be supported, it is still useful to support vec![const { ... }; n].