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.

Sign up for GitHub

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

Jump to bottom

fix Range with negative or 0 step and allow backward Ranges (having min > max) #2350

Merged
merged 4 commits into from
Nov 22, 2017
Merged

fix Range with negative or 0 step and allow backward Ranges (having min > max) #2350

merged 4 commits into from
Nov 22, 2017

Conversation

mfelsche
Copy link
Contributor

Add is_infinite method to check whether a given Range would iterate infinitely (having an inc-step which does not move min towards max).

Previously Range would also not iterate over given intervals where min > max irrelevant of the sign of the inc-step.

@mfelsche
fix Ranges with negative or 0 step and allow backward Ranges
9ffc6ad 
Add is_infinite method to check whether a given Range would iterate infinitely (having an inc-step which does not move min towards max).

Previously Range would also not iterate over given intervals where min > max irrelevant of the sign of the inc-step.
@mfelsche
Copy link
Contributor Author

I understand that this change might be controversial, as it sacrifices the simplicity of the Range implementation for a little safety.

I could also imagine to have another class called SafeRange whose constructor is partial and asserts that min <= max and inc is moving min towards max, if this change is not satisfying.

@mfelsche mfelsche added changelog - fixed Automatically add "Fixed" CHANGELOG entry on merge needs discussion during sync labels Nov 17, 2017
@jemc
Copy link
Member

jemc commented Nov 17, 2017

Seems good to me, but yes, let's discuss in the next sync.

@mfelsche
Copy link
Contributor Author

Again some unrelated OSX errors:

**** FAILED: process/WritevOrdering
	Received from stdout: 11 bytes
	Received so far: 11 bytes
	Expecting: 11 bytes
dispose: child exit code: 0
dispose: stdout: 11 bytes
dispose: stderr: 
dispose: received first data after: 	7425822 ns
dispose: total data process_time: 	8143206 ns
dispose: ProcessNotify lifetime: 	15569028 ns
ProcessError: KillError

**** FAILED: process/STDERR
dispose: child exit code: 1
dispose: stdout: 0 bytes
dispose: stderr: cat: file_does_not_exist: No such file or directory
dispose: total data process_time: 	754961948132 ns
dispose: ProcessNotify lifetime: 	45210553 ns
ProcessError: KillError

@mfelsche
Copy link
Contributor Author

And a segmentation fault on OSX.

@mfelsche mfelsche merged commit b757baf into master Nov 22, 2017
ponylang-main added a commit that referenced this pull request Nov 22, 2017
@ponylang-main
Update CHANGELOG for PR #2350 [skip ci]
bbe84a0
@mfelsche mfelsche deleted the range-improvements branch November 22, 2017 20:58
@EpicEric EpicEric mentioned this pull request Dec 18, 2017
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
changelog - fixed Automatically add "Fixed" CHANGELOG entry on merge
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@mfelsche @jemc @SeanTAllen