Skip to content

Add explicit transfer safety requirement - #25271

Open
yilin0518 wants to merge 1 commit into
bevyengine:mainfrom
yilin0518:fix_Ptr_inconsistent
Open

Add explicit transfer safety requirement#25271
yilin0518 wants to merge 1 commit into
bevyengine:mainfrom
yilin0518:fix_Ptr_inconsistent

Conversation

@yilin0518

Copy link
Copy Markdown
Contributor

Objective

  • In bevy_ptr, MovingPtr::new, PtrMut::promote, and OwningPtr::new are unsafe methods that transfer the right to move out of / drop a pointee to the returned pointer type, but their # Safety docs never say so explicitly. They only state validity/alignment/provenance requirements for the pointer while it is live, and omit what the caller (or whatever previously owned the pointee) is required to do after the returned pointer has been consumed.
  • This is a real contract gap: a caller could satisfy every bullet point as literally written, and still trigger a double-drop or use-after-move by continuing to access/drop the pointee through its original location after handing ownership to the new pointer. MovingPtr::from_value already documents this exact invariant ("Once the returned MovingPtr has been used, value must be treated as if it were uninitialized unless it was explicitly leaked via mem::forget"), so the three sibling constructors were simply inconsistent/incomplete by comparison.

Solution

  • Added an explicit bullet to the # Safety docs of MovingPtr::new, PtrMut::promote, and OwningPtr::new stating that the call transfers move/drop rights to the returned pointer, and that once it has been used (moved out of, read, dropped, or forgotten), whatever previously owned the pointee must treat it as uninitialized and must not access or drop it again unless that location is properly reinitialized — matching the wording already used by MovingPtr::from_value.
  • Doc-only change; no behavior was modified.

Testing

  • cargo check -p bevy_ptr passes; this is a doc comment change only, so no functional testing is needed.
  • This is doc-only change, so I don't check any other tests.

Thank you for your review, and looking forward to your suggestion!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant