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

Heapster bugfix to update IRT wrt recent changes #1441

Merged
merged 4 commits into from
Sep 3, 2021

Conversation

eddywestbrook
Copy link
Contributor

PR #1422 simplified the translation of permissions of the form exists x:tp. eq(e) so that, instead of generating a dependent pair type Sigma tp (\x -> unit) with a trivial second projection, these are now just translated directly to (the translation of) the type tp of the existential variable. Unfortunately, that PR forgot to update the IRT translations, so that any recursive permission defined using IRTs that has, say, a bitvector value in it is translated incorrectly and generates a Coq error.

…made the translation of bitvector permissions to just be bitvectors
@eddywestbrook eddywestbrook added the Heapster Issues specifically related to the Heapster sub-system label Aug 31, 2021
Copy link
Contributor

@m-yac m-yac left a comment

Choose a reason for hiding this comment

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

Looks good to me. Sorry for missing this in my PR about exists permissions. Do you know if that was because I overlooked a test, or was it because we didn't have any IRT tests which would've caught this?

Looks like this depends on #1440, so it should not be merged until the former is.

@eddywestbrook eddywestbrook added the ready-to-merge If at least one person approves this PR, automatically merge it when CI finishes successfully. label Sep 3, 2021
@eddywestbrook eddywestbrook merged commit e1bc957 into master Sep 3, 2021
@mergify mergify bot deleted the heapster-bugfix-bitvector-irts branch September 3, 2021 01:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Heapster Issues specifically related to the Heapster sub-system ready-to-merge If at least one person approves this PR, automatically merge it when CI finishes successfully.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants