Skip to content

Conversation

@garyb
Copy link
Member

@garyb garyb commented Jun 12, 2014

paf31 added a commit that referenced this pull request Jun 13, 2014
@paf31 paf31 merged commit 3d3a414 into master Jun 13, 2014
@paf31 paf31 deleted the unit branch June 13, 2014 08:50
Copy link
Contributor

Choose a reason for hiding this comment

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

One very slight issue here: these various FFI calls return {}, which doesn't have the runtime representation of the type Unit, so pattern matching on the result, for example, while not necessarily useful, could result in a runtime error. To be super safe, these things should probably return Prelude.unit.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I did have a slight worry about that. I checked the generated code for Eq, Ord, things like that and none of them attempt to read from the value so I think at least in PS code it should be fine. I also figure this'll be fixed when we get newtypes working.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok, so this is a problem actually when compiled with no optimizations. One fix is to not pattern match in the Prelude definitions, just use _ instead of (Unit {}). I've updated the Prelude in ps-in-ps to have the definitions this way so it's not broken in psci.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants