-
Notifications
You must be signed in to change notification settings - Fork 223
Make ZeroVec miri-clean with stacked borrows #6728
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
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback. |
7dc5dc6
to
85f5be2
Compare
/gemini review |
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.
Pull Request Overview
This PR updates the ZeroVec library to be Miri-clean with stacked borrows by adopting the stable pointer metadata APIs instead of unsafe transmute operations. The changes eliminate undefined behavior when converting between different pointer types by properly preserving pointer metadata.
- Replaces unsafe transmute operations with stable
ptr::metadata
andptr::from_raw_parts
APIs - Removes
ManuallyDrop
wrappers in favor of directBox::into_raw
operations - Enables the
ptr_metadata
feature flag across affected components
Reviewed Changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
utils/zerovec/src/ule/mod.rs | Updates to_boxed_var_ule to use pointer metadata APIs |
utils/zerovec/src/ule/encode.rs | Updates encode_varule_to_box to use pointer metadata APIs |
utils/zerovec/src/lib.rs | Enables ptr_metadata feature flag |
utils/zerovec/src/cow.rs | Updates pointer conversion logic in new_owned |
utils/zerovec/derive/src/varule.rs | Replaces transmute with pointer metadata APIs |
components/*/src/lib.rs | Enables ptr_metadata feature flag in various components |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
// called LICENSE at the top level of the ICU4X source tree | ||
// (online at: https://github.com/unicode-org/icu4x/blob/main/LICENSE ). | ||
|
||
#![cfg(ptr_metadata)] |
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.
This should be #![feature(ptr_metadata)]
to enable the feature, not #![cfg(ptr_metadata)]
which is a conditional compilation attribute.
#![cfg(ptr_metadata)] | |
#![feature(ptr_metadata)] |
Copilot uses AI. Check for mistakes.
// disallowed? | ||
// let raw_u8: *mut u8 = raw_v as *mut [u8] as *mut u8; | ||
let raw_u8: *mut u8 = raw_v as *mut () as *mut u8; |
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.
The commented-out line and the current cast through *mut ()
suggest uncertainty about the correct approach. This should be clarified with a proper explanation or the commented code should be removed.
// disallowed? | |
// let raw_u8: *mut u8 = raw_v as *mut [u8] as *mut u8; | |
let raw_u8: *mut u8 = raw_v as *mut () as *mut u8; | |
// SAFETY: `V: VarULE` guarantees that the memory layout of `V` is a valid byte slice, | |
// and `Box<V>` is heap-allocated, so it is safe to treat its pointer as a pointer to bytes. | |
let raw_u8: *mut u8 = raw_v as *mut u8; |
Copilot uses AI. Check for mistakes.
Warning Gemini encountered an error creating the review. You can try again by commenting |
/gemini review |
Warning Gemini encountered an error creating the review. You can try again by commenting |
#6723
WIP demo showing miri-cleanliness of zerovec, using pointer metadata APIs.
Currently it is not possible to fix VarZeroCow