Conversation
Refactors these routines to not depend on padding byte preservation across moves. Along the way, we make the `try_read_from` helper safe, generic, and better-optimized. Instruction counts for the `try_read_from_*` change by -36% to -55%. gherrit-pr-id: G35c8b8db33f4c984d5716af9a26c0a5ec9bd9c7e
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## Gd1b460a8ec2e8aa8adf2e070c329d9f209c7c89c #3712 +/- ##
=============================================================================
+ Coverage 92.51% 92.63% +0.11%
=============================================================================
Files 20 20
Lines 6362 6424 +62
=============================================================================
+ Hits 5886 5951 +65
+ Misses 476 473 -3 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 12bc48fa9b
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| // SAFETY: The size check ensures that `bytes` is readable for | ||
| // `size_of::<T>()` bytes. `candidate.as_mut_ptr()` is writable for the | ||
| // same number of bytes because `MaybeUninit<T>` has `T`'s size. Both | ||
| // pointers are non-null and aligned for `u8`, including when `T` is | ||
| // zero-sized. The fresh local allocation cannot overlap `source`. |
There was a problem hiding this comment.
This new unsafe block relies on uncited premises that source and the local allocation cannot overlap and that a u8 copy preserves initialization (and provenance for typed sources containing pointers). The quoted copy_nonoverlapping excerpt only states that bytes are copied and requires non-overlap; it does not substantiate those premises or even the stated read, write, and alignment requirements. Extend the proof with versioned official quotations covering these guarantees, or use an operation such as ptr::copy that removes the unproven non-overlap premise. zerocopy/agent_docs/unsafe_code.mdL54-L65
Useful? React with 👍 / 👎.
| // SAFETY: The size check ensures that `bytes` is readable for | ||
| // `size_of::<T>()` bytes. `candidate.as_mut_ptr()` is writable for the | ||
| // same number of bytes because `MaybeUninit<T>` has `T`'s size. Both | ||
| // pointers are non-null and aligned for `u8`, including when `T` is | ||
| // zero-sized. The fresh local allocation cannot overlap `source`. |
There was a problem hiding this comment.
Do we also need to cite that this is bit-valid for the destination (MaybeUninit<T>) type?
This branch has not been deployed
3 participants
Refactors these routines to not depend on padding byte preservation
across moves. Along the way, we make the
try_read_fromhelper safe,generic, and better-optimized. Instruction counts for the
try_read_from_*change by -36% to -55%.TryFromBytes::try_read_from*#3712(try_)ref_from_prefix_suffix#3711ReadOnly#3682⬇️ Download this PR
Branch
git fetch origin refs/heads/G35c8b8db33f4c984d5716af9a26c0a5ec9bd9c7e && git checkout -b pr-G35c8b8db33f4c984d5716af9a26c0a5ec9bd9c7e FETCH_HEADCheckout
git fetch origin refs/heads/G35c8b8db33f4c984d5716af9a26c0a5ec9bd9c7e && git checkout FETCH_HEADCherry Pick
git fetch origin refs/heads/G35c8b8db33f4c984d5716af9a26c0a5ec9bd9c7e && git cherry-pick FETCH_HEADPull
Stacked PRs enabled by GHerrit.