-
Notifications
You must be signed in to change notification settings - Fork 67
ParAff applicative should abort on uncaught exception #135
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
ParAff applicative should abort on uncaught exception #135
Conversation
@natefaubion Thanks, will review this today! |
r2 ← joinFiber f1 | ||
pure (r1 == "foobar" && r2.a == "foo" && r2.b == "bar") | ||
|
||
test_parallel_throw ∷ ∀ eff. TestAff eff Unit |
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.
Worth adding a test here for parallel a
/ never
.
tmp = true; | ||
kid = killId++; | ||
|
||
kills[kid] = kill(early, fail === lhs ? head._2 : head._1, function (/* unused */) { |
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.
This looks tricky. Don't understand the encoding well enough to say if this is correct.
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.
head
= the current Applicative node in the stackhead._1
= the left-hand-side nodehead._2
= the right-hand-side nodefail
= the resolved exceptionlhs
= the resolved value of the left-hand-side parallel operation (head._1._3
)rhs
= the resolved value of the right-hand-side parallel operation (head._2._3
)
So, if the left-hand-side resolved to a failure, kill the right-hand-side, otherwise kill the left-hand-side.
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 don't think I actually need rhs
anymore, so I can lose that.
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.
Nevermind, it's used in the success branches.
|
Fixes #134