Enforce safety comments on alloc - #160941
Conversation
rustbot
added
A-tidy
|
This will break a lot of ppl's CI ^^ @bors rollup=never |
|
I'm not convinced |
|
@bors rollup=iffy p=1 likely to soft conflict |
|
...i completely forgot to actually push the commit with all my manual work. apologies 🫠 |
|
Oh, I didn't know that you were actually going through and adding comments to these. I'd, probably want to do an actual review in that case. |
| type Cap = core::num::niche_types::UsizeNoHighBit; | ||
|
|
||
| // SAFETY: Untriaged. | ||
| // SAFETY: 0 *definitely* is less than isize::MAX. |
There was a problem hiding this comment.
But how can you be sure?
There was a problem hiding this comment.
easy: isize was defined before 2025 and therefore there were affordable computers around that had to address more than 0 bytes of memory!
| // SAFETY: Precondition passed to caller | ||
| // SAFETY: Untriaged. | ||
| if let Some((ptr, layout)) = unsafe { self.current_memory(elem_layout) } { | ||
| // SAFETY: Untriaged. | ||
| // SAFETY: Precondition passed to caller |
There was a problem hiding this comment.
This one looked painful to figure out
| fn into_inner_with_allocator(this: Self) -> (NonNull<ArcInner<T>>, A) { | ||
| let this = mem::ManuallyDrop::new(this); | ||
| // SAFETY: Untriaged. | ||
| // SAFETY: Pointer is valid for reads and won't be double-dropped. |
There was a problem hiding this comment.
YORO? (You only read once.)
|
Could you put a |
|
|
|
that should address review comments 🫠 |
This comment has been minimized.
This comment has been minimized.
| @@ -1403,6 +1436,7 @@ impl<T, A: Allocator> VecDeque<T, A> { | |||
| } else { | |||
| // Either there's not enough spare capacity to make the deque contiguous, or the head is shorter than the tail | |||
| // (and therefore hopefully cheaper to copy). | |||
| // SAFETY: Untriaged. | |||
There was a problem hiding this comment.
same, comment already in the unsafe block
There was a problem hiding this comment.
this one doesn't justify the access, since copy has missing safety docs. i assume the safety requirement is that the sizes are inbounds, but...
| @@ -2682,6 +2695,7 @@ impl<T, A: Allocator> Vec<T, A> { | |||
| let mut first_duplicate_idx: usize = 1; | |||
| let start = self.as_mut_ptr(); | |||
| while first_duplicate_idx != len { | |||
| // SAFETY: Untriaged. | |||
There was a problem hiding this comment.
there is a safety comment within the unsafe block that can be used
There was a problem hiding this comment.
it applies to the first unsafe op, but i'm unconvinced for the other 2. either way, good to split this up, nice catch ^^
This comment has been minimized.
This comment has been minimized.
it seems my children have come to haunt me |
0ad4d08 to
0c8e275
Compare
|
This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
|
☔ The latest upstream changes (presumably #161075) made this pull request unmergeable. Please resolve the merge conflicts by rebasing. |
7 participants
View all comments
cc @clarfonthey, @workingjubilee. Following up from #160824. I spent a painfully long time going over the comments and ensuring there's nothing too awful ^^
r? clarfonthey