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

Partial support of the primitive bvnot #745

Merged
merged 6 commits into from
Jul 25, 2023

Conversation

Halbaroth
Copy link
Collaborator

@Halbaroth Halbaroth commented Jul 21, 2023

This PR add a partial support of the bitvector primitive bvnot. Basically, the canonizer of the bitvector theory pushes the not operator to the leaves of the semantic value as much as it can.

@Halbaroth Halbaroth added this to the 2.5.0 milestone Jul 21, 2023
@Halbaroth Halbaroth marked this pull request as ready for review July 24, 2023 15:35
@Halbaroth
Copy link
Collaborator Author

There is no regression :) It is ready for review ;)

Copy link
Collaborator

@bclement-ocp bclement-ocp 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 very fond of neg_mode because the use of mode implies a more fundamental change of behavior to me. I'd rather call this just neg or maybe negated which I think is more clear that the function operates as usual, but computes the negation of the result.

But I don't really have a strong justification for this, so if you feel strongly about using neg_mode, it's fine by me.

src/lib/reasoners/bitv.ml Outdated Show resolved Hide resolved
src/lib/reasoners/bitv.ml Outdated Show resolved Hide resolved
src/lib/reasoners/bitv.ml Outdated Show resolved Hide resolved
@Halbaroth Halbaroth merged commit 084d746 into OCamlPro:next Jul 25, 2023
10 checks passed
bclement-ocp added a commit to bclement-ocp/alt-ergo that referenced this pull request Sep 13, 2023
In PR OCamlPro#745 we correctly handled the case where uninterpreted terms
appear deep within a bit-vector terms, but we accidentally missed the
case where the whole bit-vector is itself an uninterpreted term, leading
to unsoundness (see the attached test).

This patch fixes the code to properly deal with `bvnot` applied to an
uninterpreted term.
Halbaroth pushed a commit that referenced this pull request Sep 14, 2023
In PR #745 we correctly handled the case where uninterpreted terms
appear deep within a bit-vector terms, but we accidentally missed the
case where the whole bit-vector is itself an uninterpreted term, leading
to unsoundness (see the attached test).

This patch fixes the code to properly deal with `bvnot` applied to an
uninterpreted term.
Halbaroth pushed a commit to Halbaroth/alt-ergo that referenced this pull request Sep 14, 2023
In PR OCamlPro#745 we correctly handled the case where uninterpreted terms
appear deep within a bit-vector terms, but we accidentally missed the
case where the whole bit-vector is itself an uninterpreted term, leading
to unsoundness (see the attached test).

This patch fixes the code to properly deal with `bvnot` applied to an
uninterpreted term.
Halbaroth added a commit that referenced this pull request Sep 14, 2023
* Do not discard `bvnot` applied to uninterpreted terms

In PR #745 we correctly handled the case where uninterpreted terms
appear deep within a bit-vector terms, but we accidentally missed the
case where the whole bit-vector is itself an uninterpreted term, leading
to unsoundness (see the attached test).

This patch fixes the code to properly deal with `bvnot` applied to an
uninterpreted term.

* Update the changes log for v2.5.1

* Review changes

---------

Co-authored-by: Basile Clément <bc@ocamlpro.com>
Halbaroth pushed a commit to Halbaroth/alt-ergo that referenced this pull request Sep 15, 2023
In PR OCamlPro#745 we correctly handled the case where uninterpreted terms
appear deep within a bit-vector terms, but we accidentally missed the
case where the whole bit-vector is itself an uninterpreted term, leading
to unsoundness (see the attached test).

This patch fixes the code to properly deal with `bvnot` applied to an
uninterpreted term.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants