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

Allow Iterator.stop to be used inside Iterator.of block #4208

Merged
merged 1 commit into from
Apr 3, 2017
Merged

Allow Iterator.stop to be used inside Iterator.of block #4208

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions spec/std/iterator_spec.cr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,19 @@ describe Iterator do
iter = Iterator.of { a += 1 }
iter.first(3).to_a.should eq([1, 2, 3])
end

it "creates singleton from block can call Iterator.stop" do
a = 0
iter = Iterator.of do
if a >= 5
Iterator.stop
else
a += 1
end
end
iter.should be_a(Iterator(Int32))
iter.first(10).to_a.should eq([1, 2, 3, 4, 5])
end
end

describe "compact_map" do
Expand Down
10 changes: 8 additions & 2 deletions src/iterator.cr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,13 +131,19 @@ module Iterator(T)
end

def self.of(&block : -> T)
SingletonProc(T).new(block)
SingletonProc(typeof(without_stop(&block))).new(block)
end

private def self.without_stop(&block : -> T)
e = block.call
raise "" if e.is_a?(Iterator::Stop)
Copy link
Contributor

Choose a reason for hiding this comment

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

raise ArgumentError.new?

Copy link
Contributor

Choose a reason for hiding this comment

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

This method isn't called, only used for typeof.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, I see. Still, raise "" (which internally translates to raise ArgumentError.new, no?) looks odd for me...

Copy link
Contributor

Choose a reason for hiding this comment

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

No, raise "" = raise Exception.new

Copy link
Contributor

Choose a reason for hiding this comment

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

So, why no raise Exception.new? Seems imo clearer.

Copy link
Member Author

Choose a reason for hiding this comment

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

We just need a function that returns NoReturn. The same raise "" pattern is used in https://github.com/crystal-lang/crystal/blob/master/src/iterator.cr#L472 .
The code is never executed actually.

Copy link
Contributor

Choose a reason for hiding this comment

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

Fair 'nuff, I just thought it looks odd aside of its actual function.

Copy link
Member Author

Choose a reason for hiding this comment

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

We could find probably a more idiomatic way to express this. maybe a fail : NoReturn or halt or unreachable that will abort compilation at codegen but will return NoReturn in semantic could work better. #someday

e
end

private struct SingletonProc(T)
include Iterator(T)

def initialize(@proc : -> T)
def initialize(@proc : (-> (T | Iterator::Stop)) | (-> T))
end

def next
Expand Down