Partially stabilize ptr_alignment_type as alignment_type

[GrigorenkoPV]

View all comments

Tracking issue: #102070

Stabilized API

All used to be gated under ptr_alignment_type, now stabilized under alignment_type:

// in `core::mem`

#[derive(Copy, Clone, PartialEq, Eq)]
pub struct Alignment { ... }

impl Alignment {
    pub const MIN: Self;
    pub const fn of<T>() -> Self;
    pub const fn of_val<T: MetaSized>(val: &T) -> Self;
    pub const fn new(align: usize) -> Option<Self>;
    pub const unsafe fn new_unchecked(align: usize) -> Self;
    pub const fn as_usize(self) -> usize;
    pub const fn as_nonzero_usize(self) -> NonZero<usize>;
}

impl Debug for Alignment;
impl TryFrom<NonZero<usize>> for Alignment;
impl TryFrom<usize> for Alignment;
impl From<Alignment> for NonZero<usize>;
impl From<Alignment> for usize;
impl Ord for Alignment;
impl PartialOrd for Alignment;
impl Hash for Alignment;
impl Default for Alignment;

Things not getting stabilized

Remains under ptr_alignment_type:

// in core::ptr
#[deprecated(note = "moved from `ptr` to `mem`")]
pub type Alignment = mem::Alignment;

// in `core::mem`
impl Alignment {
    pub const fn log2(self) -> u32;
    
    #[deprecated(note = "renamed to `as_nonzero_usize`", suggestion = "as_nonzero_usize" )]
    pub const fn as_nonzero(self) -> NonZero<usize> {
        self.as_nonzero_usize()
    }
}

// in `core::layout`
impl Layout {
    // `pub const fn from_size_align(size: usize, align: usize) -> Result<Self, LayoutError>`'s sibling
    pub const fn from_size_alignment(size: usize, alignment: Alignment) -> Result<Self, LayoutError>;
    // `pub const unsafe fn from_size_align_unchecked(size: usize, align: usize) -> Self`'s sibling
    pub const unsafe fn from_size_alignment_unchecked(size: usize, alignment: Alignment) -> Self;
    // `pub const fn align(&self) -> usize`'s sibling
    pub const fn alignment(&self) -> Alignment;
    // `pub const fn align_to(&self, align: usize) -> Result<Self, LayoutError>`'s sibling
    pub const fn adjust_alignment_to(&self, alignment: Alignment) -> Result<Self, LayoutError>;
    // no `align: usize` sibling
    pub const fn padding_needed_for(&self, alignment: Alignment) -> usize;
}

