Skip to content

Commit 16d065d

Browse files
Fix safety comment citations (#2800)
* Fix safety comment citations to use pinned Rust versions Updates safety comments in `src/impls.rs`, `src/util/mod.rs`, `src/lib.rs`, `src/pointer/ptr.rs`, and `zerocopy-derive/src/enum.rs` to cite specific stable Rust versions (mainly 1.81.0) instead of generic `stable` or `nightly` URLs. Adds quotes from the documentation to support the safety claims, as per the issue requirements. Resolves FIXMEs related to missing citations for `NonZero` layout, `Option<NonZero>` compatibility, and `null` pointer representation. Closes #1655 * Fix safety comment citations to use pinned Rust versions Updates safety comments in `src/impls.rs`, `src/util/mod.rs`, `src/lib.rs`, `src/pointer/ptr.rs`, and `zerocopy-derive/src/enum.rs` to cite specific stable Rust versions (mainly 1.81.0) instead of generic `stable` or `nightly` URLs. Adds quotes from the documentation to support the safety claims, as per the issue requirements. Resolves FIXMEs related to missing citations for `NonZero` layout, `Option<NonZero>` compatibility, and `null` pointer representation. Clarifies safety comment in `src/pointer/ptr.rs` regarding validity preservation when iterating over slices. Closes #1655 * Update NonZero URLs to type alias format Corrects the URLs for `NonZeroU8` and `NonZeroI8` documentation to point to `type.NonZero*.html` instead of `struct.NonZero*.html`, as they are type aliases in Rust 1.81.0. Ensures quoted text regarding layout and Option compatibility is accurate. * Add NPO size guarantee quote for NonZero Adds the documentation quote "Thanks to the null pointer optimization, NonZero... and Option<NonZero...> are guaranteed to have the same size and alignment" to the safety comments for `Option<NonZeroU8>` and `Option<NonZeroI8>`. --------- Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>
1 parent 9483f7d commit 16d065d

File tree

5 files changed

+52
-24
lines changed

5 files changed

+52
-24
lines changed

src/impls.rs

Lines changed: 31 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -154,12 +154,13 @@ impl_size_eq!(char, Unalign<u32>);
154154
// Note that we don't `assert_unaligned!(str)` because `assert_unaligned!` uses
155155
// `align_of`, which only works for `Sized` types.
156156
//
157-
// FIXME(#429):
158-
// - Add quotes from documentation.
159-
// - Improve safety proof for `FromZeros` and `IntoBytes`; having the same
160-
// layout as `[u8]` isn't sufficient.
157+
// FIXME(#429): Improve safety proof for `FromZeros` and `IntoBytes`; having the same
158+
// layout as `[u8]` isn't sufficient.
159+
//
160+
// [1] Per https://doc.rust-lang.org/1.81.0/reference/type-layout.html#str-layout:
161161
//
162-
// [1] https://doc.rust-lang.org/1.81.0/reference/type-layout.html#str-layout
162+
// String slices are a UTF-8 representation of characters that have the same
163+
// layout as slices of type `[u8]`.
163164
#[allow(clippy::multiple_unsafe_ops_per_block)]
164165
const _: () = unsafe { unsafe_impl!(str: Immutable, FromZeros, IntoBytes, Unaligned) };
165166

@@ -216,15 +217,15 @@ macro_rules! unsafe_impl_try_from_bytes_for_nonzero {
216217
// how we'd prove it short of adding text to the stdlib docs that says so
217218
// explicitly, which likely wouldn't be accepted.
218219
//
219-
// [1] https://doc.rust-lang.org/1.81.0/std/num/type.NonZeroU8.html
220+
// [1] Per https://doc.rust-lang.org/1.81.0/std/num/type.NonZeroU8.html:
220221
//
221222
// `NonZeroU8` is guaranteed to have the same layout and bit validity as `u8` with
222-
// the exception that 0 is not a valid instance
223+
// the exception that 0 is not a valid instance.
223224
//
224-
// [2] https://doc.rust-lang.org/1.81.0/std/num/type.NonZeroI8.html
225+
// [2] Per https://doc.rust-lang.org/1.81.0/std/num/type.NonZeroI8.html:
225226
//
226-
// FIXME(https://github.com/rust-lang/rust/pull/104082): Cite documentation that
227-
// layout is the same as primitive layout.
227+
// `NonZeroI8` is guaranteed to have the same layout and bit validity as `i8` with
228+
// the exception that 0 is not a valid instance.
228229
#[allow(clippy::multiple_unsafe_ops_per_block)]
229230
const _: () = unsafe {
230231
unsafe_impl!(NonZeroU8: Immutable, IntoBytes, Unaligned);
@@ -267,13 +268,19 @@ const _: () = unsafe {
267268
// purpose of those types, it's virtually unthinkable that that would ever
268269
// change. The only valid alignment for a 1-byte type is 1.
269270
//
270-
// FIXME(#429): Add quotes from documentation.
271+
// [1] Per https://doc.rust-lang.org/1.81.0/std/num/type.NonZeroU8.html:
272+
//
273+
// `Option<NonZeroU8>` is guaranteed to be compatible with `u8`, including in FFI.
274+
//
275+
// Thanks to the null pointer optimization, `NonZeroU8` and `Option<NonZeroU8>`
276+
// are guaranteed to have the same size and alignment:
271277
//
272-
// [1] https://doc.rust-lang.org/stable/std/num/struct.NonZeroU8.html
273-
// [2] https://doc.rust-lang.org/stable/std/num/struct.NonZeroI8.html
278+
// [2] Per https://doc.rust-lang.org/1.81.0/std/num/type.NonZeroI8.html:
274279
//
275-
// FIXME(https://github.com/rust-lang/rust/pull/104082): Cite documentation for
276-
// layout guarantees.
280+
// `Option<NonZeroI8>` is guaranteed to be compatible with `i8`, including in FFI.
281+
//
282+
// Thanks to the null pointer optimization, `NonZeroI8` and `Option<NonZeroI8>`
283+
// are guaranteed to have the same size and alignment:
277284
#[allow(clippy::multiple_unsafe_ops_per_block)]
278285
const _: () = unsafe {
279286
unsafe_impl!(Option<NonZeroU8>: TryFromBytes, FromZeros, FromBytes, IntoBytes, Unaligned);
@@ -780,7 +787,7 @@ impl_for_transmute_from!(T: ?Sized + IntoBytes => IntoBytes for ManuallyDrop<T>[
780787
// SAFETY: `ManuallyDrop<T>` has the same layout as `T` [1], and thus has the
781788
// same alignment as `T`.
782789
//
783-
// [1] Per https://doc.rust-lang.org/nightly/core/mem/struct.ManuallyDrop.html:
790+
// [1] Per https://doc.rust-lang.org/1.81.0/std/mem/struct.ManuallyDrop.html:
784791
//
785792
// `ManuallyDrop<T>` is guaranteed to have the same layout and bit validity as
786793
// `T`
@@ -928,8 +935,14 @@ const _: () = unsafe {
928935
// `IntoBytes` for raw pointers eventually, but we are holding off until we can
929936
// figure out how to address those footguns.
930937
//
931-
// [1] FIXME(https://github.com/rust-lang/rust/pull/116988): Cite the
932-
// documentation once this PR lands.
938+
// [1] Per https://doc.rust-lang.org/1.81.0/std/ptr/fn.null.html:
939+
//
940+
// Creates a null raw pointer.
941+
//
942+
// This function is equivalent to zero-initializing the pointer:
943+
// `MaybeUninit::<*const T>::zeroed().assume_init()`.
944+
//
945+
// The resulting pointer has the address 0.
933946
#[allow(clippy::multiple_unsafe_ops_per_block)]
934947
const _: () = unsafe {
935948
unsafe_impl!(T: ?Sized => Immutable for *const T);

src/lib.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3158,7 +3158,7 @@ pub unsafe trait FromZeros: TryFromBytes {
31583158
// is sufficiently aligned. Since the produced pointer is a
31593159
// `NonNull`, it is non-null.
31603160
//
3161-
// [1] Per https://doc.rust-lang.org/nightly/std/boxed/index.html#memory-layout:
3161+
// [1] Per https://doc.rust-lang.org/1.81.0/std/boxed/index.html#memory-layout:
31623162
//
31633163
// For zero-sized values, the `Box` pointer has to be non-null and sufficiently aligned.
31643164
//

src/pointer/ptr.rs

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1225,8 +1225,17 @@ mod _project {
12251225
// 1. `elem`, conditionally, conforms to the validity invariant of
12261226
// `I::Alignment`. If `elem` is projected from data well-aligned
12271227
// for `[T]`, `elem` will be valid for `T`.
1228-
// 2. FIXME: Need to cite facts about `[T]`'s layout (same for the
1229-
// preceding points)
1228+
// 2. `elem` conforms to the validity invariant of `I::Validity`.
1229+
// Per https://doc.rust-lang.org/1.81.0/reference/type-layout.html#array-layout:
1230+
//
1231+
// Slices have the same layout as the section of the array they
1232+
// slice.
1233+
//
1234+
// Arrays are laid out so that the zero-based `nth` element of
1235+
// the array is offset from the start of the array by `n *
1236+
// size_of::<T>()` bytes. Thus, `elem` addresses a valid `T`
1237+
// within the slice. Since `self` satisfies `I::Validity`, `elem`
1238+
// also satisfies `I::Validity`.
12301239
self.as_inner().iter().map(|elem| unsafe { Ptr::from_inner(elem) })
12311240
}
12321241
}

src/util/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -372,7 +372,7 @@ where
372372
// check ensures their shared safety precondition: that the supplied
373373
// layout is not zero-sized type [1].
374374
//
375-
// [1] Per https://doc.rust-lang.org/stable/std/alloc/trait.GlobalAlloc.html#tymethod.alloc:
375+
// [1] Per https://doc.rust-lang.org/1.81.0/std/alloc/trait.GlobalAlloc.html#tymethod.alloc:
376376
//
377377
// This function is unsafe because undefined behavior can result if
378378
// the caller does not ensure that layout has non-zero size.

zerocopy-derive/src/enum.rs

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -71,8 +71,14 @@ fn generate_tag_consts(data: &DataEnum) -> TokenStream {
7171
// Because these are the same size, this is defined to be a no-op
7272
// and therefore is a lossless conversion [2].
7373
//
74-
// [1]: https://doc.rust-lang.org/stable/reference/expressions/operator-expr.html#enum-cast
75-
// [2]: https://doc.rust-lang.org/stable/reference/expressions/operator-expr.html#numeric-cast
74+
// [1] Per https://doc.rust-lang.org/1.81.0/reference/expressions/operator-expr.html#enum-cast:
75+
//
76+
// Casts an enum to its discriminant.
77+
//
78+
// [2] Per https://doc.rust-lang.org/1.81.0/reference/expressions/operator-expr.html#numeric-cast:
79+
//
80+
// Casting between two integers of the same size (e.g. i32 -> u32)
81+
// is a no-op.
7682
#[allow(non_upper_case_globals)]
7783
const #tag_ident: ___ZerocopyTagPrimitive =
7884
___ZerocopyTag::#variant_ident as ___ZerocopyTagPrimitive;

0 commit comments

Comments
 (0)