NonEmptyLayout

[clarfonthey]

Notes:

1. We can bikeshed NonEmptyLayout versus NonZeroLayout versus NonZero<Layout> later.
2. It might be nice to make Layout and NonEmptyLayout explicitly repr(C) or something to allow literally transmuting from Layout to Option<NonEmptyLayout>. I didn't do this here because I know that repr(C) might cause other undesirable side effects.
3. Right now, new::<T>() uses a const assertion to verify that T is non-zero-sized but new_for(&val) returns Option, with an unchecked variant. Is this the right approach?
4. Actually making some kind of allocator trait that works on top of this, and try to use it in Box and Vec to avoid the ZST special-casing on allocations specifically (note: there will still be types that need special-casing, like slice::Iter)
5. Technically adding new API without ACP, but since we've effectively talked about this in wg-allocators and it feels like a reasonable ask, I'm just going to create an MVP of this type and we can work out more details later.

r? nia-e (but I guess you can pass it to someone else if you want)

[rustbot]

nia-e is currently at their maximum review capacity.
They may take a while to respond.

[nia-e]

Was there an ACP for this? It seems like a decently big thing if we ever want to stabilise it (understandable if we want to keep it internal though, i see the rationale there).

in particular regarding extending Allocator, iirc the conclusion of the allocator layout discussion was that there are almost no cases where handling ZSTs isn't trivial on the allocator end.

i could see the case for extra provided methods on the base trait to use this. little worried about vtable size there but would need a benchmark

[clarfonthey]

There was explicitly no ACP for this, since I mostly just wanted to see how not-awful this would be to implement. In terms of actually making allocators that use it, we definitely wanna have some kind of ACP, since the motivation here is basically in this thread: #t-libs/wg-allocators > Are we sure about zero-sized allocations?

Essentially, it might be useful to define specifically the exact behaviour of Vec and Box in a way that can be replicated across APIs.

[rust-log-analyzer]

The job test-pr-check-2 failed! Check out the build log: (web) (plain enhanced) (plain)

Click to see the possible cause of the failure (guessed by this bot)

[programmerjake]

truncated sentence

View changes since the review

[RustyYato]

This can be made branchless via

godbolt

pub const fn is_size_align_valid(size: usize, align: usize) -> bool {
    let Some(alignment) = Alignment::new(align) else { return false };
    Self::is_nonzero_size_alignment_valid(size, alignment)
}

const fn is_nonzero_size_alignment_valid(size: usize, alignment: Alignment) -> bool {
    size.wrapping_sub(1) < max_size_for_alignment(alignment)
}

View changes since the review

[RustyYato]

If you want to optimize the case where size is known to be non-zero,

const fn is_nonzero_size_alignment_valid(size: usize, alignment: Alignment) -> bool {
    if is_val_statically_known(size > 0) {
        size <= max_size_for_alignment(alignment)
    } else {
        size.wrapping_sub(1) < max_size_for_alignment(alignment)
    }
}

[RustyYato]

This is super confusing to read, can the if let Some(k) = ... part be factored out into a separate variable?
Also, self.repeat_packed(0) is guaranteed to error, so that branch could just be return Err(LayoutError)

View changes since the review

[RustyYato]

Maybe this instead?

Suggested change

	let Ok(result) = (if let Some(k) = n.checked_sub(1) {
	let Ok(repeated) = padded.repeat_packed(k) else {
	return Err(LayoutError);
	};
	repeated.extend_packed(self.get())
	} else {
	debug_assert!(n == 0);
	self.repeat_packed(0)
	}) else {
	return Err(LayoutError);
	};
	let result = if let Some(k) = n.checked_sub(1) {
	let Ok(repeated) = padded.repeat_packed(k) else {
	return Err(LayoutError);
	};
	let Ok(result) = repeated.extend_packed(self.get()) else {
	return Err(LayoutError);
	};
	result
	} else {
	debug_assert!(n == 0);
	return Err(LayoutError);
	};