alloc: a bunch of safety comments by nia-e · Pull Request #162289 · rust-lang/rust

nia-e · 2026-09-04T11:04:26Z

Following up from #160941. Triaging this is what found the errors in #162285 & #162286. More to come, but I didn't want to make the review effort too high on any single PR.

r? libs

Darksonn · 2026-09-04T11:12:11Z

+    /// # Safety
+    ///
+    /// Either `value` is valid for reads and writes, or it is `NonNull::dangling()`
+    /// if both `T` and `H` are ZSTs.


This is redundant. As per std::ptr, all non-null pointers are valid for reads and writes when the target type is ZST.

Suggested change

/// # Safety

///

/// Either `value` is valid for reads and writes, or it is `NonNull::dangling()`

/// if both `T` and `H` are ZSTs.

/// # Safety

///

/// `value` must is valid for reads and writes.

Though I'm not really sure this is sufficient. I would expect to be able to call a method with this safety comment twice in a row.

Suggested change

/// # Safety

///

/// Either `value` is valid for reads and writes, or it is `NonNull::dangling()`

/// if both `T` and `H` are ZSTs.

/// # Safety

///

/// `value` must point at an owned `T` that can be dropped, and `self` must not

/// be accessed again after this call.

View changes since the review

Darksonn · 2026-09-04T11:20:24Z

+        // SAFETY: `slice` is a valid pointer for `len` `T`s, and the
+        // above `ManuallyDrop` ensures that the destructor of `me` which
+        // would free the allocation is never run. Moving the allocator
+        // out of `me.inner` is also sound since it is never accessed after
+        // this point.
+        unsafe { Box::from_raw_in(slice, ptr::read(&me.inner.alloc)) }


Should address why this creates a valid Box. In particular, if I call this and then drop the resulting Box, why does the destructor of Box not violate the safety requirements of dealloc, in particular the requirements about the length being the same as what it was allocated with?

View changes since the review

Darksonn · 2026-09-04T11:33:44Z

+            // SAFETY: We're copying `rem_len` elements after offsetting by `len`,
+            // with the previous `copy_nonverlapping` calls ensuring that the first `len`
+            // elements are valid `T`s and the call to `with_capacity` ensuring we have
+            // `rem_len` space to write the new elements. That is, these remaining `rem_len`
+            // elements must be preceded by more than `rem_len` previously-copied elements.
+            // Setting the length is correct since we've initialised the whole `capacity`-length
+            // space with copies of the previous `len` elements.


This must argue that it's in-bounds of the allocation and that there's no overlap.

It's clear that this in-bounds of the allocation, but this doesn't really explain why rem_len <= buf.len().

View changes since the review

Darksonn · 2026-09-04T11:37:17Z

+                // SAFETY: We're copying `len` elements after offsetting by `len`,
+                // with the previous call to `extend` ensuring that the first `len`
+                // elements are valid `T`s and the call to `with_capacity` ensuring
+                // we have `len * n` space to write the new elements.


This must argue that it's in-bounds of the allocation and that there's no overlap.

It's clear that there is no overlap, but this doesn't really explain why 2*buf.len() <= capacity.

No need to explain that the elements are valid because copy_nonoverlapping performs an untyped copy, so validity is not required. Explaining it on the set_len call is sufficient.

View changes since the review

Darksonn · 2026-09-04T11:39:45Z

+                unsafe {
                    buf.set_len(buf_len * 2);
                }


This is a super duper nit, but it formats slightly nicer if you move the semicolon

Suggested change

unsafe {

buf.set_len(buf_len * 2);

}

unsafe { buf.set_len(buf_len * 2) };

View changes since the review

Darksonn

@bors r+

View changes since this review

rust-bors · 2026-09-04T12:27:54Z

📌 Commit e578090 has been approved by Darksonn

It is now in the queue for this repository.

…, r=Darksonn alloc: a bunch of safety comments Following up from rust-lang#160941. Triaging this is what found the errors in rust-lang#162285 & rust-lang#162286. More to come, but I didn't want to make the review effort too high on any single PR. r? libs

