CAD-4750 generalise OrElse frame#15
Merged
Merged
Conversation
yogeshsajanikar
requested review from
amesgen,
bartfrenk,
coot,
dcoutts,
dnadales,
jorisdral and
nfrisby
Contributor
Author
|
This pull request |
Contributor
Author
amesgen
suggested changes
Aug 2, 2022
Contributor
amesgen
left a comment
amesgen
left a comment
There was a problem hiding this comment.
Nice simplification! Only a few minor comments.
Note that I did not comment on the IOSimPOR part it is identical to IOSim in this case (which makes sense as this code is not scheduling-related).
nfrisby
approved these changes
Aug 4, 2022
yogeshsajanikar
force-pushed
the
CAD-4750-generalise-orelse-frame
branch
3 times, most recently
from
9683782 to
6bf6856
Compare
Contributor
Author
coot
approved these changes
Aug 17, 2022
Collaborator
coot
left a comment
coot
left a comment
There was a problem hiding this comment.
Very nice, thanks! I'd only would make BranchStmA a strict field (as suggested by @nfrisby): since we are providing another level of abstraction / indirection, making this field strict will eliminate the thunk before IOSim would evaluate it.
Contributor
Author
|
resolves IntersectMBO/ouroboros-network#1461 |
yogeshsajanikar
force-pushed
the
CAD-4750-generalise-orelse-frame
branch
2 times, most recently
from
e38dcf0 to
d329b00
Compare
Contributor
Author
OrElseLeftFrame and OrElseRightFrame represent a control structure that has a alternative branch that is executed when `retry` is applied. A `catch` has a similar execution model when a `throw` is applied. The control frame is generalised to BranchFrame that can hold an alternative statement. If the execution context is `left` side of the branch then the BranchFrame contains `right` statement. When we are executing in the `right` context, the branch frame contains an empty statement.
yogeshsajanikar
force-pushed
the
CAD-4750-generalise-orelse-frame
branch
from
d555c48 to
ccc84f8
Compare
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
OrElse and Catch have similar control semantics. Both have alternate branches that needs to be considered when a certain condition is encountered (retry in OrElse, throw in Catch). This commit tries to generalise OrElseLeftFrame/OrElseRightFrame to handle these control semantics in a similar fashion.
The commit introduces
BranchFramethat indicates that a branch has an alternate execution that is indicated by either a "statement" (OrElseStmA) or an empty statement (to indicate that there is no alternate execution left, this replaces OrElseRightFrame).This should allow adding
Catchto IO-Sim a relatively easily, than introducing a new frame to handle Catch separately.