Skip to content

Implement Vec::drain, as_(mut_)view on the *Inner types, generic over Storage #500

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

Merged
merged 5 commits into from
Apr 5, 2025

Conversation

sosthene-nitrokey
Copy link
Contributor

This makes drain usable in the context of an implementation generic over the storage
This also makes it possible to get a *View in this same context

This could also be a way to "de-monomorphize" all the implementations of the Inner structs:

We could make all implementation define an inner function that is over a View type, and start the
implementation with this = self.as_(mut_)view, maybe for binary size and compilation time benefits. For example the implementation of VecInner::push could be:

    /// Appends an `item` to the back of the collection
    ///
    /// Returns back the `item` if the vector is full.
    pub fn push(&mut self, item: T) -> Result<(), T> {
        fn inner<T>(this: &mut VecView<T>, item: T) -> Result<(), T> {
            if this.len < this.storage_capacity() {
                unsafe { this.push_unchecked(item) }
                Ok(())
            } else {
                Err(item)
            }
        }
        inner(self.as_mut_view(), item)
    }

@sosthene-nitrokey sosthene-nitrokey changed the title Implement Vec::drain, as_(mut_)view the *Inner types, generic over Storage Implement Vec::drain, as_(mut_)view on the *Inner types, generic over Storage Jul 3, 2024
@sosthene-nitrokey
Copy link
Contributor Author

I checked with nightly, we can't do that with just a type Buffer<T>: Unsize<[T], because there is no implementation of Unsize<[T]> for [T], that's a shame. So even if Unsize gets stabilized, we'll have to keep all the manual conversion methods.

@sosthene-nitrokey sosthene-nitrokey force-pushed the vec-drain-generic branch 2 times, most recently from dd08393 to e4de53b Compare March 3, 2025 20:18
@sosthene-nitrokey sosthene-nitrokey force-pushed the vec-drain-generic branch 4 times, most recently from c411836 to eb0fc28 Compare March 21, 2025 16:09
@sosthene-nitrokey
Copy link
Contributor Author

To get it working with LinearMap and String (which have constraints on the the inner type of the Vec), I had to create new traits for them. I wonder if that would not be beneficial for the other types too.

@sosthene-nitrokey
Copy link
Contributor Author

The cpass tests are blocked by a compiler bug that has been fixed today. I'm not sure if the fix will be included in 1.86 or whether we will have to wait for 1.87: rust-lang/rust#138979

@sosthene-nitrokey
Copy link
Contributor Author

Ok, I have a workaround for the compiler ICE, that doesn't appear to have any downsides. Added it and documented it.

Verified

This commit was signed with the committer’s verified signature.

Verified

This commit was signed with the committer’s verified signature.

Verified

This commit was signed with the committer’s verified signature.

Verified

This commit was signed with the committer’s verified signature.

Verified

This commit was signed with the committer’s verified signature.
@reitermarkus
Copy link
Member

Thanks, @sosthene-nitrokey!

Merged via the queue into rust-embedded:main with commit 4b78bc1 Apr 5, 2025
22 of 23 checks passed
sosthene-nitrokey added a commit to Nitrokey/heapless that referenced this pull request Apr 7, 2025

Verified

This commit was signed with the committer’s verified signature.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants