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

[Merged by Bors] - feat: make ListM safe #3559

Closed
wants to merge 2 commits into from
Closed

[Merged by Bors] - feat: make ListM safe #3559

wants to merge 2 commits into from

Conversation

gebner
Copy link
Member

@gebner gebner commented Apr 21, 2023

Hide the unsafe inductive powering ListM behind a safe API using implemented_by.


The motivation is of course Scott's recent barrage of library_search improvements using ListM which marks everything as unsafe. I'd really like to stop the proliferation of unsafe, so that unsafe continues to be a meaningful indicator of memory unsafety or logical unsoundness.

Open in Gitpod

@gebner gebner added awaiting-review The author would like community review of the PR t-meta Tactics, attributes or user commands labels Apr 21, 2023
#check (rfl : ((ListM.fix F 10).takeAsList 4).run [] = some ([10, 9, 8, 7], [7, 8, 9, 10]))
#check (rfl :
(((ListM.fix F 10).map fun n => n*n).takeAsList 3).run [] = some ([100, 81, 64], [8, 9, 10]))
example : ((ListM.fix F 10).takeAsList 4).run [] = some ([10, 9, 8, 7], [8, 9, 10]) := by
Copy link
Member Author

Choose a reason for hiding this comment

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

There are two changes here:

  1. rfl no longer unfolds ListM operations because they're partial and/or opaque. (But the rfl was unsafe anyhow.)
  2. I optimized ListM.fix so that it requires fewer invocations of F to produce the same number of elements. (That's why the second part of the pair---which stores each invocation of F---got shorter.)

@semorrison
Copy link
Contributor

Oh my! This is amazing, thank you.

bors merge

@github-actions github-actions bot added ready-to-merge This PR has been sent to bors. and removed awaiting-review The author would like community review of the PR labels Apr 21, 2023
bors bot pushed a commit that referenced this pull request Apr 21, 2023
Hide the unsafe inductive powering `ListM` behind a safe API using `implemented_by`.
@bors
Copy link

bors bot commented Apr 21, 2023

Pull request successfully merged into master.

Build succeeded:

@bors bors bot changed the title feat: make ListM safe [Merged by Bors] - feat: make ListM safe Apr 21, 2023
@bors bors bot closed this Apr 21, 2023
@bors bors bot deleted the safeliftm branch April 21, 2023 04:27
kbuzzard pushed a commit that referenced this pull request Apr 22, 2023
Hide the unsafe inductive powering `ListM` behind a safe API using `implemented_by`.
semorrison pushed a commit that referenced this pull request May 10, 2023
Hide the unsafe inductive powering `ListM` behind a safe API using `implemented_by`.
hrmacbeth pushed a commit that referenced this pull request May 10, 2023
Hide the unsafe inductive powering `ListM` behind a safe API using `implemented_by`.
hrmacbeth pushed a commit that referenced this pull request May 11, 2023
Hide the unsafe inductive powering `ListM` behind a safe API using `implemented_by`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-to-merge This PR has been sent to bors. t-meta Tactics, attributes or user commands
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants