Skip to content
This repository was archived by the owner on Apr 25, 2024. It is now read-only.

Handle CTerm bottom and top gracefully #621

Merged
merged 18 commits into from
Aug 24, 2023
Merged

Handle CTerm bottom and top gracefully #621

merged 18 commits into from
Aug 24, 2023

Conversation

iFrostizz
Copy link
Contributor

closes runtimeverification/kontrol#22
closes #13

We now check when instantiating a CTerm from_kast if the pattern is bottom or top and shortcut from the split early to return either a CTermBottom or a CTermTop.

@iFrostizz iFrostizz requested a review from ehildenb August 23, 2023 17:14
@iFrostizz iFrostizz self-assigned this Aug 23, 2023
@iFrostizz iFrostizz requested a review from anvacaru August 23, 2023 17:26
@iFrostizz
Copy link
Contributor Author

Actually shouldn't we also do the check in the __init__() ? I was scared that it would duplicate code

@ehildenb
Copy link
Member

This looks good, but we should adjust the various methods in KCFGExplore to also handle the new cases correctly. I think in particular, KCFGExplore.cterm_simplify should return a CTerm now, not a KInner:

def cterm_simplify(self, cterm: CTerm) -> tuple[KInner, tuple[LogEntry, ...]]:

@ehildenb
Copy link
Member

Also, we should possibly remove these failure cases?

if is_top(new_term):

@ehildenb
Copy link
Member

ehildenb commented Aug 23, 2023

Also, we should probably have KProve.prove return a list[CTerm] as well now.

def prove(
(and prove_claim perhaps?), and prove_cterm

So for example, prove returns a KInner, which could be a #Or(...) of a bunch of states. So you could return:

return [CTerm.from_kast(disjunct) for disjunct in flatten_label('#Or', final_state)]

@ehildenb
Copy link
Member

This is also a big change, so you need to check that it will work with KEVM locally:

  • Make a new branch of KEVM, link it to this branch of pyk.
  • Try running the proof that was triggering the original bug with this branch of pyk.
  • Add any fixes needed to KEVM to make it work with this branch.
  • Link to that new branch here.

@iFrostizz iFrostizz requested a review from ehildenb August 24, 2023 10:08
@iFrostizz
Copy link
Contributor Author

Trying to run the problematic code on this branch !

@ehildenb
Copy link
Member

Looks great, thank you for the clean focused change! Once you have confirmed that this will work with KEVM, fele free to add automerge.

@iFrostizz
Copy link
Contributor Author

@ehildenb I had to move away from the inheritance CTerm type matching because it wasn't playing well with instantiating the CTerm using the __init__ function. Also added a try_cell because top and bottom CTerm don't have those, so the interfacing code can just try to fetch the cell if it's not None. And it's now fixed on kevm! Asking again for review please

@iFrostizz iFrostizz requested a review from ehildenb August 24, 2023 19:48
@iFrostizz
Copy link
Contributor Author

runtimeverification/evm-semantics@master...kevm-graceful-res#diff-8ef99db40b2010c0b478f54603fda2b277c332347b8945df0e83bb686a6eafc1

As a bottom or top node does not have any cell, in the kcfg it looks like so:
image

@rv-jenkins rv-jenkins merged commit fe10470 into master Aug 24, 2023
@rv-jenkins rv-jenkins deleted the graceful-res branch August 24, 2023 22:43
@iFrostizz iFrostizz restored the graceful-res branch August 25, 2023 15:02
Baltoli pushed a commit to runtimeverification/k that referenced this pull request Apr 9, 2024
closes runtimeverification/kontrol#22
closes runtimeverification/pyk#13

We now check when instantiating a `CTerm` `from_kast` if the pattern is
bottom or top and shortcut from the split early to return either a
`CTermBottom` or a `CTermTop`.

---------

Co-authored-by: devops <[email protected]>
Co-authored-by: François Guyot <[email protected]>
Baltoli pushed a commit to runtimeverification/k that referenced this pull request Apr 9, 2024
closes runtimeverification/kontrol#22
closes runtimeverification/pyk#13

We now check when instantiating a `CTerm` `from_kast` if the pattern is
bottom or top and shortcut from the split early to return either a
`CTermBottom` or a `CTermTop`.

---------

Co-authored-by: devops <[email protected]>
Co-authored-by: François Guyot <[email protected]>
Baltoli pushed a commit to runtimeverification/k that referenced this pull request Apr 10, 2024
closes runtimeverification/kontrol#22
closes runtimeverification/pyk#13

We now check when instantiating a `CTerm` `from_kast` if the pattern is
bottom or top and shortcut from the split early to return either a
`CTermBottom` or a `CTermTop`.

---------

Co-authored-by: devops <[email protected]>
Co-authored-by: François Guyot <[email protected]>
Baltoli pushed a commit to runtimeverification/k that referenced this pull request Apr 10, 2024
closes runtimeverification/kontrol#22
closes runtimeverification/pyk#13

We now check when instantiating a `CTerm` `from_kast` if the pattern is
bottom or top and shortcut from the split early to return either a
`CTermBottom` or a `CTermTop`.

---------

Co-authored-by: devops <[email protected]>
Co-authored-by: François Guyot <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Backend responding with #Bottom makes the proof crash CTerm(mlTop()) should be handled gracefully
4 participants