Conversation
joshlf
requested changes
May 2, 2024
jswrenn
commented
May 7, 2024
Comment on lines
+1178
to
+1194
| match Ptr::from_ref(candidate).try_cast_into_no_leftover::<Self>() { | ||
| Ok(candidate) => { | ||
| // This call may panic. If that happens, it doesn't cause any soundness | ||
| // issues, as we have not generated any invalid state which we need to | ||
| // fix before returning. | ||
| // | ||
| // Note that one panic or post-monomorphization error condition is | ||
| // calling `try_into_valid` (and thus `is_bit_valid`) with a shared | ||
| // pointer when `Self: !Immutable`. Since `Self: Immutable`, this panic | ||
| // condition will not happen. | ||
| match candidate.try_into_valid() { | ||
| Ok(valid) => Ok(valid.as_ref()), | ||
| Err(e) => Err(e.map_src(|src| src.as_bytes().as_ref()).into()), | ||
| } | ||
| } | ||
| Err(e) => Err(e.map_src(Ptr::as_ref).into()), | ||
| } |
Collaborator
Author
There was a problem hiding this comment.
Rewritten in this manner for consistency with our other from_ref methods, and because doing so allows us to name try_ref_from's argument "candidate" without thinking of new, weird identifier names.
25819dd to
0e1bf9a
Compare
Collaborator
Author
|
@joshlf Need to write a commit message, but otherwise this is ready for review. This PR intentionally omits prefix/suffix-returning versions of |
joshlf
requested changes
May 8, 2024
src/lib.rs
Outdated
| /// [here][TryFromBytes#what-is-a-valid-instance] for a discussion of how | ||
| /// these cases are handled. | ||
| /// If the bytes of `candidate` are a valid instance of `Self`, this method | ||
| /// returns a reference to those bytes interpreted as `Self`. If |
Member
There was a problem hiding this comment.
Suggested change
| /// returns a reference to those bytes interpreted as `Self`. If | |
| /// returns a reference to those bytes reinterpreted as a `Self`. If |
src/lib.rs
Outdated
| /// | ||
| /// If the first `size_of::<Self>()` bytes of `candidate` are a valid | ||
| /// instance of `Self`, this method returns both a reference to those bytes | ||
| /// interpreted as `Self`, and a reference to the remaining bytes. If |
Member
There was a problem hiding this comment.
Suggested change
| /// interpreted as `Self`, and a reference to the remaining bytes. If | |
| /// reinterpreted as a `Self`, and a reference to the remaining bytes. If |
src/lib.rs
Outdated
| /// | ||
| /// If the last `size_of::<Self>()` bytes of `candidate` are a valid | ||
| /// instance of `Self`, this method returns both a reference to those bytes | ||
| /// interpreted as `Self`, and a reference to the preceding bytes. If |
Member
There was a problem hiding this comment.
Suggested change
| /// interpreted as `Self`, and a reference to the preceding bytes. If | |
| /// reinterpreted as a `Self`, and a reference to the preceding bytes. If |
src/lib.rs
Outdated
| /// | ||
| /// If the bytes of `candidate` are a valid instance of `Self`, reads those | ||
| /// bytes as `Self`. If `candidate.len() < size_of::<Self>()` or `candidate` | ||
| /// is not aligned to `align_of::<Self>()` or the bytes are not a valid |
Makes progress towards #5
joshlf
approved these changes
May 9, 2024
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
No description provided.