Rollup of 25 pull requests Successful merges: - #159074 ([PAC] FnAbi, llvm.ptrauth.resign and Session API change (2/8)) - #159792 (A more readable debug map for IndexMaps) - #161895 (std::sys::pal::sgx: fix mismatched alloc/free alignment) - #161900 (bootstrap: Include feature-gated items in bootstrap tool docs) - #161940 (Promote `wasm32-wasip3` to a tier 2 target) - #162072 (Add new Tier-3 target: `powerpc64-sony-ps3`) - #162179 (type system const items via direct rhs) - #162277 (Introduce `rustc_middle::middel::resolve`) - #162285 (box: fixup map/try_map deallocate calls) - #162286 (string: don't unwind prematurely) - #162289 (alloc: a bunch of safety comments) - #162292 (Update `askama` version to `0.16.1`) - #160509 (Remove `RegionExt`; move methods to `Region` in `rustc_type_ir`) - #160906 (Suggest usize instead of placeholder type for array length constants) - #160936 (traits: Represent live alias arguments as bitsets) - #161400 (Improve diagnostics for references to closures) - #161656 (Suggest mutable references for FnMut closure arguments) - #161711 (Add more splat fn type tests) - #161786 (Make `tcx.def_id_partial_cmp` public) - #161953 (sanitizers: Implicitly disable mutually exclusive sanitizers) - #162155 (add suggestion for `rustc_allowed_through_unstable_modules` attribute) - #162212 (Implement `Rng` for `Box`) - #162246 (Fix incorrect meta span) - #162266 (std: fix typo) - #162291 (Add regression test from 1.98.1)

…, r=Darksonn alloc: a bunch of safety comments Following up from rust-lang#160941. Triaging this is what found the errors in rust-lang#162285 & rust-lang#162286. More to come, but I didn't want to make the review effort too high on any single PR. r? libs

Rollup of 25 pull requests Successful merges: - #159074 ([PAC] FnAbi, llvm.ptrauth.resign and Session API change (2/8)) - #159792 (A more readable debug map for IndexMaps) - #160745 (make closures act like MaybeDangling) - #161895 (std::sys::pal::sgx: fix mismatched alloc/free alignment) - #161940 (Promote `wasm32-wasip3` to a tier 2 target) - #162072 (Add new Tier-3 target: `powerpc64-sony-ps3`) - #162179 (type system const items via direct rhs) - #162277 (Introduce `rustc_middle::middel::resolve`) - #162285 (box: fixup map/try_map deallocate calls) - #162286 (string: don't unwind prematurely) - #162289 (alloc: a bunch of safety comments) - #162292 (Update `askama` version to `0.16.1`) - #160509 (Remove `RegionExt`; move methods to `Region` in `rustc_type_ir`) - #160906 (Suggest usize instead of placeholder type for array length constants) - #160936 (traits: Represent live alias arguments as bitsets) - #161400 (Improve diagnostics for references to closures) - #161656 (Suggest mutable references for FnMut closure arguments) - #161711 (Add more splat fn type tests) - #161786 (Make `tcx.def_id_partial_cmp` public) - #161953 (sanitizers: Implicitly disable mutually exclusive sanitizers) - #162155 (add suggestion for `rustc_allowed_through_unstable_modules` attribute) - #162212 (Implement `Rng` for `Box`) - #162246 (Fix incorrect meta span) - #162266 (std: fix typo) - #162291 (Add regression test from 1.98.1)

…, r=Darksonn alloc: a bunch of safety comments Following up from rust-lang#160941. Triaging this is what found the errors in rust-lang#162285 & rust-lang#162286. More to come, but I didn't want to make the review effort too high on any single PR. r? libs

