Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make use of [wrapping_]byte_{add,sub} #100819

Merged
merged 1 commit into from
Aug 29, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion compiler/rustc_arena/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#![feature(maybe_uninit_slice)]
#![feature(min_specialization)]
#![feature(decl_macro)]
#![feature(pointer_byte_offsets)]
#![feature(rustc_attrs)]
#![cfg_attr(test, feature(test))]
#![feature(strict_provenance)]
Expand Down Expand Up @@ -211,7 +212,7 @@ impl<T> TypedArena<T> {

unsafe {
if mem::size_of::<T>() == 0 {
self.ptr.set((self.ptr.get() as *mut u8).wrapping_offset(1) as *mut T);
self.ptr.set(self.ptr.get().wrapping_byte_add(1));
let ptr = ptr::NonNull::<T>::dangling().as_ptr();
// Don't drop the object. This `write` is equivalent to `forget`.
ptr::write(ptr, object);
Expand Down
13 changes: 5 additions & 8 deletions library/alloc/src/vec/into_iter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ use crate::alloc::{Allocator, Global};
use crate::raw_vec::RawVec;
use core::array;
use core::fmt;
use core::intrinsics::arith_offset;
use core::iter::{
FusedIterator, InPlaceIterable, SourceIter, TrustedLen, TrustedRandomAccessNoCoerce,
};
Expand Down Expand Up @@ -154,7 +153,7 @@ impl<T, A: Allocator> Iterator for IntoIter<T, A> {
// purposefully don't use 'ptr.offset' because for
// vectors with 0-size elements this would return the
// same pointer.
self.ptr = unsafe { arith_offset(self.ptr as *const i8, 1) as *mut T };
self.ptr = self.ptr.wrapping_byte_add(1);

// Make up a value of this ZST.
Some(unsafe { mem::zeroed() })
Expand Down Expand Up @@ -184,7 +183,7 @@ impl<T, A: Allocator> Iterator for IntoIter<T, A> {
// SAFETY: due to unchecked casts of unsigned amounts to signed offsets the wraparound
// effectively results in unsigned pointers representing positions 0..usize::MAX,
// which is valid for ZSTs.
self.ptr = unsafe { arith_offset(self.ptr as *const i8, step_size as isize) as *mut T }
self.ptr = self.ptr.wrapping_byte_add(step_size);
} else {
// SAFETY: the min() above ensures that step_size is in bounds
self.ptr = unsafe { self.ptr.add(step_size) };
Expand Down Expand Up @@ -217,7 +216,7 @@ impl<T, A: Allocator> Iterator for IntoIter<T, A> {
return Err(unsafe { array::IntoIter::new_unchecked(raw_ary, 0..len) });
}

self.ptr = unsafe { arith_offset(self.ptr as *const i8, N as isize) as *mut T };
self.ptr = self.ptr.wrapping_byte_add(N);
// Safety: ditto
return Ok(unsafe { MaybeUninit::array_assume_init(raw_ary) });
}
Expand Down Expand Up @@ -267,7 +266,7 @@ impl<T, A: Allocator> DoubleEndedIterator for IntoIter<T, A> {
None
} else if mem::size_of::<T>() == 0 {
// See above for why 'ptr.offset' isn't used
self.end = unsafe { arith_offset(self.end as *const i8, -1) as *mut T };
self.end = self.ptr.wrapping_byte_sub(1);
Comment on lines -270 to +269
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this right? It's using self.ptr instead of self.end

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed this is wrong; should be fixed by #101237.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm sorry 🥲


// Make up a value of this ZST.
Some(unsafe { mem::zeroed() })
Expand All @@ -283,9 +282,7 @@ impl<T, A: Allocator> DoubleEndedIterator for IntoIter<T, A> {
let step_size = self.len().min(n);
if mem::size_of::<T>() == 0 {
// SAFETY: same as for advance_by()
self.end = unsafe {
arith_offset(self.end as *const i8, step_size.wrapping_neg() as isize) as *mut T
}
self.end = self.end.wrapping_byte_sub(step_size);
} else {
// SAFETY: same as for advance_by()
self.end = unsafe { self.end.sub(step_size) };
Expand Down
4 changes: 2 additions & 2 deletions library/alloc/src/vec/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ use core::cmp::Ordering;
use core::convert::TryFrom;
use core::fmt;
use core::hash::{Hash, Hasher};
use core::intrinsics::{arith_offset, assume};
use core::intrinsics::assume;
use core::iter;
#[cfg(not(no_global_oom_handling))]
use core::iter::FromIterator;
Expand Down Expand Up @@ -2678,7 +2678,7 @@ impl<T, A: Allocator> IntoIterator for Vec<T, A> {
let alloc = ManuallyDrop::new(ptr::read(me.allocator()));
let begin = me.as_mut_ptr();
let end = if mem::size_of::<T>() == 0 {
arith_offset(begin as *const i8, me.len() as isize) as *const T
begin.wrapping_byte_add(me.len())
} else {
begin.add(me.len()) as *const T
};
Expand Down
2 changes: 1 addition & 1 deletion library/core/src/ptr/const_ptr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,7 @@ impl<T: ?Sized> *const T {
let offset = dest_addr.wrapping_sub(self_addr);

// This is the canonical desugarring of this operation
self.cast::<u8>().wrapping_offset(offset).cast::<T>()
self.wrapping_byte_offset(offset)
}

/// Creates a new pointer by mapping `self`'s address to a new one.
Expand Down
2 changes: 1 addition & 1 deletion library/core/src/ptr/mut_ptr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,7 @@ impl<T: ?Sized> *mut T {
let offset = dest_addr.wrapping_sub(self_addr);

// This is the canonical desugarring of this operation
self.cast::<u8>().wrapping_offset(offset).cast::<T>()
self.wrapping_byte_offset(offset)
}

/// Creates a new pointer by mapping `self`'s address to a new one.
Expand Down
2 changes: 1 addition & 1 deletion library/core/src/slice/iter/macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ macro_rules! iterator {
// backwards by `n`. `n` must not exceed `self.len()`.
macro_rules! zst_shrink {
($self: ident, $n: ident) => {
$self.end = ($self.end as * $raw_mut u8).wrapping_offset(-$n) as * $raw_mut T;
$self.end = $self.end.wrapping_byte_offset(-$n);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is fine, but I definitely look at it and wonder if it can be

Suggested change
$self.end = $self.end.wrapping_byte_offset(-$n);
$self.end = $self.end.wrapping_byte_sub($n);

since it's explicitly shrink.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't done this originally because ($n): isize, but I've just checked and it seems that this is only called with literals and smt as isize so this change makes sense.

}
}

Expand Down
2 changes: 1 addition & 1 deletion library/core/tests/atomic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ fn ptr_add_data() {

assert_eq!(atom.fetch_ptr_sub(1, SeqCst), n.wrapping_add(1));
assert_eq!(atom.load(SeqCst), n);
let bytes_from_n = |b| n.cast::<u8>().wrapping_add(b).cast::<i64>();
let bytes_from_n = |b| n.wrapping_byte_add(b);

assert_eq!(atom.fetch_byte_add(1, SeqCst), n);
assert_eq!(atom.load(SeqCst), bytes_from_n(1));
Expand Down
2 changes: 1 addition & 1 deletion library/core/tests/ptr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -650,7 +650,7 @@ fn thin_box() {
.unwrap_or_else(|| handle_alloc_error(layout))
.cast::<DynMetadata<T>>();
ptr.as_ptr().write(meta);
ptr.cast::<u8>().as_ptr().add(offset).cast::<Value>().write(value);
ptr.as_ptr().byte_add(offset).cast::<Value>().write(value);
Self { ptr, phantom: PhantomData }
}
}
Expand Down
4 changes: 2 additions & 2 deletions library/std/src/io/error/repr_bitpacked.rs
Original file line number Diff line number Diff line change
Expand Up @@ -269,10 +269,10 @@ where
}
TAG_SIMPLE_MESSAGE => ErrorData::SimpleMessage(&*ptr.cast::<SimpleMessage>().as_ptr()),
TAG_CUSTOM => {
// It would be correct for us to use `ptr::sub` here (see the
// It would be correct for us to use `ptr::byte_sub` here (see the
// comment above the `wrapping_add` call in `new_custom` for why),
// but it isn't clear that it makes a difference, so we don't.
let custom = ptr.as_ptr().cast::<u8>().wrapping_sub(TAG_CUSTOM).cast::<Custom>();
let custom = ptr.as_ptr().wrapping_byte_sub(TAG_CUSTOM).cast::<Custom>();
ErrorData::Custom(make_custom(custom))
}
_ => {
Expand Down
1 change: 1 addition & 0 deletions library/std/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -296,6 +296,7 @@
#![feature(panic_can_unwind)]
#![feature(panic_info_message)]
#![feature(panic_internals)]
#![feature(pointer_byte_offsets)]
#![feature(pointer_is_aligned)]
#![feature(portable_simd)]
#![feature(prelude_2024)]
Expand Down