From a87587602732a7fe6739fdcfadce025fafbb55dd Mon Sep 17 00:00:00 2001 From: Jacob Pratt Date: Thu, 1 Apr 2021 05:06:28 -0400 Subject: [PATCH] Make Range implementation safe --- compiler/rustc_index/src/vec.rs | 2 +- library/core/src/iter/range.rs | 51 ++++++++++------------ src/test/ui/impl-trait/example-calendar.rs | 2 +- 3 files changed, 24 insertions(+), 31 deletions(-) diff --git a/compiler/rustc_index/src/vec.rs b/compiler/rustc_index/src/vec.rs index 1b1a59a254e..9cf6756a090 100644 --- a/compiler/rustc_index/src/vec.rs +++ b/compiler/rustc_index/src/vec.rs @@ -184,7 +184,7 @@ macro_rules! newtype_index { } } - unsafe impl ::std::iter::Step for $type { + impl ::std::iter::Step for $type { #[inline] fn steps_between(start: &Self, end: &Self) -> Option { ::steps_between( diff --git a/library/core/src/iter/range.rs b/library/core/src/iter/range.rs index 2bd0380bad6..63e6181cae2 100644 --- a/library/core/src/iter/range.rs +++ b/library/core/src/iter/range.rs @@ -9,15 +9,8 @@ use super::{FusedIterator, TrustedLen, TrustedRandomAccess}; /// /// The *successor* operation moves towards values that compare greater. /// The *predecessor* operation moves towards values that compare lesser. -/// -/// # Safety -/// -/// This trait is `unsafe` because its implementation must be correct for -/// the safety of `unsafe trait TrustedLen` implementations, and the results -/// of using this trait can otherwise be trusted by `unsafe` code to be correct -/// and fulfill the listed obligations. #[unstable(feature = "step_trait", reason = "recently redesigned", issue = "42168")] -pub unsafe trait Step: Clone + PartialOrd + Sized { +pub trait Step: Clone + PartialOrd + Sized { /// Returns the number of *successor* steps required to get from `start` to `end`. /// /// Returns `None` if the number of steps would overflow `usize` @@ -237,7 +230,7 @@ macro_rules! step_integer_impls { $( #[allow(unreachable_patterns)] #[unstable(feature = "step_trait", reason = "recently redesigned", issue = "42168")] - unsafe impl Step for $u_narrower { + impl Step for $u_narrower { step_identical_methods!(); #[inline] @@ -269,7 +262,7 @@ macro_rules! step_integer_impls { #[allow(unreachable_patterns)] #[unstable(feature = "step_trait", reason = "recently redesigned", issue = "42168")] - unsafe impl Step for $i_narrower { + impl Step for $i_narrower { step_identical_methods!(); #[inline] @@ -333,7 +326,7 @@ macro_rules! step_integer_impls { $( #[allow(unreachable_patterns)] #[unstable(feature = "step_trait", reason = "recently redesigned", issue = "42168")] - unsafe impl Step for $u_wider { + impl Step for $u_wider { step_identical_methods!(); #[inline] @@ -358,7 +351,7 @@ macro_rules! step_integer_impls { #[allow(unreachable_patterns)] #[unstable(feature = "step_trait", reason = "recently redesigned", issue = "42168")] - unsafe impl Step for $i_wider { + impl Step for $i_wider { step_identical_methods!(); #[inline] @@ -408,7 +401,7 @@ step_integer_impls! { } #[unstable(feature = "step_trait", reason = "recently redesigned", issue = "42168")] -unsafe impl Step for char { +impl Step for char { #[inline] fn steps_between(&start: &char, &end: &char) -> Option { let start = start as u32; @@ -519,8 +512,8 @@ impl Iterator for ops::Range { #[inline] fn next(&mut self) -> Option { if self.start < self.end { - // SAFETY: just checked precondition - let n = unsafe { Step::forward_unchecked(self.start.clone(), 1) }; + let n = + Step::forward_checked(self.start.clone(), 1).expect("`Step` invariants not upheld"); Some(mem::replace(&mut self.start, n)) } else { None @@ -541,8 +534,8 @@ impl Iterator for ops::Range { fn nth(&mut self, n: usize) -> Option { if let Some(plus_n) = Step::forward_checked(self.start.clone(), n) { if plus_n < self.end { - // SAFETY: just checked precondition - self.start = unsafe { Step::forward_unchecked(plus_n.clone(), 1) }; + self.start = + Step::forward_checked(plus_n.clone(), 1).expect("`Step` invariants not upheld"); return Some(plus_n); } } @@ -632,8 +625,8 @@ impl DoubleEndedIterator for ops::Range { #[inline] fn next_back(&mut self) -> Option { if self.start < self.end { - // SAFETY: just checked precondition - self.end = unsafe { Step::backward_unchecked(self.end.clone(), 1) }; + self.end = + Step::backward_checked(self.end.clone(), 1).expect("`Step` invariants not upheld"); Some(self.end.clone()) } else { None @@ -644,8 +637,8 @@ impl DoubleEndedIterator for ops::Range { fn nth_back(&mut self, n: usize) -> Option { if let Some(minus_n) = Step::backward_checked(self.end.clone(), n) { if minus_n > self.start { - // SAFETY: just checked precondition - self.end = unsafe { Step::backward_unchecked(minus_n, 1) }; + self.end = + Step::backward_checked(minus_n, 1).expect("`Step` invariants not upheld"); return Some(self.end.clone()); } } @@ -711,8 +704,8 @@ impl Iterator for ops::RangeInclusive { } let is_iterating = self.start < self.end; Some(if is_iterating { - // SAFETY: just checked precondition - let n = unsafe { Step::forward_unchecked(self.start.clone(), 1) }; + let n = + Step::forward_checked(self.start.clone(), 1).expect("`Step` invariants not upheld"); mem::replace(&mut self.start, n) } else { self.exhausted = true; @@ -774,8 +767,8 @@ impl Iterator for ops::RangeInclusive { let mut accum = init; while self.start < self.end { - // SAFETY: just checked precondition - let n = unsafe { Step::forward_unchecked(self.start.clone(), 1) }; + let n = + Step::forward_checked(self.start.clone(), 1).expect("`Step` invariants not upheld"); let n = mem::replace(&mut self.start, n); accum = f(accum, n)?; } @@ -828,8 +821,8 @@ impl DoubleEndedIterator for ops::RangeInclusive { } let is_iterating = self.start < self.end; Some(if is_iterating { - // SAFETY: just checked precondition - let n = unsafe { Step::backward_unchecked(self.end.clone(), 1) }; + let n = + Step::backward_checked(self.end.clone(), 1).expect("`Step` invariants not upheld"); mem::replace(&mut self.end, n) } else { self.exhausted = true; @@ -879,8 +872,8 @@ impl DoubleEndedIterator for ops::RangeInclusive { let mut accum = init; while self.start < self.end { - // SAFETY: just checked precondition - let n = unsafe { Step::backward_unchecked(self.end.clone(), 1) }; + let n = + Step::backward_checked(self.end.clone(), 1).expect("`Step` invariants not upheld"); let n = mem::replace(&mut self.end, n); accum = f(accum, n)?; } diff --git a/src/test/ui/impl-trait/example-calendar.rs b/src/test/ui/impl-trait/example-calendar.rs index 238f3fa31ed..0d011fafb4b 100644 --- a/src/test/ui/impl-trait/example-calendar.rs +++ b/src/test/ui/impl-trait/example-calendar.rs @@ -157,7 +157,7 @@ impl<'a, 'b> std::ops::Add<&'b NaiveDate> for &'a NaiveDate { } } -unsafe impl std::iter::Step for NaiveDate { +impl std::iter::Step for NaiveDate { fn steps_between(_: &Self, _: &Self) -> Option { unimplemented!() }