Use MaybeUninit in liballoc #54924
Use MaybeUninit in liballoc #54924
Conversation
|
(rust_highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
|
@bors try |
|
☀️ Test successful - status-travis |
|
@rust-timer build 1102565 |
|
Success: Queued 1102565 with parent b1a137d, comparison URL. |
|
Finished benchmarking try commit 1102565 |
|
Looking good, doesn't it? r? @cramertj |
| @@ -73,7 +73,7 @@ struct LeafNode<K, V> { | |||
| /// This node's index into the parent node's `edges` array. | |||
| /// `*node.parent.edges[node.parent_idx]` should be the same thing as `node`. | |||
| /// This is only guaranteed to be initialized when `parent` is nonnull. | |||
| parent_idx: u16, | |||
| parent_idx: MaybeUninit<u16>, | |||
|
|
|||
| /// The number of keys and values this node stores. | |||
| /// | |||
There was a problem hiding this comment.
Can you confirm that despite introducing MaybeUninit we are still the same size? (i.e., same 32-bit word layout as indicated by the comment on len)?
There was a problem hiding this comment.
MaybeUninit won't introduce any additional alignment requirements that aren't present in the type it wraps, and will only increase the size of the resulting type as a result of inhibiting niche-filling optimizations (and u16 has not niches, so this shouldn't be an issue).
There was a problem hiding this comment.
What would be a good way to test this? We have a way to "hack" a static_assert!, but the size alone won't do it here because there's a ptr right before this, so on 64bit system it'll be 2 ptr-words in size at least. And we cannot yet do an offset! macro in CTFE. I could add a debug_assert! to test the offset at run-time?
However, I agree with what @cramertj said and this is repr(C), so I am not expecting any surprises. Still, would be nice to be sure.
|
r=me with @Mark-Simulacrum 's doc nit fixed. |
|
r+'ing as per previous review. @bors r=cramertj |
|
📌 Commit e4434be has been approved by |
bors
added
S-waiting-on-bors
|
☀️ Test successful - status-appveyor, status-travis |
All code by @japaric. This is a re-submission of a part of #53508 that hopefully does not regress performance.