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

Fully qualify claim labels/dependencies, include dependencies in output #842

Merged
merged 30 commits into from
Jan 31, 2024

Conversation

ehildenb
Copy link
Member

@ehildenb ehildenb commented Jan 26, 2024

We were having trouble in runtimeverification/evm-semantics#2272 selecting the correct claims because they can be unqualified by the user. This PR:

  • Fully qualifies all claim labels and dependency labels returned by KProve.get_claim_modules.
  • Makes sure that we return not only the user-selected claims, but their dependencies (unless the user turns that off), from KProve.get_claims.
  • Adds a test-suite for the claim selection functionality.

There is a bug in this code, and a test added (and skipped using pytest.skip()) which demonstrates a (probably minor) issue where we are not checking that claim dependencies are in a module imported by the current module.

@ehildenb ehildenb self-assigned this Jan 26, 2024
@ehildenb ehildenb changed the title Fully qualify claim labels and dependencies when loading from file Fully qualify claim labels/dependencies, include dependencies in output Jan 26, 2024
@ehildenb ehildenb marked this pull request as ready for review January 26, 2024 00:57
@ehildenb ehildenb requested a review from tothtamas28 January 26, 2024 00:57
@ehildenb
Copy link
Member Author

I'm redoing the logic here completely, I agree that the original way was very fragile.

Copy link
Collaborator

@tothtamas28 tothtamas28 left a comment

Choose a reason for hiding this comment

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

Please add a docstring for the method that specifies how the module names and claim labels in spec_file relate to claim_labels and the claim labels in the result.

@ehildenb ehildenb requested a review from tothtamas28 January 29, 2024 19:03
@ehildenb ehildenb requested a review from nwatson22 January 29, 2024 19:05
@ehildenb
Copy link
Member Author

@tothtamas28 I'm going to get this merged to unblock KEVM stuffs, we can discuss more improvements as follow-up.

Copy link
Contributor

@nwatson22 nwatson22 left a comment

Choose a reason for hiding this comment

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

Is adding a depends with an unqualified label supposed to only work if a claim with that label exists in the current module, or should you be allowed to specify as a dependency an unqualified claim label from an imported module?

@ehildenb
Copy link
Member Author

It should only work in the current module I suppose. But I guess overall it's ill-defined, we don't have a reference or spec for it.

@rv-jenkins rv-jenkins merged commit 14c565a into master Jan 31, 2024
@rv-jenkins rv-jenkins deleted the claim-labels branch January 31, 2024 00:32
rv-jenkins pushed a commit that referenced this pull request Feb 7, 2024
Here: #842, a mistake was
made where claims without labels were filtered out of the claims to be
processed and returned by `KProve.get_claims`. This PR addresses that
and adds tests. In particular:

- Two claims in the `multi-claim-spec.k` which were ill-formed because
of dependencies (but not tested for it) are removed.
- An ill-formed (because of dependencies) claim is added in a separate
module, and a new test added which exercises that a `ValueError` is
thrown when loading this module.
- A new test is added which asserts that the correct number of claims is
loaded from the main module. On `master`, this test is failing because
it skips claims without labels, but is fixed on this PR.
- Adjustments to how claims are filtered are made, so that it's simpler
and includes all claims, even ones without explicit module names in the
label.

---------

Co-authored-by: devops <[email protected]>
Baltoli pushed a commit to runtimeverification/k that referenced this pull request Apr 9, 2024
Here: runtimeverification/pyk#842, a mistake was
made where claims without labels were filtered out of the claims to be
processed and returned by `KProve.get_claims`. This PR addresses that
and adds tests. In particular:

- Two claims in the `multi-claim-spec.k` which were ill-formed because
of dependencies (but not tested for it) are removed.
- An ill-formed (because of dependencies) claim is added in a separate
module, and a new test added which exercises that a `ValueError` is
thrown when loading this module.
- A new test is added which asserts that the correct number of claims is
loaded from the main module. On `master`, this test is failing because
it skips claims without labels, but is fixed on this PR.
- Adjustments to how claims are filtered are made, so that it's simpler
and includes all claims, even ones without explicit module names in the
label.

---------

Co-authored-by: devops <[email protected]>
Baltoli pushed a commit to runtimeverification/k that referenced this pull request Apr 9, 2024
…ut (runtimeverification/pyk#842)

We were having trouble in
runtimeverification/evm-semantics#2272 selecting
the correct claims because they can be unqualified by the user. This PR:

- Fully qualifies all claim labels and dependency labels returned by
`KProve.get_claim_modules`.
- Makes sure that we return not only the user-selected claims, but their
dependencies (unless the user turns that off), from `KProve.get_claims`.
- Adds a test-suite for the claim selection functionality.

There is a bug in this code, and a test added (and skipped using
`pytest.skip()`) which demonstrates a (probably minor) issue where we
are not checking that claim dependencies are in a module imported by the
current module.

---------

Co-authored-by: devops <[email protected]>
Baltoli pushed a commit to runtimeverification/k that referenced this pull request Apr 9, 2024
…/pyk#866)

Here: runtimeverification/pyk#842, a mistake was
made where claims without labels were filtered out of the claims to be
processed and returned by `KProve.get_claims`. This PR addresses that
and adds tests. In particular:

- Two claims in the `multi-claim-spec.k` which were ill-formed because
of dependencies (but not tested for it) are removed.
- An ill-formed (because of dependencies) claim is added in a separate
module, and a new test added which exercises that a `ValueError` is
thrown when loading this module.
- A new test is added which asserts that the correct number of claims is
loaded from the main module. On `master`, this test is failing because
it skips claims without labels, but is fixed on this PR.
- Adjustments to how claims are filtered are made, so that it's simpler
and includes all claims, even ones without explicit module names in the
label.

---------

Co-authored-by: devops <[email protected]>
Baltoli pushed a commit to runtimeverification/k that referenced this pull request Apr 9, 2024
…ut (runtimeverification/pyk#842)

We were having trouble in
runtimeverification/evm-semantics#2272 selecting
the correct claims because they can be unqualified by the user. This PR:

- Fully qualifies all claim labels and dependency labels returned by
`KProve.get_claim_modules`.
- Makes sure that we return not only the user-selected claims, but their
dependencies (unless the user turns that off), from `KProve.get_claims`.
- Adds a test-suite for the claim selection functionality.

There is a bug in this code, and a test added (and skipped using
`pytest.skip()`) which demonstrates a (probably minor) issue where we
are not checking that claim dependencies are in a module imported by the
current module.

---------

Co-authored-by: devops <[email protected]>
Baltoli pushed a commit to runtimeverification/k that referenced this pull request Apr 9, 2024
…/pyk#866)

Here: runtimeverification/pyk#842, a mistake was
made where claims without labels were filtered out of the claims to be
processed and returned by `KProve.get_claims`. This PR addresses that
and adds tests. In particular:

- Two claims in the `multi-claim-spec.k` which were ill-formed because
of dependencies (but not tested for it) are removed.
- An ill-formed (because of dependencies) claim is added in a separate
module, and a new test added which exercises that a `ValueError` is
thrown when loading this module.
- A new test is added which asserts that the correct number of claims is
loaded from the main module. On `master`, this test is failing because
it skips claims without labels, but is fixed on this PR.
- Adjustments to how claims are filtered are made, so that it's simpler
and includes all claims, even ones without explicit module names in the
label.

---------

Co-authored-by: devops <[email protected]>
Baltoli pushed a commit to runtimeverification/k that referenced this pull request Apr 10, 2024
…ut (runtimeverification/pyk#842)

We were having trouble in
runtimeverification/evm-semantics#2272 selecting
the correct claims because they can be unqualified by the user. This PR:

- Fully qualifies all claim labels and dependency labels returned by
`KProve.get_claim_modules`.
- Makes sure that we return not only the user-selected claims, but their
dependencies (unless the user turns that off), from `KProve.get_claims`.
- Adds a test-suite for the claim selection functionality.

There is a bug in this code, and a test added (and skipped using
`pytest.skip()`) which demonstrates a (probably minor) issue where we
are not checking that claim dependencies are in a module imported by the
current module.

---------

Co-authored-by: devops <[email protected]>
Baltoli pushed a commit to runtimeverification/k that referenced this pull request Apr 10, 2024
…/pyk#866)

Here: runtimeverification/pyk#842, a mistake was
made where claims without labels were filtered out of the claims to be
processed and returned by `KProve.get_claims`. This PR addresses that
and adds tests. In particular:

- Two claims in the `multi-claim-spec.k` which were ill-formed because
of dependencies (but not tested for it) are removed.
- An ill-formed (because of dependencies) claim is added in a separate
module, and a new test added which exercises that a `ValueError` is
thrown when loading this module.
- A new test is added which asserts that the correct number of claims is
loaded from the main module. On `master`, this test is failing because
it skips claims without labels, but is fixed on this PR.
- Adjustments to how claims are filtered are made, so that it's simpler
and includes all claims, even ones without explicit module names in the
label.

---------

Co-authored-by: devops <[email protected]>
Baltoli pushed a commit to runtimeverification/k that referenced this pull request Apr 10, 2024
…ut (runtimeverification/pyk#842)

We were having trouble in
runtimeverification/evm-semantics#2272 selecting
the correct claims because they can be unqualified by the user. This PR:

- Fully qualifies all claim labels and dependency labels returned by
`KProve.get_claim_modules`.
- Makes sure that we return not only the user-selected claims, but their
dependencies (unless the user turns that off), from `KProve.get_claims`.
- Adds a test-suite for the claim selection functionality.

There is a bug in this code, and a test added (and skipped using
`pytest.skip()`) which demonstrates a (probably minor) issue where we
are not checking that claim dependencies are in a module imported by the
current module.

---------

Co-authored-by: devops <[email protected]>
Baltoli pushed a commit to runtimeverification/k that referenced this pull request Apr 10, 2024
…/pyk#866)

Here: runtimeverification/pyk#842, a mistake was
made where claims without labels were filtered out of the claims to be
processed and returned by `KProve.get_claims`. This PR addresses that
and adds tests. In particular:

- Two claims in the `multi-claim-spec.k` which were ill-formed because
of dependencies (but not tested for it) are removed.
- An ill-formed (because of dependencies) claim is added in a separate
module, and a new test added which exercises that a `ValueError` is
thrown when loading this module.
- A new test is added which asserts that the correct number of claims is
loaded from the main module. On `master`, this test is failing because
it skips claims without labels, but is fixed on this PR.
- Adjustments to how claims are filtered are made, so that it's simpler
and includes all claims, even ones without explicit module names in the
label.

---------

Co-authored-by: devops <[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.

5 participants