Skip to content

Commit

Permalink
Rollup merge of #121201 - RalfJung:align_offset_contract, r=cuviper
Browse files Browse the repository at this point in the history
align_offset, align_to: no longer allow implementations to spuriously fail to align

For a long time, we have allowed `align_offset` to fail to compute a properly aligned offset, and `align_to` to return a smaller-than-maximal "middle slice". This was done to cover the implementation of `align_offset` in const-eval and Miri. See #62420 for more background. For about the same amount of time, this has caused confusion and surprise, where people didn't realize they have to write their code to be defensive against `align_offset` failures.

Another way to put this is: the specification is effectively non-deterministic, and non-determinism is hard to test for -- in particular if the implementation everyone uses to test always produces the same reliable result, and nobody expects it to be non-deterministic to begin with.

With #117840, Miri has stopped making use of this liberty in the spec; it now always behaves like rustc. That only leaves const-eval as potential motivation for this behavior. I do not think this is sufficient motivation. Currently, none of the relevant functions are stably const: `align_offset` is unstably const, `align_to` is not const at all. I propose that if we ever want to make these const-stable, we just accept the fact that they can behave differently at compile-time vs at run-time. This is not the end of the world, and it seems to be much less surprising to programmers than unexpected non-determinism. (Related: rust-lang/rfcs#3352.)

`@thomcc` has repeatedly made it clear that they strongly dislike the non-determinism in align_offset, so I expect they will support this. `@oli-obk,` what do you think? Also, whom else should we involve? The primary team responsible is clearly libs-api, so I will nominate this for them. However, allowing const-evaluated code to behave different from run-time code is t-lang territory. The thing is, this is not stabilizing anything t-lang-worthy immediately, but it still does make a decision we will be bound to: if we accept this change, then
- either `align_offset`/`align_to` can never be called in const fn,
- or we allow compile-time behavior to differ from run-time behavior.

So I will nominate for t-lang as well, with the question being: are you okay with accepting either of these outcomes (without committing to which one, just accepting that it has to be one of them)? This closes the door to "have `align_offset` and `align_to` at compile-time and also always have compile-time behavior match run-time behavior".

Closes #62420
  • Loading branch information
matthiaskrgr authored Mar 8, 2024
2 parents a655e64 + 507583a commit 948d32d
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 16 deletions.
13 changes: 10 additions & 3 deletions library/core/src/ptr/const_ptr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1326,9 +1326,7 @@ impl<T: ?Sized> *const T {
/// `align`.
///
/// If it is not possible to align the pointer, the implementation returns
/// `usize::MAX`. It is permissible for the implementation to *always*
/// return `usize::MAX`. Only your algorithm's performance can depend
/// on getting a usable offset here, not its correctness.
/// `usize::MAX`.
///
/// The offset is expressed in number of `T` elements, and not bytes. The value returned can be
/// used with the `wrapping_add` method.
Expand All @@ -1337,6 +1335,15 @@ impl<T: ?Sized> *const T {
/// beyond the allocation that the pointer points into. It is up to the caller to ensure that
/// the returned offset is correct in all terms other than alignment.
///
/// When this is called during compile-time evaluation (which is unstable), the implementation
/// may return `usize::MAX` in cases where that can never happen at runtime. This is because the
/// actual alignment of pointers is not known yet during compile-time, so an offset with
/// guaranteed alignment can sometimes not be computed. For example, a buffer declared as `[u8;
/// N]` might be allocated at an odd or an even address, but at compile-time this is not yet
/// known, so the execution has to be correct for either choice. It is therefore impossible to
/// find an offset that is guaranteed to be 2-aligned. (This behavior is subject to change, as usual
/// for unstable APIs.)
///
/// # Panics
///
/// The function panics if `align` is not a power-of-two.
Expand Down
13 changes: 10 additions & 3 deletions library/core/src/ptr/mut_ptr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1589,9 +1589,7 @@ impl<T: ?Sized> *mut T {
/// `align`.
///
/// If it is not possible to align the pointer, the implementation returns
/// `usize::MAX`. It is permissible for the implementation to *always*
/// return `usize::MAX`. Only your algorithm's performance can depend
/// on getting a usable offset here, not its correctness.
/// `usize::MAX`.
///
/// The offset is expressed in number of `T` elements, and not bytes. The value returned can be
/// used with the `wrapping_add` method.
Expand All @@ -1600,6 +1598,15 @@ impl<T: ?Sized> *mut T {
/// beyond the allocation that the pointer points into. It is up to the caller to ensure that
/// the returned offset is correct in all terms other than alignment.
///
/// When this is called during compile-time evaluation (which is unstable), the implementation
/// may return `usize::MAX` in cases where that can never happen at runtime. This is because the
/// actual alignment of pointers is not known yet during compile-time, so an offset with
/// guaranteed alignment can sometimes not be computed. For example, a buffer declared as `[u8;
/// N]` might be allocated at an odd or an even address, but at compile-time this is not yet
/// known, so the execution has to be correct for either choice. It is therefore impossible to
/// find an offset that is guaranteed to be 2-aligned. (This behavior is subject to change, as usual
/// for unstable APIs.)
///
/// # Panics
///
/// The function panics if `align` is not a power-of-two.
Expand Down
14 changes: 4 additions & 10 deletions library/core/src/slice/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3786,11 +3786,8 @@ impl<T> [T] {
/// maintained.
///
/// This method splits the slice into three distinct slices: prefix, correctly aligned middle
/// slice of a new type, and the suffix slice. How exactly the slice is split up is not
/// specified; the middle part may be smaller than necessary. However, if this fails to return a
/// maximal middle part, that is because code is running in a context where performance does not
/// matter, such as a sanitizer attempting to find alignment bugs. Regular code running
/// in a default (debug or release) execution *will* return a maximal middle part.
/// slice of a new type, and the suffix slice. The middle part will be as big as possible under
/// the given alignment constraint and element size.
///
/// This method has no purpose when either input element `T` or output element `U` are
/// zero-sized and will return the original slice without splitting anything.
Expand Down Expand Up @@ -3854,11 +3851,8 @@ impl<T> [T] {
/// types is maintained.
///
/// This method splits the slice into three distinct slices: prefix, correctly aligned middle
/// slice of a new type, and the suffix slice. How exactly the slice is split up is not
/// specified; the middle part may be smaller than necessary. However, if this fails to return a
/// maximal middle part, that is because code is running in a context where performance does not
/// matter, such as a sanitizer attempting to find alignment bugs. Regular code running
/// in a default (debug or release) execution *will* return a maximal middle part.
/// slice of a new type, and the suffix slice. The middle part will be as big as possible under
/// the given alignment constraint and element size.
///
/// This method has no purpose when either input element `T` or output element `U` are
/// zero-sized and will return the original slice without splitting anything.
Expand Down

0 comments on commit 948d32d

Please sign in to comment.