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

Stabilize Index traits and most range notation #21258

Merged
merged 5 commits into from
Jan 22, 2015

Conversation

aturon
Copy link
Member

@aturon aturon commented Jan 16, 2015

This PR marks as #[stable]:

  • The Index and IndexMut traits. These are stabilized as taking the
    index itself by reference; after extensive discussion it was
    determined that this is a better match with our choices
    elsewhere (e.g. making comparison operators auto-reference), and that
    the use cases for by-value indices are better handled through
    IndexSet.
  • The Range, RangeFrom and RangeTo structs, introduced for range
    notation.
  • Various impls of Index and IndexMut.

The FullRange struct is left unstable as we may wish to rename it to
RangeFull in the future.

This PR also deprecates slice, slice_from, slice_to and their
mutable variants in favor of slice notation.

The as_slice methods are left intact, for now.

This PR also removes the Step trait in favor of direct
implementation of iterator traits on ranges for integers. The Step
trait was not a terribly useful factoring internally, and it is likely
that external integer types are best off implementing range iterators
directly. It was removed to simplify the API surface. We can always
reintroduce Step later if it turns out to be useful.

Due to this removal, this is a:

[breaking-change]

Closes #19148
Closes #20981

@rust-highfive
Copy link
Collaborator

r? @pcwalton

(rust_highfive has picked a reviewer for you, use r? to override)

@aturon
Copy link
Member Author

aturon commented Jan 16, 2015

r? @alexcrichton

@aturon
Copy link
Member Author

aturon commented Jan 16, 2015

@alexcrichton BTW, I didn't deprecate the range stuff in iterator yet -- I want to do this as part of the work @gankro is doing, which aims to replace all of the range_ functions with range notation + adapters.


range_other_impls!(uint u8 u16 u32 u64 int i8 i16 i32 i64);
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't i64/u64 not get ExactSizeIterator on 32-bit platforms? (due to range_impl_no_hint above)

@alexcrichton
Copy link
Member

BTW, I didn't deprecate the range stuff in iterator yet -- I want to do this as part of the work @gankro is doing, which aims to replace all of the range_ functions with range notation + adapters.

This sounds fantastic!

@alexcrichton
Copy link
Member

Does this close #19148 ?

@aturon
Copy link
Member Author

aturon commented Jan 16, 2015

@alexcrichton

Does this close #19148 ?

Sure, why not! I've marked it as such and I'll open a separate ticket for the remaining prelude and renaming issue.

@alexcrichton
Copy link
Member

r=me with the #[stable] tag for slice/str/String impls as well

@Gankra
Copy link
Contributor

Gankra commented Jan 17, 2015

Will taking by-reference have a poor interaction with IndexSet (which should be by-val, I think)?

@aturon aturon force-pushed the stab-3-index branch 2 times, most recently from cc49f5a to c6db309 Compare January 20, 2015 23:39
@aturon
Copy link
Member Author

aturon commented Jan 21, 2015

@gankro No, IndexSet will be a separate, mutually-exclusive trait. (That said I want to work with @nikomatsakis to ensure we're confident with the final IndexSet design soon.)

@aturon
Copy link
Member Author

aturon commented Jan 21, 2015

@bors: r=alexcrichton 0ee5f82

@bors
Copy link
Contributor

bors commented Jan 21, 2015

⌛ Testing commit 0ee5f82 with merge 74a7fd9...

@bors
Copy link
Contributor

bors commented Jan 21, 2015

💔 Test failed - auto-win-64-nopt-t

This commit marks as `#[stable]`:

* The `Index` and `IndexMut` traits. These are stabilized as taking the
  index itself *by reference*; after extensive discussion it was
  determined that this is a better match with our choices
  elsewhere (e.g. making comparison operators auto-reference), and that
  the use cases for by-value indices are better handled through
  `IndexSet`.

* The `Range`, `RangeFrom` and `RangeTo` structs, introduced for range
  notation.

* Various impls of `Index` and `IndexMut`.

The `FullRange` struct is left unstable as we may wish to rename it to
`RangeFull` in the future.

This commit also *removes* the `Step` trait in favor of direct
implementation of iterator traits on ranges for integers. The `Step`
trait was not a terribly useful factoring internally, and it is likely
that external integer types are best off implementing range iterators
directly. It was removed to simplify the API surface. We can always
reintroduce `Step` later if it turns out to be useful.

Due to this removal, this is a:

[breaking-change]
This commit deprecates `slice`, `slice_from`, `slice_to` and their
mutable variants in favor of slice notation.

The `as_slice` methods are left intact, for now.

[breaking-change]
@aturon
Copy link
Member Author

aturon commented Jan 21, 2015

@bors: r=alexcrichton 0fc3e8a

@aturon
Copy link
Member Author

aturon commented Jan 21, 2015

@bors: r-

@aturon
Copy link
Member Author

aturon commented Jan 21, 2015

@bors: r=alexcrichton da8023d

@bors
Copy link
Contributor

bors commented Jan 21, 2015

⌛ Testing commit da8023d with merge 29ae8d6...

@bors
Copy link
Contributor

bors commented Jan 21, 2015

💔 Test failed - auto-mac-64-opt

@aturon
Copy link
Member Author

aturon commented Jan 21, 2015

@bors: r=alexcrichton 537889a

alexcrichton added a commit to alexcrichton/rust that referenced this pull request Jan 21, 2015
Conflicts:
	src/libcore/ops.rs
	src/librustc_typeck/astconv.rs
	src/libstd/io/mem.rs
	src/libsyntax/parse/lexer/mod.rs
@bors bors merged commit 537889a into rust-lang:master Jan 22, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Overflow/panic problem in Step and ExactSizeIterator for Range<BigInt> Implement cmp and ops reform
7 participants