Skip to content

Make RangeInclusive just a two-field struct - #42134

Merged
bors merged 2 commits into
rust-lang:masterfrom
scottmcm:rangeinclusive-struct
May 25, 2017
Merged

Make RangeInclusive just a two-field struct#42134
bors merged 2 commits into
rust-lang:masterfrom
scottmcm:rangeinclusive-struct

Conversation

@scottmcm

@scottmcmscottmcm commented May 21, 2017

Copy link
Copy Markdown
Member

Not being an enum improves ergonomics and consistency, especially since NonEmpty variant wasn't prevented from being empty. It can still be iterable without an extra "done" bit by making the range have !(start <= end), which is even possible without changing the Step trait.

Implements merged rust-lang/rfcs#1980; tracking issue #28237.

This is definitely a breaking change to anything consuming RangeInclusive directly (not as an Iterator) or constructing it without using the sugar. Is there some change that would make sense before this so compilation failures could be compatibly fixed ahead of time?

r? @aturon (as FCP proposer on the RFC)

Not being an enum improves ergonomics, especially since NonEmpty could be Empty. It can still be iterable without an extra "done" bit by making the range have !(start <= end), which is even possible without changing the Step trait.
Implements RFC 1980
Fixesrust-lang#42135
Found while fixing run-pass/range_inclusive test failure.
@frewsxcvfrewsxcv added the T-libs-api [DEPRECATED; DO NOT USE] label May 22, 2017
@carols10centscarols10cents added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 22, 2017
@aturon

Copy link
Copy Markdown
Contributor

Awesome! LGTM.

@bors: r+

@bors

bors commented May 23, 2017

Copy link
Copy Markdown
Collaborator

📌 Commit 7eaca60 has been approved by aturon

Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this pull request May 24, 2017
…turon
Make RangeInclusive just a two-field struct
Not being an enum improves ergonomics and consistency, especially since NonEmpty variant wasn't prevented from being empty. It can still be iterable without an extra "done" bit by making the range have !(start <= end), which is even possible without changing the Step trait.
Implements merged rust-lang/rfcs#1980; tracking issue rust-lang#28237.
This is definitely a breaking change to anything consuming `RangeInclusive` directly (not as an Iterator) or constructing it without using the sugar. Is there some change that would make sense before this so compilation failures could be compatibly fixed ahead of time?
r? @aturon (as FCP proposer on the RFC)
Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this pull request May 24, 2017
…turon
Make RangeInclusive just a two-field struct
Not being an enum improves ergonomics and consistency, especially since NonEmpty variant wasn't prevented from being empty. It can still be iterable without an extra "done" bit by making the range have !(start <= end), which is even possible without changing the Step trait.
Implements merged rust-lang/rfcs#1980; tracking issue rust-lang#28237.
This is definitely a breaking change to anything consuming `RangeInclusive` directly (not as an Iterator) or constructing it without using the sugar. Is there some change that would make sense before this so compilation failures could be compatibly fixed ahead of time?
r? @aturon (as FCP proposer on the RFC)
Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this pull request May 24, 2017
…turon
Make RangeInclusive just a two-field struct
Not being an enum improves ergonomics and consistency, especially since NonEmpty variant wasn't prevented from being empty. It can still be iterable without an extra "done" bit by making the range have !(start <= end), which is even possible without changing the Step trait.
Implements merged rust-lang/rfcs#1980; tracking issue rust-lang#28237.
This is definitely a breaking change to anything consuming `RangeInclusive` directly (not as an Iterator) or constructing it without using the sugar. Is there some change that would make sense before this so compilation failures could be compatibly fixed ahead of time?
r? @aturon (as FCP proposer on the RFC)
bors added a commit that referenced this pull request May 24, 2017
Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this pull request May 25, 2017
…turon
Make RangeInclusive just a two-field struct
Not being an enum improves ergonomics and consistency, especially since NonEmpty variant wasn't prevented from being empty. It can still be iterable without an extra "done" bit by making the range have !(start <= end), which is even possible without changing the Step trait.
Implements merged rust-lang/rfcs#1980; tracking issue rust-lang#28237.
This is definitely a breaking change to anything consuming `RangeInclusive` directly (not as an Iterator) or constructing it without using the sugar. Is there some change that would make sense before this so compilation failures could be compatibly fixed ahead of time?
r? @aturon (as FCP proposer on the RFC)
Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this pull request May 25, 2017
…turon
Make RangeInclusive just a two-field struct
Not being an enum improves ergonomics and consistency, especially since NonEmpty variant wasn't prevented from being empty. It can still be iterable without an extra "done" bit by making the range have !(start <= end), which is even possible without changing the Step trait.
Implements merged rust-lang/rfcs#1980; tracking issue rust-lang#28237.
This is definitely a breaking change to anything consuming `RangeInclusive` directly (not as an Iterator) or constructing it without using the sugar. Is there some change that would make sense before this so compilation failures could be compatibly fixed ahead of time?
r? @aturon (as FCP proposer on the RFC)
bors added a commit that referenced this pull request May 25, 2017
@bors
bors merged commit 7eaca60 into rust-lang:masterMay 25, 2017
@scottmcm
scottmcm deleted the rangeinclusive-struct branch May 25, 2017 06:25
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-reviewStatus: Awaiting review from the assignee but also interested parties.T-libs-api[DEPRECATED; DO NOT USE]

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@scottmcm@aturon@bors@carols10cents@frewsxcv