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

Lazy map operator #245

Merged
merged 4 commits into from
Dec 18, 2020
Merged

Lazy map operator #245

merged 4 commits into from
Dec 18, 2020

Conversation

yannham
Copy link
Member

@yannham yannham commented Dec 8, 2020

Depend on #243. For some reason the map primitive operation on list was not lazy in its first argument, f, which is unjustified. This PRs fixes another small issue at the same time, which was that map and mapRec (renamed recordMap on the way) created lists and records respectively which content was not directly put into a thunk (closurized). This means that expression had to be recomputed multiple times in a call-by-name way, and was also responsible of serialization errors.

@yannham yannham changed the title Lazy map operators Lazy map operator Dec 8, 2020
@aspiwack
Copy link
Member

aspiwack commented Dec 9, 2020

Frankly, this whole laziness thing is a bit of a distraction. We don't have to care too much about laziness. And I assume that we will be revisiting the semantics in more details soon (such as for #103 ).

Copy link
Member

@aspiwack aspiwack left a comment

Choose a reason for hiding this comment

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

Anyway, what's done is done. And it doesn't make things worse. So let's merge.

Comment on lines +400 to +402
// List elements are closurized to preserve lazyness of data structures. It
// maintains the invariant that any data structure only contain thunks (that is,
// currently, variables).
Copy link
Member

Choose a reason for hiding this comment

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

For instance, I think this is not a particularly useful invariant. But, again, let's revisit later.

Copy link
Member Author

@yannham yannham Dec 9, 2020

Choose a reason for hiding this comment

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

I agree that this may not very important in general, but currently, this allows for simple serialization: because this invariant is enforced, when you deepSeq a term, you are guaranteed to only encounter values or variables. Hence the serializer can just evaluate let x = stuff in deepSeq x x, walk the AST of the evaluated term and serialize basic values. Here, let x = list.map (fun x => x + 1) [1,2,3]) in deepSeq x x would leave thunks of the form %somevar + 1 inside, on which the serializer choked with errors, so very concretely, you couldn't export a term with a lists.map, which was one of the (a bit orthogonal to map's laziness) motivation behind this PR. Sorry, I should have made this clear.

@yannham yannham force-pushed the task/primitive-operators-representation branch from e1cb155 to 9fb8df7 Compare December 9, 2020 14:40
Base automatically changed from task/primitive-operators-representation to master December 18, 2020 10:52
@dpulls
Copy link

dpulls bot commented Dec 18, 2020

🎉 All dependencies have been resolved !

@yannham yannham merged commit 61c2ab8 into master Dec 18, 2020
@yannham yannham deleted the task/lazy-lists-map branch December 18, 2020 10:52
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.

None yet

2 participants