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

Added a README describing the Heapster implication rules #1554

Merged
merged 23 commits into from
Jun 14, 2022

Conversation

eddywestbrook
Copy link
Contributor

No description provided.

@eddywestbrook eddywestbrook requested a review from jpaykin January 11, 2022 22:54
@eddywestbrook eddywestbrook added subsystem: heapster Issues specifically related to memory verification using Heapster PR: ready to merge Magic flag for pull requests to ask Mergify to merge given an approval and a successful CI run labels Jan 11, 2022
Copy link
Contributor

@jpaykin jpaykin 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! This is honestly very useful. I just pointed out one spot that could be cleaned up.


The main logic for proving most of the permission constructs is in the next function down, `proveVarImplInt`, which proves a single permission on a variable `x`. It does this by first pushing the primary permsisions `p` for `x` onto the top of the stack and then proving an implication `x:p |- x:mb_p`, where `mb_p` is the desired permission that we want to prove for `x`. The "mb" prefix indicates that `mb_p` is a permission inside a multi-binding, i.e., a binding of 0 or more existential variables that could be used in the permission we are trying to prove. The `proveVarImplInt` then proceeds by case analysis on `p` and `mb_p`, in most cases using either an introduction rule for proving a construct in `mb_p` on the right or an elimination rule for eliminating a construct in `p` on the left combined with a recursive call to `proveVarImplInt` to prove the remaining implication for smaller `p` and/or smaller `mb_p`. The most complex cases are for proving an equality permission on the right or for proving an implication `x:p1 * ... * pn |- x:p1' * ... * pm'` between conjuncts permissions. For an equality permission on the right, the special-purpose helper function `proveVarEq` is used to prove equality permissions `x:eq(e)` based on the structure of `e`. For proving an implication of conjuncts, the function `proveVarConjImpl` is used, which is structured in a similar manner to `proveVarsImplAppendInt`, in that it repeatedly chooses the "best" permission on the right to prove, proves it, and then recursively proves the remaining permissions on the right until they have all been proved.

The `proveVarAtomicImpl` function is called by `proveVarConjImpl` to prove each conjunct. This function performs
Copy link
Contributor

Choose a reason for hiding this comment

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

This and the next few paragraphs have cut-off sentences. It's fine that this is work in progress, I would just remove the sentence fragments an put TODO's or FIXME's there if you need to remember that spot.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Whoops, you're right! Need to fix that...

@eddywestbrook eddywestbrook removed the PR: ready to merge Magic flag for pull requests to ask Mergify to merge given an approval and a successful CI run label Jan 11, 2022
@eddywestbrook eddywestbrook added the PR: ready to merge Magic flag for pull requests to ask Mergify to merge given an approval and a successful CI run label Jun 10, 2022
@eddywestbrook
Copy link
Contributor Author

This now includes documentation about the Rust-to-Heapster translation, which is a useful way to help understand the Heapster type system

@eddywestbrook eddywestbrook removed the PR: ready to merge Magic flag for pull requests to ask Mergify to merge given an approval and a successful CI run label Jun 10, 2022
@eddywestbrook eddywestbrook added the PR: ready to merge Magic flag for pull requests to ask Mergify to merge given an approval and a successful CI run label Jun 10, 2022
@eddywestbrook eddywestbrook requested a review from m-yac June 11, 2022 00:26
@mergify mergify bot merged commit 761f535 into master Jun 14, 2022
@mergify mergify bot deleted the heapster/type-system-docs branch June 14, 2022 19:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: ready to merge Magic flag for pull requests to ask Mergify to merge given an approval and a successful CI run subsystem: heapster Issues specifically related to memory verification using Heapster
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants