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

Add foundry-merge-nodes option #1934

Merged
merged 85 commits into from
Aug 19, 2023
Merged

Add foundry-merge-nodes option #1934

merged 85 commits into from
Aug 19, 2023

Conversation

nwatson22
Copy link
Member

@nwatson22 nwatson22 commented Jun 30, 2023

Adds an option foundry-merge-nodes used with:

kevm foundry-merge-nodes [Contract.test_name] --node [node id] --node [node id]

which creates a new node which is the anti_unify_with_constraints of all the listed nodes and makes this node a cover of all the listed nodes.
Optionally, include the --include-disjunct flag to keep the abstracted nodes as an orBool disjunction, e.g.

<k> 1 </k>

merged with

<k> 2 </k>

would yield:

<k> N:Int </k>
#And (N ==K 1 orBool N ==K 2)

Additionally, the PR changes foundry-remove-node so that it no longer prunes the target node if it is a child of the removed node, and fixes foundry-step-node, foundry-section-edge, foundry-remove-node, foundry-list and foundry-to-dot, which were assuming that the proof being operated on was an APRProof and not an APRBMCProof, so they converted the proof back to an APRProof.

@nwatson22 nwatson22 self-assigned this Jun 30, 2023
@nwatson22 nwatson22 linked an issue Jun 30, 2023 that may be closed by this pull request
@ehildenb
Copy link
Member

Let's reduce whitespace a bit, no need to define some of the intermediates (eg. intermeditae for new_cterm, can just directly create it an insert it as a node). Also, instead of naming the variables apr_proof, I've been trying to switch to just the more compact proof name everywhere.

@ehildenb
Copy link
Member

ehildenb commented Jun 30, 2023

We should also be putting a sanity check on the nodes the user has asked to merge. Specifically, they all must:

  • Have the exact same content in the following cells: <k>, <program>, <pc>, <callDepth>. Otherwise, throw an error.

@nwatson22 nwatson22 requested a review from ehildenb August 17, 2023 23:50
@rv-jenkins rv-jenkins merged commit f01c196 into master Aug 19, 2023
@rv-jenkins rv-jenkins deleted the noah/merge-branches branch August 19, 2023 00:31
iFrostizz pushed a commit that referenced this pull request Aug 28, 2023
* Add foundry-merge-nodes option

* Set Version: 1.0.225

* Anti-unify without disjunct

* Update kevm-pyk/src/kevm_pyk/foundry.py

Co-authored-by: Everett Hildenbrandt <everett.hildenbrandt@gmail.com>

* Update kevm-pyk/src/kevm_pyk/foundry.py

Co-authored-by: Everett Hildenbrandt <everett.hildenbrandt@gmail.com>

* Fix formatting

* Rename apr_proof to proof, reduce whitespace

* Disallow merging nodes if they do not match on <k>, <program>, <pc>, or <callDepth>

* Improve variable naming

* Add test for branch merging

* Update expected contracts.k

* Set Version: 1.0.226

* Set Version: 1.0.226

* Update expected output

* Set Version: 1.0.227

* Update test_foundry_fail expected files

* Keep disjunction of concrete values

* Switch to using associated pyk branch

* Set Version: 1.0.228

* Fix foundry_get_apr_proof and foundry_step_node to be compatible with APR and APRBMC proofs

* Fix various commands to work with APRBMCProof

* Update kevm-pyk/src/kevm_pyk/__main__.py

* Update kevm-pyk/src/kevm_pyk/foundry.py

* Update foundry_merge_nodes test

* Fix formatting

* Update expected files

* Set Version: 1.0.229

* Set Version: 1.0.231

* Set Version: 1.0.232

* Set Version: 1.0.235

* Set Version: 1.0.236

* Revert poetry.lock

* Fix setUp method execution being limited by max_iterations

* Set Version: 1.0.239

* Set Version: 1.0.244

* Set Version: 1.0.249

* Set Version: 1.0.250

* Revert foundry_get_proof and foundry_get_apr_proof changes as they have been moved to another branch

* Fix foundry_list proof_data_exists call

* Use digest, not just method name for looking up setUp method

* Revert change to proof loading path

* Use foundry.get_apr_proof in foundry_merge_nodes

* Set Version: 1.0.251

* Use foundry.get_apr_proof in test

* Set Version: 1.0.252

* Set Version: 1.0.262

* Use new anti_unification function

* Set Version: 1.0.263

* Set Version: 1.0.264

* Include kdefinition in anti_unify call and fix test

* Re-add foundry-merge-nodes command

* Set Version: 1.0.265

* Set Version: 1.0.265

* Set Version: 1.0.266

* Set Version: 1.0.267

---------

Co-authored-by: devops <devops@runtimeverification.com>
Co-authored-by: Everett Hildenbrandt <everett.hildenbrandt@gmail.com>
Baltoli pushed a commit to runtimeverification/k that referenced this pull request Apr 9, 2024
Needed for
runtimeverification/evm-semantics#1934.
Adds an additional option to `anti_unify_with_constraints` which adds an
additional constraint which is an `_orBool_` of the two anti-unifying
substitutions.
Adds a test for the method with this option which tests a new variable
is created to abstract and the `orBool` constraint is added as
described.

---------

Co-authored-by: devops <devops@runtimeverification.com>
Baltoli pushed a commit to runtimeverification/k that referenced this pull request Apr 9, 2024
- Changed `anti_unify` and `anti_unify_with_constraints` to require a
`KDefinition` which they use to grab the tightest sort for the terms
they are abstracting
- Since these actions are now associated with a specific K definition,
moved them into `KDefinition` as methods.
- Adds `KDefinition.least_common_supersort` which returns the most
specific sort that will cover two terms (as long as one sort is a
subsort of the other).
- Uses this function in `KDefinition.sort` to determine the sort of a
`KRewrite`.
- Rewrote the tests for `anti_unify` and `anti_unify_with_constraints`
to use `KDefinition` and test that the abstracted variables are now more
tightly typed based on the original terms.

This is an attempt to fix an issue in
runtimeverification/evm-semantics#1934 where the
looser sort bound is preventing simplification after the merging
(anti-unification) of two nodes.

---------

Co-authored-by: devops <devops@runtimeverification.com>
Baltoli pushed a commit to runtimeverification/k that referenced this pull request Apr 9, 2024
Needed for
runtimeverification/evm-semantics#1934.
Adds an additional option to `anti_unify_with_constraints` which adds an
additional constraint which is an `_orBool_` of the two anti-unifying
substitutions.
Adds a test for the method with this option which tests a new variable
is created to abstract and the `orBool` constraint is added as
described.

---------

Co-authored-by: devops <devops@runtimeverification.com>
Baltoli pushed a commit to runtimeverification/k that referenced this pull request Apr 9, 2024
- Changed `anti_unify` and `anti_unify_with_constraints` to require a
`KDefinition` which they use to grab the tightest sort for the terms
they are abstracting
- Since these actions are now associated with a specific K definition,
moved them into `KDefinition` as methods.
- Adds `KDefinition.least_common_supersort` which returns the most
specific sort that will cover two terms (as long as one sort is a
subsort of the other).
- Uses this function in `KDefinition.sort` to determine the sort of a
`KRewrite`.
- Rewrote the tests for `anti_unify` and `anti_unify_with_constraints`
to use `KDefinition` and test that the abstracted variables are now more
tightly typed based on the original terms.

This is an attempt to fix an issue in
runtimeverification/evm-semantics#1934 where the
looser sort bound is preventing simplification after the merging
(anti-unification) of two nodes.

---------

Co-authored-by: devops <devops@runtimeverification.com>
Baltoli pushed a commit to runtimeverification/k that referenced this pull request Apr 9, 2024
Needed for
runtimeverification/evm-semantics#1934.
Adds an additional option to `anti_unify_with_constraints` which adds an
additional constraint which is an `_orBool_` of the two anti-unifying
substitutions.
Adds a test for the method with this option which tests a new variable
is created to abstract and the `orBool` constraint is added as
described.

---------

Co-authored-by: devops <devops@runtimeverification.com>
Baltoli pushed a commit to runtimeverification/k that referenced this pull request Apr 9, 2024
- Changed `anti_unify` and `anti_unify_with_constraints` to require a
`KDefinition` which they use to grab the tightest sort for the terms
they are abstracting
- Since these actions are now associated with a specific K definition,
moved them into `KDefinition` as methods.
- Adds `KDefinition.least_common_supersort` which returns the most
specific sort that will cover two terms (as long as one sort is a
subsort of the other).
- Uses this function in `KDefinition.sort` to determine the sort of a
`KRewrite`.
- Rewrote the tests for `anti_unify` and `anti_unify_with_constraints`
to use `KDefinition` and test that the abstracted variables are now more
tightly typed based on the original terms.

This is an attempt to fix an issue in
runtimeverification/evm-semantics#1934 where the
looser sort bound is preventing simplification after the merging
(anti-unification) of two nodes.

---------

Co-authored-by: devops <devops@runtimeverification.com>
Baltoli pushed a commit to runtimeverification/k that referenced this pull request Apr 10, 2024
Needed for
runtimeverification/evm-semantics#1934.
Adds an additional option to `anti_unify_with_constraints` which adds an
additional constraint which is an `_orBool_` of the two anti-unifying
substitutions.
Adds a test for the method with this option which tests a new variable
is created to abstract and the `orBool` constraint is added as
described.

---------

Co-authored-by: devops <devops@runtimeverification.com>
Baltoli pushed a commit to runtimeverification/k that referenced this pull request Apr 10, 2024
- Changed `anti_unify` and `anti_unify_with_constraints` to require a
`KDefinition` which they use to grab the tightest sort for the terms
they are abstracting
- Since these actions are now associated with a specific K definition,
moved them into `KDefinition` as methods.
- Adds `KDefinition.least_common_supersort` which returns the most
specific sort that will cover two terms (as long as one sort is a
subsort of the other).
- Uses this function in `KDefinition.sort` to determine the sort of a
`KRewrite`.
- Rewrote the tests for `anti_unify` and `anti_unify_with_constraints`
to use `KDefinition` and test that the abstracted variables are now more
tightly typed based on the original terms.

This is an attempt to fix an issue in
runtimeverification/evm-semantics#1934 where the
looser sort bound is preventing simplification after the merging
(anti-unification) of two nodes.

---------

Co-authored-by: devops <devops@runtimeverification.com>
Baltoli pushed a commit to runtimeverification/k that referenced this pull request Apr 10, 2024
Needed for
runtimeverification/evm-semantics#1934.
Adds an additional option to `anti_unify_with_constraints` which adds an
additional constraint which is an `_orBool_` of the two anti-unifying
substitutions.
Adds a test for the method with this option which tests a new variable
is created to abstract and the `orBool` constraint is added as
described.

---------

Co-authored-by: devops <devops@runtimeverification.com>
Baltoli pushed a commit to runtimeverification/k that referenced this pull request Apr 10, 2024
- Changed `anti_unify` and `anti_unify_with_constraints` to require a
`KDefinition` which they use to grab the tightest sort for the terms
they are abstracting
- Since these actions are now associated with a specific K definition,
moved them into `KDefinition` as methods.
- Adds `KDefinition.least_common_supersort` which returns the most
specific sort that will cover two terms (as long as one sort is a
subsort of the other).
- Uses this function in `KDefinition.sort` to determine the sort of a
`KRewrite`.
- Rewrote the tests for `anti_unify` and `anti_unify_with_constraints`
to use `KDefinition` and test that the abstracted variables are now more
tightly typed based on the original terms.

This is an attempt to fix an issue in
runtimeverification/evm-semantics#1934 where the
looser sort bound is preventing simplification after the merging
(anti-unification) of two nodes.

---------

Co-authored-by: devops <devops@runtimeverification.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge engagement Related to an ongoing engagement high priority
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow merging branches back after a split
4 participants