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.

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

strip VResult before storing value in REPLDone #978

Merged
merged 3 commits into from
Jan 8, 2023
Merged

Conversation

byorgey
Copy link
Member

@byorgey byorgey commented Jan 7, 2023

Fixes #977.

-- Log exceptions in the message queue so we can check for them in tests
Left exn -> do
let h = hypotheticalRobot (Out VUnit emptyStore []) 0
em <- use entityMap
m <- evalState @Robot h $ createLogEntry ErrorTrace (formatExn em exn)
emitMessage m
Right (VBool res) -> when res markWin
Right (VResult (VBool res) _env) -> when res markWin
Copy link
Member

@kostmo kostmo Jan 7, 2023

Choose a reason for hiding this comment

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

#927 continues to accrue merge conflicts in this part of the code (see previous comment).

Copy link
Member

Choose a reason for hiding this comment

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

@kostmo if you are OK with letting me push to your branch, I can fix it for you. 😉

We have a historic number of open PRs now, so I will try to close mine and review and help where I can.

Afterwards, we should make a release, there is a lot of cool stuff to share. 👍

@@ -87,6 +88,11 @@ data Value where
VRef :: Int -> Value
deriving (Eq, Show, Generic, FromJSON, ToJSON)

-- | Ensure that a value is not wrapped in 'VResult'.
stripVResult :: Value -> Value
stripVResult (VResult v _) = v
Copy link
Member

Choose a reason for hiding this comment

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

Just to make sure, there is no way a VResult could contain a VResult, right?

Copy link
Member Author

Choose a reason for hiding this comment

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

Uhh... well, it's not supposed to, but it's a good question. We might as well make stripVResult recursive, just in case.

-- Log exceptions in the message queue so we can check for them in tests
Left exn -> do
let h = hypotheticalRobot (Out VUnit emptyStore []) 0
em <- use entityMap
m <- evalState @Robot h $ createLogEntry ErrorTrace (formatExn em exn)
emitMessage m
Right (VBool res) -> when res markWin
Right (VResult (VBool res) _env) -> when res markWin
Copy link
Member

Choose a reason for hiding this comment

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

@kostmo if you are OK with letting me push to your branch, I can fix it for you. 😉

We have a historic number of open PRs now, so I will try to close mine and review and help where I can.

Afterwards, we should make a release, there is a lot of cool stuff to share. 👍

@byorgey byorgey added the merge me Trigger the merge process of the Pull request. label Jan 8, 2023
@mergify mergify bot merged commit 10d281a into main Jan 8, 2023
@mergify mergify bot deleted the fix/issue-977 branch January 8, 2023 20:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge me Trigger the merge process of the Pull request.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fatal error when using itN resulting from a bind
3 participants