add the align(ptr) charter

[Lokathor]

This is a charter for #35

Closes #35

RENDERED

[nikomatsakis]

Makes sense! Relatively simple.

@rfcbot fcp merge

[rfcbot]

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

• @cramertj
• @joshtriplett
• @nikomatsakis
• @pnkfelix
• @scottmcm
• @withoutboats

Concerns:

• coordinate-with-const-generics (add the align(ptr) charter #43 (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.

[jswrenn]

Why is AtomicPtr defined like this?

UnsafeCell is #[repr(transparent)] and AtomicPtr is #[repr(C)]; my understanding is that the minimum alignment of AtomicPtr therefore must be equal to the minimum alignment of *mut _ .

Unless there's a platform in which the minimum alignment of AtomicPtr must be greater than the minimum alignment of *mut _, the cfg_attrs and align(*) modifiers seem redundant: AtomicPtr could be annotated with just #[repr(C)].

Am I missing something?

[Lokathor]

I don't think you're missing anything.

[rfcbot]

🔔 This is now entering its final comment period, as per the review above. 🔔

psst @nikomatsakis, I wasn't able to add the final-comment-period label, please do so.

[rfcbot]

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

The RFC will be merged soon.

psst @nikomatsakis, I wasn't able to add the finished-final-comment-period label, please do so.

[comex]

Hmm… a bit too late now, but I hadn’t noticed that this charter proposal had evolved to define “the general concept of const expressions within an align attribute” as out of scope. It’s not the end of the world, but I’m disappointed: it adds unnecessary complexity to the language (a magic ptr attribute value when align_of::<*const u8>() would do) just to save time compared to the more general solution, which is not all that complex to specify (though perhaps harder to implement).

It would also add an edge case if align is ever extended to support arbitrary const expressions in the future: there would be ambiguity in case there happened to be a const ptr: usize in scope.

[Lokathor]

I mean, you're right.

And another thing is that the open conversation above where it was pointed out that all the align stuff is probably pointless since it'll have the correct alignment anyway.

So honestly, this particular project group might be a dud.

Then again, if the conclusion of the group is "nothing that's in scope for us turned out to be a good idea", then that's ultimately an okay conclusion.

[joshtriplett]

@rfcbot concern coordinate-with-const-generics

Thanks for suggesting that, @comex!

We discussed this in the meeting today, and we should coordinate this with the const eval folks, to figure out if this could use const evaluation so that #[repr(align(align_of::<*const u8>()))] could work.

We might not need const generic support quite yet, though that might be useful at some point.

[scottmcm]

FWIW, align(size) in particular feels like something that would be very hard to write using a const generics expression, so there may be value in some "canned" values here even if the general thing is also supported.

[Lokathor]

Zulip topic created to check on the feasibility of general const expression usage: https://rust-lang.zulipchat.com/#narrow/stream/146212-t-compiler.2Fconst-eval/topic/const.20eval.20within.20attributes

[jswrenn]

FWIW, align(size) in particular feels like something that would be very hard to write using a const generics expression, so there may be value in some "canned" values here even if the general thing is also supported.

align(size) actually isn't too hard to whip up with const generics: https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=66801018f8aa2447b41d4c0099bd1d88

(I don't think there's a satisfactory, analogous trick for packed(N), unfortunately.)

[jswrenn]

The align modifier only sets a minimum alignment. Oddly, Rust doesn't have an ergonomic way to specify the alignment of a type is exactly equal to a value.

You can set a maximum alignment with the packed modifier, but it's an error to have both align and packed annotations on the same type, even if they're not contradictory:

error[E0587]: type has conflicting packed and align representation hints
 --> src/main.rs:1:30
  |
1 | #[repr(align(1), packed(2))] pub struct Foo;
  |                              ^^^^^^^^^^^^^^^

[scottmcm]

align(size) actually isn't too hard to whip up with const generics

I think it was unclear what I meant by align(size) here. Specifically, I've been using that to mean alignof(T)==sizeof(T), by adding just enough padding to round the total size of the struct up to a power of two. And doing that in a const fn would, I think, require reimplementing the repr(rust) size calculation over all the fields. Which is doable eventually (once size_of and math work in const generics), but not trivial.

[jswrenn]

Surprisingly, size_of already works!?

Using my demo, you could implement what you describe by desugaring this:

#[repr(C, align(size))]
struct Foo {
  bar: u8,
  baz: u16,
}

...to this:

#[repr(C)]
struct Foo {
  _align: Align<{
    #[repr(C)]
    struct Foo {
      bar: u8,
      baz: u8,
    }
    if let Some(align) =  mem::size_of::<Foo>().checked_next_power_of_two() {
      align
    } else {
      usize::MAX
    }
  }>,
  bar: u8,
  baz: u16,
}

[joshtriplett]

We discussed this in the lang team meeting, and we'd like to charter this group, but we believe based on the linked discussions that this should be doable with simple const expressions. @Lokathor proposed to update the charter to account for this. We'll wait for that updated charter, then merge.

[nikomatsakis]

@joshtriplett can you resolve your outstanding concern?

[nikomatsakis]

or are you waiting for the final edits

[Lokathor]

I was going to update the charter based on feedback about const eval in the attribute usage and we were going to discuss again. Hopefully I can do that this weekend.

[Lokathor]

I no longer have time to work on this, particularly if the solution is going to potentially be more complicated.

Discussed at the meeting Oct 5, 2020.

• We decided to close this as "postponed" since no one else currently has the bandwidth either.

Closes #35