-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Add select keyword #3130
Merged
Merged
Add select keyword #3130
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,68 @@ | ||
require "../../spec_helper" | ||
|
||
describe "Normalize: case" do | ||
it "normalizes select with call" do | ||
assert_expand "select; when foo; body; when bar; baz; end", <<-CODE | ||
__temp_1, __temp_2 = ::Channel.select({foo_select_action, bar_select_action}) | ||
case __temp_1 | ||
when 0 | ||
body | ||
when 1 | ||
baz | ||
else | ||
::raise("Bug: invalid select index") | ||
end | ||
CODE | ||
end | ||
|
||
it "normalizes select with assign" do | ||
assert_expand "select; when x = foo; x + 1; end", <<-CODE | ||
__temp_1, __temp_2 = ::Channel.select({foo_select_action}) | ||
case __temp_1 | ||
when 0 | ||
x = __temp_2.as(typeof(foo)) | ||
x + 1 | ||
else | ||
::raise("Bug: invalid select index") | ||
end | ||
CODE | ||
end | ||
|
||
it "normalizes select with else" do | ||
assert_expand "select; when foo; body; else; baz; end", <<-CODE | ||
__temp_1, __temp_2 = ::Channel.select({foo_select_action}, true) | ||
case __temp_1 | ||
when 0 | ||
body | ||
else | ||
baz | ||
end | ||
CODE | ||
end | ||
|
||
it "normalizes select with assign and question method" do | ||
assert_expand "select; when x = foo?; x + 1; end", <<-CODE | ||
__temp_1, __temp_2 = ::Channel.select({foo_select_action?}) | ||
case __temp_1 | ||
when 0 | ||
x = __temp_2.as(typeof(foo?)) | ||
x + 1 | ||
else | ||
::raise("Bug: invalid select index") | ||
end | ||
CODE | ||
end | ||
|
||
it "normalizes select with assign and bang method" do | ||
assert_expand "select; when x = foo!; x + 1; end", <<-CODE | ||
__temp_1, __temp_2 = ::Channel.select({foo_select_action!}) | ||
case __temp_1 | ||
when 0 | ||
x = __temp_2.as(typeof(foo!)) | ||
x + 1 | ||
else | ||
::raise("Bug: invalid select index") | ||
end | ||
CODE | ||
end | ||
end |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
_for_select or _action, which is it? :)
Not too sure a about either name yet, have to sleep over it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, good catch.
_for_select
was our original choice, but_select_action
matches theSelectAction
"interface". We aren't sure about the name either, but since it's not going to be written manually...There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just conceptually, not referring to the existing
Future
, can'tSelectAction
be thought of as a promise/future?Channel.select
receives a list of promises and returns the first that could compute a value. Following that line of thought,SelectActon
becomesPromise
and the suffix becomes_promise
, andchannel.receive_promise
suddenly reads pretty well to me. Going further the whole concept could be decoupled fromChannel
andChannel.select
could becomePromise.select
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I must admit it sounds good, but on second thought this is not really a promise. It's one possible action that can be executed in a select branch, but it's not guaranteed that it will be executed.
Plus, adding more functionality to it is pretty hard, you have to know the runtime internals. I'd say it's extensible so we can add more cases in the standard library, but I wouldn't expect 3rd party libraries to extend this, at least not now. So I prefer it to keep it under Channel and with a name that explicitly states it's for a
select
expression, and only through aselect
expression (so the underlying names don't matter much)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I asked a bit around whether there are existing terms or concepts to this problem.
One interesting note was that it could be a modled as a priority queue with dynamic weights. The weight would denote the readiness state, the queue would hold the operations and pop once. We would need then need to implement a constrained pop that blocks until an element with a high enough weight is available. But all of this doesn't help much with the name for the containers in the queue.
The other note I received was that OCaml calls this "alternatives", or its formal parent CSP calls it "choice". https://en.wikipedia.org/wiki/Communicating_sequential_processes#Algebraic_operators
I think that could actually work here as a generalized construct,
Choice.select
,Channel#receive_choice
,Channel#send_choice
,Choice#ready?
,Choice#execute
,Choice#wait
,Choice#unwait
.I'm not sure I follow you on the runtime internals,
Choice
would define the the interface with how the operations have to behave, things like "wait
should ensure the current fiber is rescheduled when value ofready?
changes" and "unwait
should remove any reschedule setup bywait
". We then just have to documentScheduler.enqueue
and that's it, the rest is up to the implementer.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good, but I prefer to wait until we have true parallelism. I don't think any action could be put inside a
select
, there will be mutexes and locks involved and it will be highly coupled with how the runtime works. I'm not sure there will be many extensions to this, we just did it this way so we could more easily implement all the operations.In any case, this will probably evolve and change in the future until we stabilize everything in 1.0. But I'm convinced that using these things outside a
select
expression makes little sense, so the real names aren't very important, and if they are calledSelectAction
then it's pretty clear it's for them to be used in aselect
.