feat(data_structures): add Stack::clear

[sapphi-red]

Added Stack::clear for #14029.

[sapphi-red]

• feat(data_structures): add Stack::clear #14028 👈 (View in Graphite)
• main

---

How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

• 0-merge - adds this PR to the back of the merge queue
• hotfix - for urgent hot fixes, skip the queue and merge this PR next

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

This stack of pull requests is managed by Graphite. Learn more about stacking.

[codspeed-hq]

CodSpeed Instrumentation Performance Report

Merging #14028 will not alter performance

_{Comparing 09-23-feat_data_structures_add_stack_clear_ (1521a1f) with main (70bd141)}

Summary

✅ 37 untouched

[overlookmotel]

self.is_empty() check may not actually be required. When the stack's content type (T) is non-Drop, self.drop_contents() becomes a no-op anyway. So if we could remove self.is_empty() check,this function becomes just a single instruction and no branches, when T is non-Drop.

drop_contents has a safety comment saying "Stack must be allocated", but I'm not sure it's actually required. I think it's fine to call ptr::drop_in_place with an empty slice. But let's leave it as is - maybe there was a reason I added this safety constraint which I don't remember right now.

[sapphi-red]

When the stack is not allocated, self.start is a dangling pointer. In that case, the return value of ptr::slice_from_raw_parts_mut(self.start().as_ptr(), self.len()) is not valid. I think that's the reason of the "Stack must be allocated" comment.
This self.is_empty() check indirectly checks that the stack is allocated from the fact that "non-empty stack would always have the stack allocated".

[overlookmotel]

When the stack is not allocated, self.start is a dangling pointer. In that case, the return value of ptr::slice_from_raw_parts_mut(self.start().as_ptr(), self.len()) is not valid. I think that's the reason of the "Stack must be allocated" comment. This self.is_empty() check indirectly checks that the stack is allocated from the fact that "non-empty stack would always have the stack allocated".

Yeah, I think that's what I had in mind at the time I wrote the code. But now I think probably it is legal to create a slice starting at a dangling pointer, as long as it's 0 length (dangling pointers are correctly aligned).

Anyway, it doesn't really matter much. This PR just made me think about it again.

[graphite-app]

Merge activity

• Sep 24, 11:10 AM UTC: sapphi-red added this pull request to the Graphite merge queue.
• Sep 24, 11:16 AM UTC: Merged by the Graphite merge queue.

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

[sapphi-red]

Yeah, I think that's what I had in mind at the time I wrote the code. But now I think probably it is legal to create a slice starting at a dangling pointer, as long as it's 0 length (dangling pointers are correctly aligned).

Ah, I see. I think it's legal. I've just found this:

data must be non-null and aligned even for zero-length slices or slices of ZSTs. One reason for this is that enum layout optimizations may rely on references (including slices of any length) being aligned and non-null to distinguish them from other data. You can obtain a pointer that is usable as data for zero-length slices using NonNull::dangling().
https://doc.rust-lang.org/std/slice/fn.from_raw_parts_mut.html#:~:text=data%20must%20be,NonNull%3A%3Adangling().

...which is what we're doing here.

[overlookmotel]

You can obtain a pointer that is usable as data for zero-length slices using NonNull::dangling()

Nice. I hadn't spotted that in docs before.