-
Notifications
You must be signed in to change notification settings - Fork 209
Implements TryFrom for Deque from array #524
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wont be able to properly review this for a bit, wanted to highlight a couple typos until then.
aa6f7a2
to
e922e5a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the slow review, can merge after these small changes!
Co-authored-by: Alex Martens <alex@thinglab.org>
9c6db76
to
8907839
Compare
.gitignore
Outdated
@@ -2,3 +2,4 @@ | |||
.#* | |||
Cargo.lock | |||
target/ | |||
.idea |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressed aca4aa9
src/deque.rs
Outdated
/// ``` | ||
/// use heapless::Deque; | ||
/// | ||
/// let deq1 = Deque::<u8, 4>::try_from([1, 2, 3, 4]).unwrap(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might make more sense to use different NS
/ND
in the example.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this what you mean? 2dfeabe
src/deque.rs
Outdated
/// | ||
/// assert_eq!(deq1, deq2); | ||
/// ``` | ||
type Error = (); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should use CapacityError
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressed here aca4aa9
Fixes #522.
Ideally, this PR is updated after PR #521 is merged to simplify the tests.
This PR implements the
TryFrom
trait for creating aDeque
from a slice.Note the use of
unsafe
for copying all bytes from the slice to theDeque
buffer after ensuring that theDeque
buffer has enough space.Also note the use of
ManuallyDrop
to ensure that any heap memory referred to by the element contents is not dropped at the end of this method since the elements in theDeque
buffer will be pointing to it.