Skip to content

core: generalize BorrowedCursor::ensure_init - #160432

Open
joboet wants to merge 2 commits into
rust-lang:mainfrom
joboet:ensure_init_generic
Open

core: generalize BorrowedCursor::ensure_init#160432
joboet wants to merge 2 commits into
rust-lang:mainfrom
joboet:ensure_init_generic

Conversation

@joboet

@joboet joboet commented Aug 3, 2026

Copy link
Copy Markdown
Member

Tracking issue: #160476

This implements the future possibility left out in #149749 (comment) and makes ensure_init generic over Default. I used specialisation to make sure the performance in the u8 case stays equivalent to memset irrespective of how clever the optimiser happens to be.

CC @joshtriplett as you've been working on this stuff recently.

This also adds a public write_default method on [MaybeUninit<T>] that's doing the equivalent of .write_init(|_| Default::default()), but uses specialisation for integers. This uses the existing tracking issue for maybe_uninit_fill.

@rustbot 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 Aug 3, 2026
@rustbot

rustbot commented Aug 3, 2026

Copy link
Copy Markdown
Collaborator

r? @clarfonthey

rustbot has assigned @clarfonthey.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

  • Owners of files modified in this PR: libs
  • libs expanded to 12 candidates
  • Random selection from 6 candidates

@joboet joboet added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Aug 3, 2026
Comment thread library/core/src/io/borrowed_buf.rs Outdated

impl<T: Default + Copy> InitSpec for T {
default fn initialize(buf: &mut [MaybeUninit<Self>]) {
buf.write_with(|_| Self::default());

@clarfonthey clarfonthey Aug 3, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just looking at write_with, perhaps it would be nice to just have a general write_default that we could override for all slices, rather than just this type, so it could be reused elsewhere.

If you'd be okay doing that instead, adding an unstable/private method to slices instead?

View changes since the review

@joboet joboet Aug 4, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That seems very useful. I went ahead and added it publicly under the existing maybe_uninit_fill tracking issue since it's probably so uncontroversial.

Comment thread library/core/src/io/borrowed_buf.rs Outdated
@rust-bors

This comment has been minimized.

@joboet
joboet force-pushed the ensure_init_generic branch from 5135717 to 0100285 Compare August 4, 2026 14:38
@rustbot

rustbot commented Aug 4, 2026

Copy link
Copy Markdown
Collaborator

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.

@joboet

joboet commented Aug 4, 2026

Copy link
Copy Markdown
Member Author

CC @rust-lang/libs-api for the write_default addition, just in case.

@nia-e

nia-e commented Aug 4, 2026

Copy link
Copy Markdown
Member

sure! we're happy with this addition. thanks ^^

@joshtriplett

Copy link
Copy Markdown
Member

Seems reasonable to me! We'd left ensure_init un-generalized because generalizing it would require specialization for performance, but since you've done that specialization, 👍.

where
T: Default,
{
trait DefaultSpec: Default {

@clarfonthey clarfonthey Aug 4, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This feels close to something that exists for Vec but maybe I'm misremembering. Either way it might be worth not nesting in the function in case it becomes useful elsewhere. Kinda want to avoid the duplication if we can avoid it

View changes since the review

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are probably thinking of IsZero, which Vec::from_elem uses to check if it can use zeroed allocation. In this case however we don't have an existing value whose zeroness we could check but rather want to test a property of the Default implementation.

I guess I could add a marker trait if you'd like. But I'm sort of tempted to leave all that complexity for when someone complains...

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nah, you're right, this is pretty unique. I'm down for that plan; I was just trying to remember if we had code somewhere that was similar enough.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants