From: kennytm Date: Mon, 18 Jun 2018 20:08:20 +0000 (+0800) Subject: Change RangeInclusive to a three-field struct. X-Git-Url: https://git.lizzy.rs/?a=commitdiff_plain;h=0d7e9933d3cac85bc1f11dc0fec67fcad77784ca;p=rust.git Change RangeInclusive to a three-field struct. Fix #45222. --- diff --git a/src/libcore/iter/range.rs b/src/libcore/iter/range.rs index 0b279f66b88..16849e84f27 100644 --- a/src/libcore/iter/range.rs +++ b/src/libcore/iter/range.rs @@ -10,7 +10,7 @@ use convert::TryFrom; use mem; -use ops::{self, Add, Sub, Try}; +use ops::{self, Add, Sub}; use usize; use super::{FusedIterator, TrustedLen}; @@ -330,23 +330,23 @@ impl Iterator for ops::RangeInclusive { #[inline] fn next(&mut self) -> Option { - if self.start <= self.end { - if self.start < self.end { - let n = self.start.add_one(); - Some(mem::replace(&mut self.start, n)) - } else { - let last = self.start.replace_one(); - self.end.replace_zero(); - Some(last) - } + if self.is_empty() { + self.is_iterating = Some(false); + return None; + } + if self.start < self.end { + let n = self.start.add_one(); + self.is_iterating = Some(true); + Some(mem::replace(&mut self.start, n)) } else { - None + self.is_iterating = Some(false); + Some(self.start.clone()) } } #[inline] fn size_hint(&self) -> (usize, Option) { - if !(self.start <= self.end) { + if self.is_empty() { return (0, Some(0)); } @@ -358,25 +358,29 @@ fn size_hint(&self) -> (usize, Option) { #[inline] fn nth(&mut self, n: usize) -> Option { + if self.is_empty() { + self.is_iterating = Some(false); + return None; + } + if let Some(plus_n) = self.start.add_usize(n) { use cmp::Ordering::*; match plus_n.partial_cmp(&self.end) { Some(Less) => { + self.is_iterating = Some(true); self.start = plus_n.add_one(); return Some(plus_n) } Some(Equal) => { - self.start.replace_one(); - self.end.replace_zero(); + self.is_iterating = Some(false); return Some(plus_n) } _ => {} } } - self.start.replace_one(); - self.end.replace_zero(); + self.is_iterating = Some(false); None } @@ -394,68 +398,24 @@ fn min(mut self) -> Option { fn max(mut self) -> Option { self.next_back() } - - #[inline] - fn try_fold(&mut self, init: B, mut f: F) -> R where - Self: Sized, F: FnMut(B, Self::Item) -> R, R: Try - { - let mut accum = init; - if self.start <= self.end { - loop { - let (x, done) = - if self.start < self.end { - let n = self.start.add_one(); - (mem::replace(&mut self.start, n), false) - } else { - self.end.replace_zero(); - (self.start.replace_one(), true) - }; - accum = f(accum, x)?; - if done { break } - } - } - Try::from_ok(accum) - } } #[stable(feature = "inclusive_range", since = "1.26.0")] impl DoubleEndedIterator for ops::RangeInclusive { #[inline] fn next_back(&mut self) -> Option { - if self.start <= self.end { - if self.start < self.end { - let n = self.end.sub_one(); - Some(mem::replace(&mut self.end, n)) - } else { - let last = self.end.replace_zero(); - self.start.replace_one(); - Some(last) - } - } else { - None + if self.is_empty() { + self.is_iterating = Some(false); + return None; } - } - - #[inline] - fn try_rfold(&mut self, init: B, mut f: F) -> R where - Self: Sized, F: FnMut(B, Self::Item) -> R, R: Try - { - let mut accum = init; - if self.start <= self.end { - loop { - let (x, done) = - if self.start < self.end { - let n = self.end.sub_one(); - (mem::replace(&mut self.end, n), false) - } else { - self.start.replace_one(); - (self.end.replace_zero(), true) - }; - accum = f(accum, x)?; - if done { break } - } + if self.start < self.end { + let n = self.end.sub_one(); + self.is_iterating = Some(true); + Some(mem::replace(&mut self.end, n)) + } else { + self.is_iterating = Some(false); + Some(self.end.clone()) } - Try::from_ok(accum) } } diff --git a/src/libcore/ops/range.rs b/src/libcore/ops/range.rs index 01e279589da..3f9ac8a54bf 100644 --- a/src/libcore/ops/range.rs +++ b/src/libcore/ops/range.rs @@ -9,6 +9,7 @@ // except according to those terms. use fmt; +use hash::{Hash, Hasher}; /// An unbounded range (`..`). /// @@ -326,15 +327,37 @@ pub fn contains(&self, item: &U) -> bool /// assert_eq!(arr[1..=2], [ 1,2 ]); // RangeInclusive /// ``` #[doc(alias = "..=")] -#[derive(Clone, PartialEq, Eq, Hash)] // not Copy -- see #27186 +#[derive(Clone)] // not Copy -- see #27186 #[stable(feature = "inclusive_range", since = "1.26.0")] pub struct RangeInclusive { - // FIXME: The current representation follows RFC 1980, - // but it is known that LLVM is not able to optimize loops following that RFC. - // Consider adding an extra `bool` field to indicate emptiness of the range. - // See #45222 for performance test cases. pub(crate) start: Idx, pub(crate) end: Idx, + pub(crate) is_iterating: Option, + // This field is: + // - `None` when next() or next_back() was never called + // - `Some(true)` when `start <= end` assuming no overflow + // - `Some(false)` otherwise + // The field cannot be a simple `bool` because the `..=` constructor can + // accept non-PartialOrd types, also we want the constructor to be const. +} + +#[stable(feature = "inclusive_range", since = "1.26.0")] +impl PartialEq for RangeInclusive { + #[inline] + fn eq(&self, other: &Self) -> bool { + self.start == other.start && self.end == other.end + } +} + +#[stable(feature = "inclusive_range", since = "1.26.0")] +impl Eq for RangeInclusive {} + +#[stable(feature = "inclusive_range", since = "1.26.0")] +impl Hash for RangeInclusive { + fn hash(&self, state: &mut H) { + self.start.hash(state); + self.end.hash(state); + } } impl RangeInclusive { @@ -350,7 +373,7 @@ impl RangeInclusive { #[stable(feature = "inclusive_range_methods", since = "1.27.0")] #[inline] pub const fn new(start: Idx, end: Idx) -> Self { - Self { start, end } + Self { start, end, is_iterating: None } } /// Returns the lower bound of the range (inclusive). @@ -492,8 +515,9 @@ pub fn contains(&self, item: &U) -> bool /// assert!(r.is_empty()); /// ``` #[unstable(feature = "range_is_empty", reason = "recently added", issue = "48111")] + #[inline] pub fn is_empty(&self) -> bool { - !(self.start <= self.end) + !self.is_iterating.unwrap_or_else(|| self.start <= self.end) } } diff --git a/src/libcore/slice/mod.rs b/src/libcore/slice/mod.rs index ed29d80cb90..e6db4cb38ec 100644 --- a/src/libcore/slice/mod.rs +++ b/src/libcore/slice/mod.rs @@ -2262,36 +2262,36 @@ impl SliceIndex<[T]> for ops::RangeInclusive { #[inline] fn get(self, slice: &[T]) -> Option<&[T]> { - if self.end == usize::max_value() { None } - else { (self.start..self.end + 1).get(slice) } + if *self.end() == usize::max_value() { None } + else { (*self.start()..self.end() + 1).get(slice) } } #[inline] fn get_mut(self, slice: &mut [T]) -> Option<&mut [T]> { - if self.end == usize::max_value() { None } - else { (self.start..self.end + 1).get_mut(slice) } + if *self.end() == usize::max_value() { None } + else { (*self.start()..self.end() + 1).get_mut(slice) } } #[inline] unsafe fn get_unchecked(self, slice: &[T]) -> &[T] { - (self.start..self.end + 1).get_unchecked(slice) + (*self.start()..self.end() + 1).get_unchecked(slice) } #[inline] unsafe fn get_unchecked_mut(self, slice: &mut [T]) -> &mut [T] { - (self.start..self.end + 1).get_unchecked_mut(slice) + (*self.start()..self.end() + 1).get_unchecked_mut(slice) } #[inline] fn index(self, slice: &[T]) -> &[T] { - if self.end == usize::max_value() { slice_index_overflow_fail(); } - (self.start..self.end + 1).index(slice) + if *self.end() == usize::max_value() { slice_index_overflow_fail(); } + (*self.start()..self.end() + 1).index(slice) } #[inline] fn index_mut(self, slice: &mut [T]) -> &mut [T] { - if self.end == usize::max_value() { slice_index_overflow_fail(); } - (self.start..self.end + 1).index_mut(slice) + if *self.end() == usize::max_value() { slice_index_overflow_fail(); } + (*self.start()..self.end() + 1).index_mut(slice) } } diff --git a/src/libcore/str/mod.rs b/src/libcore/str/mod.rs index 5ae2f6349e5..255e8a07d75 100644 --- a/src/libcore/str/mod.rs +++ b/src/libcore/str/mod.rs @@ -2004,31 +2004,31 @@ impl SliceIndex for ops::RangeInclusive { type Output = str; #[inline] fn get(self, slice: &str) -> Option<&Self::Output> { - if self.end == usize::max_value() { None } - else { (self.start..self.end+1).get(slice) } + if *self.end() == usize::max_value() { None } + else { (*self.start()..self.end()+1).get(slice) } } #[inline] fn get_mut(self, slice: &mut str) -> Option<&mut Self::Output> { - if self.end == usize::max_value() { None } - else { (self.start..self.end+1).get_mut(slice) } + if *self.end() == usize::max_value() { None } + else { (*self.start()..self.end()+1).get_mut(slice) } } #[inline] unsafe fn get_unchecked(self, slice: &str) -> &Self::Output { - (self.start..self.end+1).get_unchecked(slice) + (*self.start()..self.end()+1).get_unchecked(slice) } #[inline] unsafe fn get_unchecked_mut(self, slice: &mut str) -> &mut Self::Output { - (self.start..self.end+1).get_unchecked_mut(slice) + (*self.start()..self.end()+1).get_unchecked_mut(slice) } #[inline] fn index(self, slice: &str) -> &Self::Output { - if self.end == usize::max_value() { str_index_overflow_fail(); } - (self.start..self.end+1).index(slice) + if *self.end() == usize::max_value() { str_index_overflow_fail(); } + (*self.start()..self.end()+1).index(slice) } #[inline] fn index_mut(self, slice: &mut str) -> &mut Self::Output { - if self.end == usize::max_value() { str_index_overflow_fail(); } - (self.start..self.end+1).index_mut(slice) + if *self.end() == usize::max_value() { str_index_overflow_fail(); } + (*self.start()..self.end()+1).index_mut(slice) } } diff --git a/src/test/codegen/issue-45222.rs b/src/test/codegen/issue-45222.rs new file mode 100644 index 00000000000..30a03243f01 --- /dev/null +++ b/src/test/codegen/issue-45222.rs @@ -0,0 +1,74 @@ +// Copyright 2018 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// compile-flags: -O +// min-llvm-version 6.0 + +#![crate_type = "lib"] + +// verify that LLVM recognizes a loop involving 0..=n and will const-fold it. + +//------------------------------------------------------------------------------ +// Example from original issue #45222 + +fn foo2(n: u64) -> u64 { + let mut count = 0; + for _ in 0..n { + for j in (0..=n).rev() { + count += j; + } + } + count +} + +// CHECK-LABEL: @check_foo2 +#[no_mangle] +pub fn check_foo2() -> u64 { + // CHECK: ret i64 500005000000000 + foo2(100000) +} + +//------------------------------------------------------------------------------ +// Simplified example of #45222 + +fn triangle_inc(n: u64) -> u64 { + let mut count = 0; + for j in 0 ..= n { + count += j; + } + count +} + +// CHECK-LABEL: @check_triangle_inc +#[no_mangle] +pub fn check_triangle_inc() -> u64 { + // CHECK: ret i64 5000050000 + triangle_inc(100000) +} + +//------------------------------------------------------------------------------ +// Demo in #48012 + +fn foo3r(n: u64) -> u64 { + let mut count = 0; + (0..n).for_each(|_| { + (0 ..= n).rev().for_each(|j| { + count += j; + }) + }); + count +} + +// CHECK-LABEL: @check_foo3r +#[no_mangle] +pub fn check_foo3r() -> u64 { + // CHECK: ret i64 500005000000000 + foo3r(100000) +}