Rollup of 27 pull requests Successful merges: - #159074 ([PAC] FnAbi, llvm.ptrauth.resign and Session API change (2/8)) - #159792 (A more readable debug map for IndexMaps) - #160745 (make closures act like MaybeDangling) - #161940 (Promote `wasm32-wasip3` to a tier 2 target) - #162030 (Prevent `--test` to be used in `rustdoc-html` testsuite) - #162072 (Add new Tier-3 target: `powerpc64-sony-ps3`) - #162179 (type system const items via direct rhs) - #162262 (Avoid manually instantiating some binders in error reporting with `-Znext-solver`) - #162277 (Introduce `rustc_middle::middel::resolve`) - #162285 (box: fixup map/try_map deallocate calls) - #162286 (string: don't unwind prematurely) - #162289 (alloc: a bunch of safety comments) - #162290 (abby test DSL: AliasTyOutlivesViaEnv) - #162292 (Update `askama` version to `0.16.1`) - #160509 (Remove `RegionExt`; move methods to `Region` in `rustc_type_ir`) - #160906 (Suggest usize instead of placeholder type for array length constants) - #160936 (traits: Represent live alias arguments as bitsets) - #161400 (Improve diagnostics for references to closures) - #161656 (Suggest mutable references for FnMut closure arguments) - #161711 (Add more splat fn type tests) - #161786 (Make `tcx.def_id_partial_cmp` public) - #161953 (sanitizers: Implicitly disable mutually exclusive sanitizers) - #162155 (add suggestion for `rustc_allowed_through_unstable_modules` attribute) - #162212 (Implement `Rng` for `Box`) - #162246 (Fix incorrect meta span) - #162266 (std: fix typo) - #162291 (Add regression test from 1.98.1)

…, r=Darksonn alloc: a bunch of safety comments Following up from rust-lang#160941. Triaging this is what found the errors in rust-lang#162285 & rust-lang#162286. More to come, but I didn't want to make the review effort too high on any single PR. r? libs

alloc: a bunch of safety comments

b1a9fca

rustbot assigned Darksonn Sep 4, 2026

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Sep 4, 2026

Darksonn reviewed Sep 4, 2026

View reviewed changes

review

1c48bd6

This comment has been minimized.

Sign in to view

typo

e578090

Darksonn approved these changes Sep 4, 2026

View reviewed changes

rust-bors Bot added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 4, 2026

JonathanBrouwer mentioned this pull request Sep 4, 2026

Rollup of 13 pull requests #162293

Closed

GuillaumeGomez mentioned this pull request Sep 4, 2026

Rollup of 25 pull requests #162297

Closed

GuillaumeGomez mentioned this pull request Sep 4, 2026

Rollup of 25 pull requests #162298

Closed

GuillaumeGomez mentioned this pull request Sep 4, 2026

Rollup of 25 pull requests #162300

Closed

GuillaumeGomez mentioned this pull request Sep 4, 2026

Rollup of 28 pull requests #162307

Closed

GuillaumeGomez mentioned this pull request Sep 4, 2026

Rollup of 27 pull requests #162308

Closed

JonathanBrouwer mentioned this pull request Sep 4, 2026

Rollup of 25 pull requests #162310

Open

Navigation Menu

Uh oh!

Uh oh!

alloc: a bunch of safety comments - #162289

alloc: a bunch of safety comments#162289
nia-e wants to merge 3 commits into
rust-lang:mainfrom
nia-e:alloc-safety-comments-part-2

nia-e commented Sep 4, 2026 •

edited by rustbot

Loading

Uh oh!

Darksonn Sep 4, 2026 •

edited by rustbot

Loading

Uh oh!

Darksonn Sep 4, 2026 •

edited by rustbot

Loading

Uh oh!

Darksonn Sep 4, 2026 •

edited by rustbot

Loading

Uh oh!

Darksonn Sep 4, 2026 •

edited by rustbot

Loading

Uh oh!

Darksonn Sep 4, 2026 •

edited by rustbot

Loading

Uh oh!

This comment has been minimized.

Darksonn left a comment •

edited by rustbot

Loading

Uh oh!

rust-bors Bot commented Sep 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Uh oh!

Conversation

nia-e commented Sep 4, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Darksonn Sep 4, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Darksonn Sep 4, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Darksonn Sep 4, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Darksonn Sep 4, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Darksonn Sep 4, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

Darksonn left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rust-bors Bot commented Sep 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

nia-e commented Sep 4, 2026 •

edited by rustbot

Loading

Darksonn Sep 4, 2026 •

edited by rustbot

Loading

Darksonn Sep 4, 2026 •

edited by rustbot

Loading

Darksonn Sep 4, 2026 •

edited by rustbot

Loading

Darksonn Sep 4, 2026 •

edited by rustbot

Loading

Darksonn Sep 4, 2026 •

edited by rustbot

Loading

Darksonn left a comment •

edited by rustbot

Loading