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

impl Copy for Range<Idx> where Idx: Copy? #48649

Closed
SoniEx2 opened this issue Mar 2, 2018 · 5 comments
Closed

impl Copy for Range<Idx> where Idx: Copy? #48649

SoniEx2 opened this issue Mar 2, 2018 · 5 comments
Labels
C-enhancement Category: An issue proposing an enhancement or a PR with one. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@SoniEx2
Copy link
Contributor

SoniEx2 commented Mar 2, 2018

Would it be possible for Range<Idx> to be Copy if Idx is Copy?

@sinkuu
Copy link
Contributor

sinkuu commented Mar 2, 2018

Definitely possible, but there is a reason why it is currently not.
#27186 (comment)

@SoniEx2
Copy link
Contributor Author

SoniEx2 commented Mar 3, 2018

Can't you implement iterator for &mut Range, and have Range itself be Copy?

It would solve the lint problem, altho it's a breaking change and also requires you to use "for x in &mut a..b" I guess...

@hanna-kruppe
Copy link
Contributor

Yeah that would be a breaking change (and not the kind that epochs enable). If we could remove the impl Iterator for Range<...>, we should do that and just replace it with an IntoIterator impl. Alas...

@pietroalbini pietroalbini added C-enhancement Category: An issue proposing an enhancement or a PR with one. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Mar 6, 2018
@danielhenrymantilla
Copy link
Contributor

danielhenrymantilla commented Mar 31, 2019

Against Copy

The choice of removing Copy impls instead of adjusted for loop desugaring or linting was made to prevent this problematic case:

let mut iter = 0..n;
for i in iter { if i > 2 { break; } }
iter.collect()

The above code triggers an unused_mut lint

Maybe in 2015 this looked like a dangerous thing to have, but I really feel like nowadays Rustaceans know that for loops capture by value (most will even know that it happens because of the implicit .into_iter() call).

I mean, if we are to be oblivious to .into_iter(), I don't see that much difference between the above code and:

let mut iter = &[0, 1, 2, 3, 4, 5];
for i in iter { if *i > 2 { break; } }
FromIterator::from_iter(iter)

Plus there are two ways to avoid using a mutable iterator as an iterable (IntoIter):

  • manually advance the iteration by explicitly calling .next().

    • For instance, with while let Some(x) = iterator.next()
  • feed the iterator by ref: for x in iterator.by_ref() or for x in &mut iterator, where both are very explicit and readable idioms.

In favor of Copy

Factoring out repeated ranges:

let mut s = 0;
let range = 0 .. 10;

println!("{}", s);
for i in range {
    s += i;
    println!("{}", s);
}
for i in range.rev() { // currently errors
    s -= i;
    println!("{}", s);
}

Storing Ranges in Copy structs

pub struct ZipSubSlice {
  left: [u8; 8],
  right: [u8; 8],
  pub range: Range<usize>,
}

impl Copy for ZipSubSlice {} // currently errors

Alternatives

Edition 2019: Iterator -> IntoIterator

Wait for the next edition to change impl<Idx : Step> Iterator for Range<Idx> { //... into impl<Idx : Copy> Copy for Range<Idx> {} impl <Idx : Step> IntoIterator for Range<Idx> { //...

A secondary CopyRange struct in std

which would be copy, and with

impl<Idx : Copy> From<Range<Idx>> for CopyRange<Idx> { /* ... */ }
impl<Idx : Copy + Step> IntoIterator for CopyRange<Idx> {
    type IntoIter = Range<Idx>;
    // ...
}

For instance, something along these lines:

use ::std::{fmt::Debug, iter::Step};

/// constraint alias
pub
trait Idx : Copy + Debug + Eq + Ord + Step {}
impl<T : Copy + Debug + Eq + Ord + Step> Idx for T {}

// type level enum
trait BoundType : Debug + Default + Copy + Eq { fn less<T : Idx> (lhs: T, rhs: T) -> bool; }
    impl BoundType for Exclusive {
        #[inline]
        fn less<T : Idx> (lhs: T, rhs: T) -> bool { lhs < rhs }
    }
    #[derive(Debug, Default, Clone, Copy, PartialEq, Eq)]
    pub
    struct Exclusive;

    impl BoundType for Inclusive {
        #[inline]
        fn less<T : Idx> (lhs: T, rhs: T) -> bool { lhs <= rhs }
    }
    #[derive(Debug, Default, Clone, Copy, PartialEq, Eq)]
    pub
    struct Inclusive;


#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub
struct RangeCopy<T : Idx, B : BoundType>
{
    pub
    start: T,

    pub
    end: T,

    _bound_type: B,
}

pub
type RangeExclusiveCopy<T> = RangeCopy<T, Exclusive>;

pub
type RangeInclusiveCopy<T> = RangeCopy<T, Inclusive>;

impl<T : Idx, B : BoundType> RangeCopy<T, B> {
    #[inline(always)]
    pub
    fn contains (self: Self, value: T) -> bool
    {
        self.start <= value && B::less(value, self.end)
    }
}

impl<T : Idx> From<Range<T>> for RangeExclusiveCopy<T> {
    #[inline]
    fn from (range: Range<T>) -> Self
    {
        let Range { start, end } = range;
        RangeCopy { start, end, _bound_type: BoundType::default() }
    }
}

impl<T : Idx> From<RangeExclusiveCopy<T>> for Range<T> {
    #[inline]
    fn from (range: RangeExclusiveCopy<T>) -> Self
    {
        let RangeCopy { start, end, .. } = range;
        Self { start, end }
    }
}

impl<T : Idx> IntoIterator for RangeExclusiveCopy<T> {
    type Item = T;
    type IntoIter = Range<T>;

    #[inline]
    fn into_iter (self: Self) -> Self::IntoIter
    {
         self.into()
    }
}

// etc.

@Mark-Simulacrum
Copy link
Member

Since we've consciously made this decision in the past I think a (small) RFC would be warranted. Closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement Category: An issue proposing an enhancement or a PR with one. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

6 participants