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

Fix #1159 #1246

Merged
merged 1 commit into from
Mar 18, 2022
Merged

Fix #1159 #1246

merged 1 commit into from
Mar 18, 2022

Conversation

mario-bucev
Copy link
Collaborator

@mario-bucev mario-bucev commented Mar 15, 2022

Following the suggestion #1160 (comment)

Comment on lines 259 to 263
case Old(v: Variable) if freshVarDecls.contains(v) =>
Some(freshVars(freshVarDecls.indexOf(v)))
Copy link
Member

Choose a reason for hiding this comment

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

Ideally, this should be more restrictive and only apply within the postcondition to allow the ImperativeCleanup phase to correctly catch all incorrect uses of old. Could you maybe make this part of the extraVarReplace (e.g. by making it into a Map[Expr, Expr])?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good point!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

(I'll push an updated version)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Actually, the Old(v) expression will already have its variable rewritten to its freshened variant (contained in freshVarDecls). I pushed a version that is simply passed a flag to tell us whether we should rewrite old expressions (less elegant, that's for sure...).

@vkuncak
Copy link
Collaborator

vkuncak commented Mar 18, 2022

@samarion again it's marked "approved". Did you mean to just comment without clicking "approve"?

@vkuncak vkuncak merged commit 8064e2e into epfl-lara:main Mar 18, 2022
@mario-bucev mario-bucev deleted the fix-i1159 branch June 8, 2022 14:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants