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.

Sign up for GitHub

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

Jump to bottom

Memoize #394

Merged
merged 17 commits into from
Jul 24, 2024
Merged

Memoize #394

merged 17 commits into from
Jul 24, 2024

Conversation

justingrubbs
Copy link
Collaborator

@justingrubbs justingrubbs commented Jul 6, 2024 •
edited
Loading

In this PR, we:

  • Add automatic memoization of most functions
  • Include an example disco file, example/fib.disco, that demonstrates this automatic memoization as well as some of its limits

@justingrubbs justingrubbs requested a review from byorgey July 6, 2024 18:56
@restyled-io restyled-io bot mentioned this pull request Jul 6, 2024
Merged
byorgey
byorgey reviewed Jul 6, 2024
View reviewed changes
example/rsa.disco Outdated Show resolved Hide resolved
src/Disco/AST/Core.hs Outdated Show resolved Hide resolved
src/Disco/Compile.hs Outdated Show resolved Hide resolved
src/Disco/Compile.hs Outdated Show resolved Hide resolved
src/Disco/Compile.hs Outdated Show resolved Hide resolved
disco.cabal Outdated Show resolved Hide resolved
src/Disco/Compile.hs Outdated Show resolved Hide resolved
byorgey
byorgey reviewed Jul 10, 2024
View reviewed changes
Copy link
Member

@byorgey byorgey left a comment

Choose a reason for hiding this comment

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

Fantastic work! Just a few more minor things to address.

example/fib.disco Outdated Show resolved Hide resolved
src/Disco/Compile.hs Outdated Show resolved Hide resolved
src/Disco/Compile.hs Outdated Show resolved Hide resolved
src/Disco/Compile.hs Outdated Show resolved Hide resolved
src/Disco/Interpret/CESK.hs Outdated Show resolved Hide resolved
src/Disco/Interpret/CESK.hs Outdated Show resolved Hide resolved
@byorgey
Copy link
Member

byorgey commented Jul 10, 2024

Also, be sure to update the PR description to accurately describe the changes here. The PR description will turn into the commit description once we merge this.

@byorgey
Copy link
Member

byorgey commented Jul 13, 2024

For the Restyled check, go to #395 and click the "Squash + merge" button.

restyled-io bot and others added 2 commits July 13, 2024 16:55
@restyled-io @restyled-commits
Restyle Memoize (#395)
3f10e6a 
* Restyled by fourmolu

* Restyled by hlint

---------

Co-authored-by: Restyled.io <[email protected]>
@justingrubbs
Fixing restyled edit
3b52052
@restyled-io restyled-io bot mentioned this pull request Jul 13, 2024
Closed
@justingrubbs justingrubbs marked this pull request as ready for review July 19, 2024 16:46
byorgey
byorgey approved these changes Jul 23, 2024
View reviewed changes
Copy link
Member

@byorgey byorgey left a comment

Choose a reason for hiding this comment

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

Looks great!

@justingrubbs justingrubbs added the merge me Trigger mergify bot label Jul 24, 2024
@mergify mergify bot merged commit 3dd8b2f into main Jul 24, 2024
10 checks passed
@mergify mergify bot deleted the memoize branch July 24, 2024 16:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@byorgey byorgey byorgey approved these changes

Assignees
No one assigned
Labels
merge me Trigger mergify bot
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@justingrubbs @byorgey