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

Set the default error handler to None #92

Merged
merged 3 commits into from
Sep 14, 2020
Merged

Set the default error handler to None #92

merged 3 commits into from
Sep 14, 2020

Conversation

SeeSpring
Copy link
Contributor

@fitzgen
Copy link
Contributor

fitzgen commented Sep 14, 2020

Can you describe what happens now when get_model is called on a solver that doesn't have a satisfiable answer? Does it hang? Does it return an empty model?

@SeeSpring
Copy link
Contributor Author

get_model should return a null pointer if the solver is unsatisfiable. I think another option would be to change get_model to return Option<Model<'ctx>> and have Z3_model be a nonnull pointer.

@fitzgen
Copy link
Contributor

fitzgen commented Sep 14, 2020

get_model should return a null pointer if the solver is unsatisfiable

So would that lead to UB with the current patch, which always returns a Model?

I think another option would be to change get_model to return Option<Model<'ctx>> and have Z3_model be a nonnull pointer.

Yes, I think this would be preferable.

@SeeSpring
Copy link
Contributor Author

One issue is that Z3_model is defined in z3-sys as

pub type Z3_model = *mut _Z3_model;

so don't think it can be made into non-null without changing bindgen. Another option would be to panic if Z3_ast_map_find/Z3_optimize_get_model/Z3_params_validate/Z3_solver_get_model/Z3_solver_get_proof would have normally invoked the error handler. C++ calls and checks Z3_get_error_code but Java, C# and ML only check if the return value is null.

@fitzgen
Copy link
Contributor

fitzgen commented Sep 14, 2020

We wouldn't change the z3-sys bindings, just the higher-level z3 crate built on top of them.

@SeeSpring
Copy link
Contributor Author

SeeSpring commented Sep 14, 2020

One issue with this is that this would be semver breaking since it changes function signatures vs panicking if we get null.

@fitzgen
Copy link
Contributor

fitzgen commented Sep 14, 2020

Releasing a new breaking version isn't a big deal.

Copy link
Contributor

@fitzgen fitzgen left a comment

Choose a reason for hiding this comment

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

Thanks!

@fitzgen fitzgen merged commit e6ba783 into prove-rs:master Sep 14, 2020
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.

Are there any ways to "catch" the error of Z3_mk_model?
2 participants