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

Impl conversion from array to Vec #352

Merged
merged 2 commits into from
Jan 1, 2024

Conversation

rjsberry
Copy link
Contributor

@rjsberry rjsberry commented Mar 2, 2023

This PR implements conversion from arrays to vec. The array does not have to be the same length as the capacity of the vec. Const generics are asserted at compile time. Unfortunately the method currently requires a copy of the source array.

Partially addresses requests in #341 and #344.

Also I'm not totally happy about the const assertions given #351. I've not added any tests in the cfail suite as I don't think we can practically check this with trybuild at the moment. There is a bit of a work-around you can do but it's not pretty.

Questions

  1. Would we prefer to just implement From<[T; N]> for Vec<T, N>? One of the issues with this impl is that users will need to be explicit about vec capacity. Because lengths N and M can be different, you can't write:

    let v = Vec::from([1, 2, 3]);

    It needs to be:

    let v = Vec::<_, 3>::from([1, 2, 3]);
  2. Should from_array be internal, and the API exposed by the crate simply be the From trait?

@rjsberry rjsberry force-pushed the impl-vec-from-array branch 2 times, most recently from c0aeb9d to 81c61a2 Compare March 2, 2023 10:06
Dirbaio
Dirbaio previously approved these changes Jan 1, 2024
Copy link
Member

@Dirbaio Dirbaio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thank you!

@Dirbaio Dirbaio added this pull request to the merge queue Jan 1, 2024
Merged via the queue into rust-embedded:main with commit e23e9b6 Jan 1, 2024
22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants