-
Notifications
You must be signed in to change notification settings - Fork 37
clean intf #863
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
clean intf #863
Conversation
|
|
||
| let eq e1 e2 = | ||
| if phys_equal e1 e2 then Symbolic_boolean.true_ else relop Ty_bool Eq e1 e2 | ||
| (* TODO: why do we use `Ty_bool` in the first two cases and `ty` in the others? What is the righe choice? *) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cc @filipeom, I can't make sense of this, could you explain how it works to me?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bitvectors use the default equality operator in SMTLIB (i.e., =). Since we pattern match on the theory to dispatch the operator we can just use Ty_bool for bitvectors to get the default boolean equality operator. But probably using ty would also work. Internally it defaults to the same operator used for Ty_bool.
| relop Ty_bool Eq e (of_concrete c) |> Symbolic_boolean.of_expr | ||
|
|
||
| let eq e1 e2 = | ||
| if phys_equal e1 e2 then Symbolic_boolean.true_ else relop Ty_bool Eq e1 e2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cc @filipeom, I removed the phys_equal shortcut as it did not seem used that often to me and I also thinks it would make more sense to have it in smtml, WDYT?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
merged 😄
No description provided.