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,
}

[TimDiekmann]

Unspecified is a poor name because that's not used anywhere else in Rust. Just say Uninitialized.

Fair point. Additionally, AllocInit::Uninitialized feels fluently, and it's possible to use core::alloc::{AllocInit::*, ReAllocPlacement::*};, which might comes in handy when using the API

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).

I'd use ReallocPlacement as other allocation APIs also uses the term realloc, not re_alloc.

The comments on the placement variants are kinda fuzzy and could probably be improved.

Thanks, sounds better!

The new address of the memory can be any heap location.

Is that true? Is this necessarily a heap location? I can imagine, that this could also be a global buffer or something, which has been allocated on the stack.