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

viper: associate the first exhale with the async expression #3524

Merged
merged 3 commits into from
Nov 18, 2022
Merged

Conversation

ggreif
Copy link
Contributor

@ggreif ggreif commented Oct 27, 2022

This changes the srcloc of certain Viper subexpression to better track the original Motoko.

!!(SeqnS (
let (!!) p = !!! at p in
!!([],
!@(ExhaleS (!@(AndE(!@(MacroCall("$Perm", self ctxt at)),
Copy link
Contributor Author

@ggreif ggreif Oct 27, 2022

Choose a reason for hiding this comment

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

This is the relevant change, sorry for the other noise...

(self ctxt Source.no_region, !!id),
(!!(AddE(!!(FldAcc (self ctxt (s.at), !!id)),
intLitE Source.no_region 1)))));
!@(ExhaleS (!@(AndE(!@(MacroCall("$Perm", self ctxt at)),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the relevant change, sorry for the other noise...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

!! still marks up the await, but !@ points at the async.

Copy link
Contributor

Choose a reason for hiding this comment

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

Right but doesn't the exhale belong to the await, not the async? This is when you need to establish the invariant before suspending. That was my thinking at least. But I don't know why it doesn't highlight the await rather than default to the unknow region.

Copy link
Contributor Author

@ggreif ggreif Oct 27, 2022

Choose a reason for hiding this comment

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

I think the exhale is due to the async being filed into the continuation table. But you are right, that only the await opens up the context switch so that a havoc can occur. I.e. a totally unrelated async could be it too.

@ggreif ggreif requested a review from crusso October 27, 2022 09:17
Copy link
Contributor

@crusso crusso left a comment

Choose a reason for hiding this comment

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

I'm not sure this is correct, but I'll approve for the sake of the demo. Maybe just add a comment to review this later.

(self ctxt at, !@id),
(!@(SubE(!@(FldAcc (self ctxt at, !@id)),
intLitE at 1)))));
!!! (e.at) (SeqnS stmts);
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this maybe confusing things and should really be !@

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not so sure.

@ggreif
Copy link
Contributor Author

ggreif commented Oct 27, 2022

sake of the demo

Not needed for the demo and I won't merge right now. This can wait.

@ggreif ggreif added automerge-squash When ready, merge (using squash) and removed DO-NOT-MERGE automerge-squash When ready, merge (using squash) labels Nov 18, 2022
@ggreif ggreif merged commit f93555f into viper Nov 18, 2022
@ggreif ggreif deleted the gabor/exha branch November 18, 2022 11:22
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