Remove `_zeroed` variants and pass a flag instead

[TimDiekmann]

Currently, for almost all methods (except dealloc and shrink(_in_place)) in AllocRef, there is a _zeroed variant, which blows up the trait.

I propose to remove those variants and add a zeroed: bool to the signatures. E.g.:

fn alloc(&self, layout: Layout, zeroed: bool) -> Result<...>;
fn realloc(&self, ptr: NonNull<u8>, layout: Layout, new_size: usize, zeroed: bool) -> Result<...>;

[Amanieu]

I would prefer an enum over a bool: http://blakesmith.me/2019/05/07/rust-patterns-enums-instead-of-booleans.html

Otherwise this seems like a good idea.

Do we want to do something similar for grow/grow_in_place and shrink/shrink_in_place?

[TimDiekmann]

I'm aware of that post, but I wasn't able to find a good enum name so far.

Maybe something like

enum ??? {
    Zeroed,
    Unspecified
}

Yes for grow/grow_in_place, for shrink/shrink_in_place it doesn't make sense, as no new memory is returned.

[Amanieu]

I mean having an InPlace flag instead of having two separate methods.

[TimDiekmann]

Oh, yeah, that's also a good idea. So basically like grow and shrink in rust-lang/rust#69824? 😄
https://github.com/rust-lang/rust/blob/32c0b1761d0cbef051408c6c28f78302935d8188/src/liballoc/raw_vec.rs#L465

[Amanieu]

Tentative names:

enum AllocContents {
    Zero,
    Uninitialized, // With a big warning that reading this is UB if you haven't initialized it
}

enum AllocConstraints {
    None,
    InPlace,
}

Note that I am less sure about the InPlace thing, so we might want to defer on that for now.

[Lokathor]

I'd call it NewMemory, because a realloc doesn't also zeroed the previous data.

[TimDiekmann]

As this will be in core::alloc::, I'd omit the Alloc prefix.
I just came up with enum Init { Zero, Uninitialized/Unspecified }. enum Constraint sounds like a bitmask with the abillity to add more variants later.

[Lokathor]

Names should generally make sense on their own, without knowing the module path to the item.

[TimDiekmann]

Can we agree on this?

/// Passed to an allocating method to determine how newly requested memory is 
/// initialized.
#[derive(Debug, Copy, Clone, PartialEq, Eq)]
enum AllocInit {
    /// Depending on the used allocator, newly allocated memory is not 
    /// necessarily initialized. Reading uninitialized memory is 
    /// [undefined behavior]!
    ///
    /// [undefined behavior]: https://doc.rust-lang.org/reference/behavior-considered-undefined.html
    Unspecified,
    /// Newly allocated memory returned by an allocator is guaranteed to be 
    /// zeroed.
    Zero,
}

/// Passed to an allocator to determine how newly requested memory is 
/// allocated when growing or shrinking an existing memory block.
#[derive(Debug, Copy, Clone, PartialEq, Eq)]
enum AllocPlacement {
    /// The location is not specified. The allocator is allowed to make a 
    /// byte-by-byte copy of the memory.
    Unspecified,
    /// When allocating or releasing memory, the contents of the memory must not 
    /// be copied.
    InPlace,
}

[Wodann]

Sounds good to me. Nice work on the RawVec PR too 👏🏻

[TimDiekmann]

Thank you!

[Lokathor]

I feel, dubious about those definitions.

• Unspecified is a poor name because that's not used anywhere else in Rust. Just say Uninitialized.
• Zero should be changed to Zeroed, so that the tag names are both adjectives.
• AllocPlacement only makes sense with realloc, so we can call it ReAllocPlacement (bikeshed the capitalization there).
• The comments on the placement variants are kinda fuzzy and could probably be improved.

Consider this revision:

/// A desired initial state for allocated memory.
#[derive(Debug, Copy, Clone, PartialEq, Eq)]
enum AllocInit {
    /// The new memory is not initialized to any particular value.
    /// 
    /// Remember that reading uninitialized memory is Undefined Behavior.
    Uninitialized,
    /// The new memory is guaranteed to be zeroed.
    Zeroed,
}

/// A reallocation constraint.
#[derive(Debug, Copy, Clone, PartialEq, Eq)]
enum ReAllocPlacement {
    /// The new address of the memory can be any heap location.
    ///
    /// If the allocation _does_ move, it's the responsibility of the allocator
    /// to also move the data from the previous location to the new location.
    Unspecified,
    /// The address of the new memory must not change.
    ///
    /// If the allocation would have to be moved to a new location to fit, the
    /// reallocation request should instead fail.
    InPlace,
}