Skip to content

Commit 32d963e

Browse files
SimonSapinSimonSapin
committed
Rewrite mixed-width mixed-signedness integer arithmetic in Step impls
1 parent 5334226 commit 32d963e

File tree

1 file changed

+153
-104
lines changed

1 file changed

+153
-104
lines changed

‎src/libcore/iter/range.rs

Lines changed: 153 additions & 104 deletions
Original file line numberDiff line numberDiff line change
@@ -47,133 +47,182 @@ pub trait Step: Clone + PartialOrd + Sized {
4747
fn backward(&self, step_count: usize) -> Option<Self>;
4848
}
4949

50-
macro_rules! step_impl_unsigned {
51-
($($t:ty)*) => ($(
52-
#[unstable(feature = "step_trait",
53-
reason = "recently redesigned",
54-
issue = "42168")]
55-
impl Step for $t {
56-
#[inline]
57-
#[allow(trivial_numeric_casts)]
58-
fn steps_between(start: &$t, end: &$t) -> Option<usize> {
59-
if *start < *end {
60-
// Note: We assume $t <= usize here
61-
Some((*end - *start) as usize)
62-
} else {
63-
Some(0)
50+
macro_rules! step_integer_impls {
51+
(
52+
narrower than or same width as usize:
53+
$( [ $narrower_unsigned:ident $narrower_signed: ident ] ),+;
54+
wider than usize:
55+
$( [ $wider_unsigned:ident $wider_signed: ident ] ),+;
56+
) => {
57+
$(
58+
#[unstable(feature = "step_trait",
59+
reason = "recently redesigned",
60+
issue = "42168")]
61+
impl Step for $narrower_unsigned {
62+
#[inline]
63+
fn steps_between(start: &Self, end: &Self) -> Option<usize> {
64+
if *start < *end {
65+
// This relies on $narrower_unsigned <= usize
66+
Some((*end - *start) as usize)
67+
} else {
68+
Some(0)
69+
}
6470
}
65-
}
6671

67-
#[inline]
68-
fn forward(&self, n: usize) -> Option<Self> {
69-
match <$t>::try_from(n) {
70-
Ok(n_as_t) => self.checked_add(n_as_t),
71-
Err(_) => None,
72+
#[inline]
73+
fn forward(&self, n: usize) -> Option<Self> {
74+
match Self::try_from(n) {
75+
Ok(n_converted) => self.checked_add(n_converted),
76+
Err(_) => None, // if n is out of range, `something_unsigned + n` is too
77+
}
7278
}
73-
}
7479

75-
#[inline]
76-
fn backward(&self, n: usize) -> Option<Self> {
77-
match <$t>::try_from(n) {
78-
Ok(n_as_t) => self.checked_sub(n_as_t),
79-
Err(_) => None,
80+
#[inline]
81+
fn backward(&self, n: usize) -> Option<Self> {
82+
match Self::try_from(n) {
83+
Ok(n_converted) => self.checked_sub(n_converted),
84+
Err(_) => None, // if n is out of range, `something_in_range - n` is too
85+
}
8086
}
8187
}
82-
}
83-
)*)
84-
}
85-
macro_rules! step_impl_signed {
86-
($( [$t:ty : $unsigned:ty] )*) => ($(
87-
#[unstable(feature = "step_trait",
88-
reason = "recently redesigned",
89-
issue = "42168")]
90-
impl Step for $t {
91-
#[inline]
92-
#[allow(trivial_numeric_casts)]
93-
fn steps_between(start: &$t, end: &$t) -> Option<usize> {
94-
if *start < *end {
95-
// Note: We assume $t <= isize here
96-
// Use .wrapping_sub and cast to usize to compute the
97-
// difference that may not fit inside the range of isize.
98-
Some((*end as isize).wrapping_sub(*start as isize) as usize)
99-
} else {
100-
Some(0)
88+
89+
#[unstable(feature = "step_trait",
90+
reason = "recently redesigned",
91+
issue = "42168")]
92+
impl Step for $narrower_signed {
93+
#[inline]
94+
fn steps_between(start: &Self, end: &Self) -> Option<usize> {
95+
if *start < *end {
96+
// This relies on $narrower_signed <= usize
97+
//
98+
// Casting to isize extends the width but preserves the sign.
99+
// Use wrapping_sub in isize space and cast to usize
100+
// to compute the difference that may not fit inside the range of isize.
101+
Some((*end as isize).wrapping_sub(*start as isize) as usize)
102+
} else {
103+
Some(0)
104+
}
101105
}
102-
}
103106

104-
#[inline]
105-
fn forward(&self, n: usize) -> Option<Self> {
106-
match <$unsigned>::try_from(n) {
107-
Ok(n_as_unsigned) => {
108-
// Wrapping in unsigned space handles cases like
109-
// `-120_i8.forward(200) == Some(80_i8)`,
110-
// even though 200_usize is out of range for i8.
111-
let wrapped = (*self as $unsigned).wrapping_add(n_as_unsigned) as $t;
112-
if wrapped >= *self {
113-
Some(wrapped)
114-
} else {
115-
None // Addition overflowed
107+
#[inline]
108+
fn forward(&self, n: usize) -> Option<Self> {
109+
match <$narrower_unsigned>::try_from(n) {
110+
Ok(n_unsigned) => {
111+
// Wrapping in unsigned space handles cases like
112+
// `-120_i8.forward(200) == Some(80_i8)`,
113+
// even though 200_usize is out of range for i8.
114+
let self_unsigned = *self as $narrower_unsigned;
115+
let wrapped = self_unsigned.wrapping_add(n_unsigned) as Self;
116+
if wrapped >= *self {
117+
Some(wrapped)
118+
} else {
119+
None // Addition overflowed
120+
}
116121
}
122+
// If n is out of range of e.g. u8,
123+
// then it is bigger than the entire range for i8 is wide
124+
// so `any_i8 + n` would overflow i8.
125+
Err(_) => None,
117126
}
118-
Err(_) => None,
119127
}
120-
}
121-
#[inline]
122-
fn backward(&self, n: usize) -> Option<Self> {
123-
match <$unsigned>::try_from(n) {
124-
Ok(n_as_unsigned) => {
125-
// Wrapping in unsigned space handles cases like
126-
// `-120_i8.forward(200) == Some(80_i8)`,
127-
// even though 200_usize is out of range for i8.
128-
let wrapped = (*self as $unsigned).wrapping_sub(n_as_unsigned) as $t;
129-
if wrapped <= *self {
130-
Some(wrapped)
131-
} else {
132-
None // Subtraction underflowed
128+
#[inline]
129+
fn backward(&self, n: usize) -> Option<Self> {
130+
match <$narrower_unsigned>::try_from(n) {
131+
Ok(n_unsigned) => {
132+
// Wrapping in unsigned space handles cases like
133+
// `-120_i8.forward(200) == Some(80_i8)`,
134+
// even though 200_usize is out of range for i8.
135+
let self_unsigned = *self as $narrower_unsigned;
136+
let wrapped = self_unsigned.wrapping_sub(n_unsigned) as Self;
137+
if wrapped <= *self {
138+
Some(wrapped)
139+
} else {
140+
None // Subtraction underflowed
141+
}
133142
}
143+
// If n is out of range of e.g. u8,
144+
// then it is bigger than the entire range for i8 is wide
145+
// so `any_i8 - n` would underflow i8.
146+
Err(_) => None,
134147
}
135-
Err(_) => None,
136148
}
137149
}
138-
}
139-
)*)
140-
}
150+
)+
151+
152+
$(
153+
#[unstable(feature = "step_trait",
154+
reason = "recently redesigned",
155+
issue = "42168")]
156+
impl Step for $wider_unsigned {
157+
#[inline]
158+
fn steps_between(start: &Self, end: &Self) -> Option<usize> {
159+
if *start < *end {
160+
usize::try_from(*end - *start).ok()
161+
} else {
162+
Some(0)
163+
}
164+
}
141165

142-
macro_rules! step_impl_no_between {
143-
($($t:ty)*) => ($(
144-
#[unstable(feature = "step_trait",
145-
reason = "recently redesigned",
146-
issue = "42168")]
147-
impl Step for $t {
148-
#[inline]
149-
fn steps_between(_start: &Self, _end: &Self) -> Option<usize> {
150-
None
151-
}
166+
#[inline]
167+
fn forward(&self, n: usize) -> Option<Self> {
168+
self.checked_add(n as Self)
169+
}
152170

153-
#[inline]
154-
fn forward(&self, n: usize) -> Option<Self> {
155-
self.checked_add(n as $t)
171+
#[inline]
172+
fn backward(&self, n: usize) -> Option<Self> {
173+
self.checked_sub(n as Self)
174+
}
156175
}
157176

158-
#[inline]
159-
fn backward(&self, n: usize) -> Option<Self> {
160-
self.checked_sub(n as $t)
177+
#[unstable(feature = "step_trait",
178+
reason = "recently redesigned",
179+
issue = "42168")]
180+
impl Step for $wider_signed {
181+
#[inline]
182+
fn steps_between(start: &Self, end: &Self) -> Option<usize> {
183+
if *start < *end {
184+
match end.checked_sub(*start) {
185+
Some(diff) => usize::try_from(diff).ok(),
186+
// If the difference is too big for e.g. i128,
187+
// it’s also gonna be too big for usize with fewer bits.
188+
None => None
189+
}
190+
} else {
191+
Some(0)
192+
}
193+
}
194+
195+
#[inline]
196+
fn forward(&self, n: usize) -> Option<Self> {
197+
self.checked_add(n as Self)
198+
}
199+
200+
#[inline]
201+
fn backward(&self, n: usize) -> Option<Self> {
202+
self.checked_sub(n as Self)
203+
}
161204
}
162-
}
163-
)*)
205+
)+
206+
}
164207
}
165208

166-
step_impl_unsigned!(usize u8 u16 u32);
167-
step_impl_signed!([isize: usize] [i8: u8] [i16: u16] [i32: u32]);
168-
#[cfg(target_pointer_width = "64")]
169-
step_impl_unsigned!(u64);
170209
#[cfg(target_pointer_width = "64")]
171-
step_impl_signed!([i64: u64]);
172-
// If the target pointer width is not 64-bits, we
173-
// assume here that it is less than 64-bits.
174-
#[cfg(not(target_pointer_width = "64"))]
175-
step_impl_no_between!(u64 i64);
176-
step_impl_no_between!(u128 i128);
210+
step_integer_impls! {
211+
narrower than or same width as usize: [u8 i8], [u16 i16], [u32 i32], [u64 i64], [usize isize];
212+
wider than usize: [u128 i128];
213+
}
214+
215+
#[cfg(target_pointer_width = "32")]
216+
step_integer_impls! {
217+
narrower than or same width as usize: [u8 i8], [u16 i16], [u32 i32], [usize isize];
218+
wider than usize: [u64 i64], [u128 i128];
219+
}
220+
221+
#[cfg(target_pointer_width = "16")]
222+
step_integer_impls! {
223+
narrower than or same width as usize: [u8 i8], [u16 i16], [usize isize];
224+
wider than usize: [u32 i32], [u64 i64], [u128 i128];
225+
}
177226

178227
macro_rules! range_exact_iter_impl {
179228
($($t:ty)*) => ($(

0 commit comments

Comments
 (0)