-
Notifications
You must be signed in to change notification settings - Fork 18
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
feat: add automation support for pairwise memory separation #126
Merged
Merged
Changes from 6 commits
Commits
Show all changes
21 commits
Select commit
Hold shift + click to select a range
f55ff94
chore: add defn for List.pairwise mem_separate xs
bollu 3e7ba47
chore: completely untested code for separation
bollu 35f5e6c
chore: don't use decideProof in ways that don't work
bollu 80454da
chore: I don't understand this failure
bollu f2877b9
chore: add separate example
bollu 13e12ef
chore: add examples for pairwise
bollu 16ec26c
chore: show example where better address normalization is needed
bollu f9d879a
chore: cleanup
bollu a8a8f64
chore: touchup
bollu 75c3435
metaprogramming nits
bollu d71b7b4
feat: cleanup
bollu d1facdd
Merge branch 'main' into mem-pairwise
shigoel 56d5154
Update Proofs/Experiments/MemoryAliasing.lean
bollu d38e1a5
chore: scope trace enablement
bollu 743c4e3
Merge remote-tracking branch 'origin/mem-pairwise' into mem-pairwise
bollu 34ded47
chore: simp only
bollu dd24b6d
chore: use custom simp set
bollu 46c2947
Merge branch 'main' into mem-pairwise
shigoel 194b745
Merge branch 'main' into mem-pairwise
shigoel 67618c4
Merge branch 'main' into mem-pairwise
shigoel 2bf5368
Merge branch 'main' into mem-pairwise
shigoel File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bollu I believe
LNSymSimpContext
can work here. Fair warning: haven't tried out the suggestion, excuse typos, etc.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, open to making
LNSymSimpContext
nicer (separate issue).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@shigoel I think I would prefer to keep it as-is, because it's hard to see what
LNSymSimpContext
builds unless one goes and reads its code. Since this code is meant to be performant, I would rather have the building of the simp-set inline, so it's easy to audit.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Weren't you using
LNSymSimpContext
before though?dd24b6d
Not a big deal, but I want to make sure I understand what the reasons are.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed, I was reusing
LNSymSimpContext
to make sure that ifLNSymSimpContext
got more powerful, I would get the same power for free :) But thinking this through, I found your argument for why this should not be expensive to be compelling. Therefore, I'd prefer to build a custom simp-set that's tuned down, and to add power to it as necessary.From an engineering perspective, I'm advocating for [WET / write everything twice] (https://en.wikipedia.org/wiki/Rule_of_three_(computer_programming)) instead of DRY. 😉
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Gotcha, though I would note that the particular invocation of
LNSymSimpContext
is equivalent to what you wrote inline.