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

Z3 from string #226

Merged
merged 2 commits into from
Apr 5, 2023
Merged

Z3 from string #226

merged 2 commits into from
Apr 5, 2023

Conversation

Pat-Lafon
Copy link
Contributor

I am interested in pushing through #194.

The changes I made are:

@Pat-Lafon
Copy link
Contributor Author

I have two additional comments which came up while I was attempting this. First, I was wondering if instead of creating a solver object, the api here should be mutating an already existing solver object with some smtlib2 string-based assertions? Second, I noticed some of the surrounding unsafe code does explicit null checks on the pointers it gets from the ffi. Should some of the Z3 pointers instead be wrapped in https://doc.rust-lang.org/std/ptr/struct.NonNull.html ?

@waywardmonkeys
Copy link
Contributor

For the first question, I would look to see what the other language bindings for Z3 do.

for the second, I am not very familiar with that, so I haven’t thought about it before. Want to Kay out some arguments in favor in a separate issue?

Copy link
Contributor

@waywardmonkeys waywardmonkeys left a comment

Choose a reason for hiding this comment

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

Can you rebase this into fewer commits? Some of the changes modify the earlier commits in the sequence, but shouldn't be needed in the history...

verpeteren and others added 2 commits April 5, 2023 13:29
patch add new_from_smtlib2 to Solver

This makes it consistent with the Optimize impl.
@waywardmonkeys waywardmonkeys merged commit 691ac97 into prove-rs:master Apr 5, 2023
@Pat-Lafon Pat-Lafon deleted the Z3_from_string branch April 6, 2023 15:03
@Pat-Lafon Pat-Lafon mentioned this pull request Apr 12, 2023
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.

3 participants