Keep Vec::capacity const
#498
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
In 0.8.0 it was const, but this was removed in rust-embedded#486 The other container types did not make the `capacity` method const, and therefore can kept with the normal name and the generic implementation.
sosthene-nitrokey
force-pushed
the
breaking-change-vec-capacity
branch
from
94b0ea5 to
27bff4a
Compare
reitermarkus
reviewed
Jul 2, 2024
| @@ -408,7 +415,7 @@ impl<T, S: Storage> VecInner<T, S> { | |||
| } | |||
|
|
|||
| /// Returns the maximum number of elements the vector can hold. | |||
| pub fn capacity(&self) -> usize { | |||
| pub fn storage_capacity(&self) -> usize { | |||
There was a problem hiding this comment.
@sosthene-nitrokey, why can't this use the same name?
There was a problem hiding this comment.
storage_capacity applies to VecInner<T, S>, for any storage
capacity applies to VecInner<T, OwnedStorage<N>> aka Vec<T, N> only.
so for Vec<T, N> both apply, so it'd give ambiguity errors if the user wrote vec.capacity()
There was a problem hiding this comment.
so for Vec<T, N> both apply, so it'd give ambiguity errors if the user wrote vec.capacity()
It wouldn't even allow getting to that point. The compiler would complain about duplicate definitions with the same name.
We could implement fn capacity on Vec and VecView independently, but then it would make it unusable in a generic context over the storage.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In 0.8.0 it was const, but this was removed in #486, which would be an unexpected breaking change.
The other container types did not make the
capacitymethod const, and therefore can kept with the normal name and the generic implementation.No need for changelog for this change, since it affects b previous PR that has not been released.