Moved from ptr_alignment_type to layout_for_ptr (#69835):

impl Alignment {
    pub const unsafe fn of_val_raw<T: MetaSized>(val: *const T) -> Self;
}

Moved from ptr_alignment_type to ptr_mask (#98290):

impl Alignment
    pub const fn mask(self) -> usize;
}

const trait impls

These are stabilized as non-const (see "Stabilized APIs"), but const counterparts remain unstable under the same feature gates:

under const_convert (#143773)

impl const TryFrom<NonZero<usize>> for Alignment;
impl const TryFrom<usize> for Alignment;
impl const From<Alignment> for NonZero<usize>;
impl const From<Alignment> for usize;

under const_default (#143894).

impl const Default for Alignment;

under const_cmp (#143800).

impl const PartialOrd for Alignment;
impl const Ord for Alignment;
impl const PartialEq for Alignment;
impl const Eq for Alignment;

under const_clone (#142757).

impl const Clone for Alignment;

[rustbot]

r? @Mark-Simulacrum

rustbot has assigned @Mark-Simulacrum.
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: @scottmcm, libs
• @scottmcm, libs expanded to 8 candidates
• Random selection from Mark-Simulacrum, joboet, scottmcm

[GrigorenkoPV]

For reasons behind what exactly is being stabilized, why moved from core::ptr to core::mem, and why renamed from as_nonzero to as_nonzero_usize see the discussion on the tracking issue starting with this comment:
#102070 (comment)

@rustbot label -T-libs +T-libs-api +I-libs-api-nominated

[GrigorenkoPV]

This currently fails due to bumpalo having an internal padding_needed_for method in its UnstableLayoutMethods trait, but their method accepts an usize instead of Alignment.

So we either have to change our name, patch bumpalo locally, or convince bumpalo to change their and wait until this repo updates to a newer version of bumpalo.

[clarfonthey]

So, part of the reason why I suggested just stabilising the type instead of, for example, the Layout methods is so that it could be used in new stable APIs without worrying about extra potential conflicts like that. I would argue that the bumpalo conflict is a compelling argument to hold off on stabilising those for now.

[GrigorenkoPV]

Good argument, I've updated the PR and the description accordingly.

[clarfonthey]

Side note: it's kind of bad form to stabilise stuff under one flag while keeping other methods unstable. You can just name the stuff left out as ptr_alignment_type_extras or something, or if you want to make things a bit clearer:

• log2: alignment_type_extras
• Layout methods: layout_alignment_methods

(ptr_alignment doesn't make sense any more if this is moved to mem::Alignment)

---

Rereading the PR, you did already rename the feature flags, so, my bad. You can do this suggestion to further separate the names if you want, but it's not actually as necessary as I thought it was, since the stable/unstable features are different.

[nia-e]

This was discussed in today's @rust-lang/libs-api meeting and we concluded that we'd like to see the methods on Layout stabilised also. There seemed to be some discussion in the tracking issue regarding which module to place everything in, but there was consensus on everything living in core::mem.

[tgross35]

Could the renames and moves be split to a separate PR from stabilization? There is a lot going on here at once.

We also don't need to add deprecated methods/aliases IMO. Users of unstable API need to be aware that there will occasionally be breaking changes, we shouldn't take on the extra burden of trying to smooth this except in rare cases.

[scottmcm]

(Came here from the meeting notes) I'd proposed the partial here, @nia-e, because I figured some pieces of this were "obvious" (assuming we want the type at all) like new+new_unchecked+as_nonzero, at least under some spelling*, but things like Alignment::mask or Layout::padding_needed_for are currently under the same feature gate and not as obviously necessary as part of the original stabilization.

I'm perfectly happy to see more stabilized if folks are happy with it :)

[scottmcm]

Oh, and +💯 to separating out the move from any stabilizations. That can be landed easily without FCP, but with the mir-opt implications and such it's noisy so valuable to have separate from the feature gating changes.

[Amanieu]

@rfcbot merge libs-api

[rust-rfcbot]

@Amanieu has proposed to merge this. The next step is review by the rest of the tagged team members:

• @Amanieu
• @BurntSushi
• @dtolnay
• @joshtriplett
• @the8472

Concerns:

• non_zero_name_consistency (Partially stabilize ptr_alignment_type as alignment_type #153261 (comment))

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

[scottmcm]

suggestion: maybe just delete the old name as part of this PR?

[GrigorenkoPV]

Sure, given that when this PR lands, there will have been quite some time since the deprecation of the unstable names, I think it's fine to remove them as part of this PR.

[rustbot]

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.

[BurntSushi]

In #157226, we seem poised to stabilize on non_null naming. But here, we have nonzero. I don't think we have any stable methods containing non_null, nonnull, non_zero or nonzero. So we have a chance here to pick a precedent and stick to it. I think the underscore is most consistent. i.e., go with what's in #157226 and rename nonzero here to non_zero.

@rfcbot concern non_zero_name_consistency

[theemathas]

@BurntSushi See #154237 (comment)

[BurntSushi]

@theemathas Thank you! I hadn't seen that. I'm not really convinced by that though on its own. As of now, we're headed in a direction where we have nonzero and non_null, which feels oddly inconsistent given the type names of NonZero and NonNull, respectively.

(To me it's not about the as_<type name> consistency. I'm on board with that not being a hard rule or whatever. It's more about being consistent with how as_<whatever> is spelled. And in more contexts than just as_.)