Skip to content

Conversation

facundominguez
Copy link
Member

No description provided.

@mboes
Copy link
Member

mboes commented Dec 19, 2017

I propose the following invariant instead:

Invariant: the argument and the result are distinct. That is, the result and the argument share no direct JVM object references.

-- reifying induces allocations and copies.
class Interpretation a => Reify a where
-- | Invariant: the argument and the result are distinct. That is, the result
-- and the argument share no direct JVM object references.
Copy link
Member Author

Choose a reason for hiding this comment

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

This is the only relevant condition:

The result and the argument share no direct JVM object references.

The values being distinct does not imply it.

Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure I get your point. I intended the second sentence as the definition of the word "distinct" in the first sentence. Said definition doesn't mention values. We could choose to remove introducing a definition entirely.

Copy link
Member Author

Choose a reason for hiding this comment

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

I see. I find the meaning of distinct surprising here. It would be best to avoid it as you suggest.

jvm/CHANGELOG.md Outdated
* The `Reify`/`Reflect` instances for `()` is now mapped to
a *serializable* small JVM object. This is a more useful instance
for sparkle users.
* `Reify`/`Reflect` were given additional pre and post conditions.
Copy link
Member Author

Choose a reason for hiding this comment

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

This should be reformulated a bit.

@mboes mboes merged commit 16d564c into master Dec 19, 2017
@mboes mboes deleted the fd/fixrrlists branch December 19, 2017 17:55
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.

2 participants