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

Make bv_automata fallible #670

Merged
merged 1 commit into from
Oct 3, 2024
Merged

Conversation

ineol
Copy link
Contributor

@ineol ineol commented Sep 30, 2024

Currently, bv_automata does not fail if the goal is false, because native_decide doesn't fail, it produces an ill-typed proof.

On consequence is that it is awkward to use bv_automata in a tactic such as bv_auto, since it relied on tactics to fail to be able to try the next tactic (cf issue #660) It also give an error that is not helpful at all after the proof is complete.

This PR is a proof of concept that modifies the implementation of native_decide to check that the problem is solved by the decision procedure before constructing the proof object.

@tobiasgrosser
Copy link
Collaborator

tobiasgrosser commented Sep 30, 2024

This seems to be sensible. Should we merge it for now?

@ineol ineol changed the title WIP: Make bv_automata fallible Make bv_automata fallible Oct 1, 2024
@ineol
Copy link
Contributor Author

ineol commented Oct 1, 2024

I'm confused about the CI failure. The error is that it doesn't have permission to add the comment to PR with the Alive stats, but it works for other PRs...

@tobiasgrosser
Copy link
Collaborator

I guess our CI only works for branches within the repo. Can you just create your branch within the repo and see if it goes through?

@ineol ineol mentioned this pull request Oct 1, 2024
github-merge-queue bot pushed a commit that referenced this pull request Oct 1, 2024
Same as #670 but from a branch on the main repo.
@tobiasgrosser tobiasgrosser added this pull request to the merge queue Oct 3, 2024
Merged via the queue into opencompl:main with commit b4bdf9f Oct 3, 2024
3 of 4 checks passed
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.

2 participants