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

Attempts to avoid Why3's polymorphism encoding #903

Draft
wants to merge 19 commits into
base: master
Choose a base branch
from

Conversation

dewert99
Copy link
Collaborator

In order to continue trying to avoid matching loops/timeouts I am trying to avoid Why3's encoding_smt_if_poly and eliminate_algebraic_if_poly transformations. I created the driver z3_no_poly.drv which is mostly a copy of z3_486 but doesn't include these transformations. Most of the Why3 generated by Creusot is already monomorphic enough for the "smtv2.6" printer with the following exceptions (that I've found) which I've tried to resolve.

  • Why3 standard library sequences
    • I replaced creusot_contracts::Seq with an implementation with opaque methods that aren't replaced with functions from Why3's sequences
    • I implemented it in terms of the original version so I could avoid using #[trusted]. Does CI check that creusot_contracts verify?
    • The implementation of Seq::EMPTY is somewhat hacky since Creusot doesn't support constants
    • Since Creusot doesn't support opaque structs the new Seq implementations still depend on the Why3 standard library so I made z3_no_poly.drv remove all why3 standard library sequence methods.
  • Prelude ghost type
    • I made ghost_ty a single field struct, and made new and inner inline to the constructor and field accessor respectively
  • ADT field accessor functions generated by Why3
    • I added the inline:trivial attribute, but this only worked for structs

Ideally, the field accessor functions would get their own monomorphic cloneable module so that inline:trivial wouldn't be necessary, and z3_no_poly.drv would also support enum accessors, but I didn't want to mess with the CloneMap especially when it may be removed soon. Personally, combining these changes (including the driver) with the --simple-triggers options got things to the pointer where errors were reported as Unknown instead of Timeout and z3's Array/Mapping extensionality axioms could be relied upon.

@jhjourdan
Copy link
Collaborator

I created the driver z3_no_poly.drv which is mostly a copy of z3_486 but doesn't include these transformations.

That should be useless. If the goal is already monomorphic, then encoding_smt_if_poly and eliminate_algebraic_if_poly should be no-ops. The real work to be done if we want to get rid of polymorphism encoding is to get rid of polymorphism.
(If that's not the case, then it's probably a bug in Why3.)

Does CI check that creusot_contracts verify?

No.

I replaced creusot_contracts::Seq with an implementation with opaque methods that aren't replaced with functions from Why3's sequences

But then this means that SMT solvers do not have access to Why3's stdlib lemmas on sequences? I'm a bit afraid of the loss of proof power, then!

Personally, combining these changes (including the driver) with the --simple-triggers options got things to the pointer where errors were reported as Unknown instead of Timeout and z3's Array/Mapping extensionality axioms could be relied upon.

Could you explain a bit more the issue you are facing?

I actually think that there is a simpler way to get rid of the matching loops caused by polymorphism encoding: if, after polymorphism encoding, we remove all the polymorphic instances of lemmas, then essentially you get a goal without all the noise of polymorphism encoding. That's incomplete, but in practice, I'm don't think the polymorphic instances are used so much (especially in goals generated by Creusot).

@dewert99
Copy link
Collaborator Author

That should be useless. If the goal is already monomorphic, then encoding_smt_if_poly and eliminate_algebraic_if_poly should be no-ops. The real work to be done if we want to get rid of polymorphism encoding is to get rid of polymorphism.
(If that's not the case, then it's probably a bug in Why3.)

I generally agree, but I found removing the transformations useful for debugging where there was polymorphism to get rid of (I prefer the error to silently using the polymorphism encoding)

But then this means that SMT solvers do not have access to Why3's stdlib lemmas on sequences? I'm a bit afraid of the loss of proof power, then!

I tried to give the sequence methods similar specifications to what they had in why3 so within the solver I would assume they would work similarly, but I guess you would lose the the extra lemmas when using the IDE.

Could you explain a bit more the issue you are facing?
I think there are two main issues.

  1. the polymorphic instances of the Why3 sequence lemmas seem to contain a matching loop (I think it has to do with singleton)
  2. The eliminate_algebraic transformation seems to generate quantifiers when translating a match expression that returns a boolean. This seems to sometimes cause matching loops and I can't seem to find a reference to it in the paper for eliminate_algebraic https://inria.hal.science/inria-00439232.
    • The encoding_smt transformation doesn't work without first running eliminate_algebraic so in order to avoid this issue we need the input Why3 to be completely monomorphic

I actually think that there is a simpler way to get rid of the matching loops caused by polymorphism encoding: if, after polymorphism encoding, we remove all the polymorphic instances of lemmas, then essentially you get a goal without all the noise of polymorphism encoding. That's incomplete, but in practice, I'm don't think the polymorphic instances are used so much (especially in goals generated by Creusot).

If we could get some sort of eliminate_polymorphism transformation at the Why3 level to run after discriminate instead of encoding_smt_if_poly and eliminate_algebraic_if_poly that would be great.

@dewert99
Copy link
Collaborator Author

In general, it might be beneficial for Why3 to have a version of eliminate_algebraic that does not introduce quantifiers when translating formulas

Comment on lines 212 to 214
type ghost_ty 't
val function new (ghost x : 't) : ghost_ty 't
val function inner (x : ghost_ty 't) : 't
axiom new_spec: forall x: 't [new x]. inner (new x) = x
axiom inner_spec: forall x: ghost_ty 't [inner x]. new (inner x) = x
type ghost_ty 't = { ghost inner2 : 't }
let function new [@inline:trivial] (ghost x : 't) = { inner2 = x; }
let ghost function inner [@inline:trivial] (x : ghost_ty 't) = x.inner2
Copy link
Collaborator

Choose a reason for hiding this comment

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

ghost_ty was intentionally made an opaque type in #823 to fix the issue with infinite values in #436.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sorry, maybe I'm missing something, but I thought the goal of #823 was to prevent unsound recursive type definitions involving Ghost in Rust, and I don't see how that is related to the way Ghost is encoded in Why3.

Copy link
Collaborator

Choose a reason for hiding this comment

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

The check on the Rust level does not catch all unsound type definitions. For example, the following works:

struct Foo<T>(Ghost<T>);
struct Bad(Foo<Bad>);

To still prevent such unsound types, we additionally rely on the strict positivity check of Why3.
As Ghost is defined as opaque type, the parameter T of Foo is not considered strictly positive and thus the definition of Bad is rejected.

@dewert99
Copy link
Collaborator Author

I also noticed that the discriminate transformation doesn't seem to create a monomorphic instance of the Seq.create function even when using:

theory BuiltIn
  meta "select_inst" "all"
  meta "select_lskept" "all"
  meta "select_lsinst" "all"
  meta "select_kept" "all"
end

theory seq.Seq
  meta "encoding:lskept" function create
end

The result functions that seem to be generated by eliminate_epsilon also aren't monomorphized unless discriminate is run before eliminate_epsilon (I can seem to cause Seq.create to be monomorphized by running discriminate both before and after eliminate_epsilon but this creates duplicate hypotheses).

@dewert99
Copy link
Collaborator Author

dewert99 commented Nov 6, 2023

Are there any plans to add a remove_polymorphism/eliminate_polymorphism transformation to Why3? I think it would be useful to have some way of bypassing Why3s polymorphism encoding either directly within Why3 or by having Creusot output monomorphic Why3.

@jhjourdan
Copy link
Collaborator

This has been on my TODO list for months, already.

The problem is that I'd like to experiment with a stronger discriminate to mitigate the fact that polymorphic lemmas do not exists.

@dewert99
Copy link
Collaborator Author

dewert99 commented Nov 7, 2023

Unless I'm misunderstanding it seems like polymorphic lemmas do exist (eg. https://why3.lri.fr/stdlib/seq.html#associative_135), or do you mean that Creusot doesn't use any polymorphic lemmas?

@jhjourdan
Copy link
Collaborator

These lemmas do exist indeed and Creusot does use them. The goal of discriminate is to instantiate them at relevant types so that, in practice, we don't loose too much completeness by removing polymorphic versions.

@dewert99
Copy link
Collaborator Author

dewert99 commented Nov 7, 2023

Were you planning on adding a single new transformation that does a better discriminate and removes polymorphism, or adding a better discriminate transformation and a remove polymorphism transformation separately? If you add them separately, would it be possible to make a public Why3 branch with just the remove polymorphism transformation sooner? I think combining it with the existing discriminate transformation would already be helpful even if it means that the Seq.create function was eliminated.

# Conflicts:
#	creusot-contracts/src/logic/fmap.rs
#	creusot-contracts/src/std/slice.rs
#	creusot/src/backend/ty.rs
#	creusot/src/translation/constant.rs
#	prelude/prelude.mlw
@dewert99
Copy link
Collaborator Author

I've able to mostly make Seq and Snapshot normal trusted Creusot types instead of having them defined in Why3. The one issue is handling slice length in FMIR since it is currently translated to a polymorphic function in the prelude.

@jhjourdan
Copy link
Collaborator

The issue with not using Why3's support for sequences is that we will not be able to use prover's native support for sequences.

# Conflicts:
#	creusot/src/backend/clone_map/elaborator.rs
#	creusot/tests/should_fail/bug/01_resolve_unsoundness.coma
#	creusot/tests/should_fail/bug/869.coma
#	creusot/tests/should_fail/bug/specialize.coma
#	creusot/tests/should_fail/final_borrows.coma
#	creusot/tests/should_succeed/100doors.coma
#	creusot/tests/should_succeed/all_zero.coma
#	creusot/tests/should_succeed/bdd.coma
#	creusot/tests/should_succeed/bug/217.coma
#	creusot/tests/should_succeed/bug/682.coma
#	creusot/tests/should_succeed/bug/874.coma
#	creusot/tests/should_succeed/bug/two_phase.coma
#	creusot/tests/should_succeed/cell/02.coma
#	creusot/tests/should_succeed/filter_positive.coma
#	creusot/tests/should_succeed/hashmap.coma
#	creusot/tests/should_succeed/heapsort_generic.coma
#	creusot/tests/should_succeed/hillel.coma
#	creusot/tests/should_succeed/index_range.coma
#	creusot/tests/should_succeed/inferred_invarianrs.coma
#	creusot/tests/should_succeed/inplace_list_reversal.coma
#	creusot/tests/should_succeed/insertion_sort.coma
#	creusot/tests/should_succeed/invariant_moves.coma
#	creusot/tests/should_succeed/iterators/01_range.coma
#	creusot/tests/should_succeed/iterators/02_iter_mut.coma
#	creusot/tests/should_succeed/iterators/03_std_iterators.coma
#	creusot/tests/should_succeed/iterators/04_skip.coma
#	creusot/tests/should_succeed/iterators/05_map.coma
#	creusot/tests/should_succeed/iterators/06_map_precond.coma
#	creusot/tests/should_succeed/iterators/07_fuse.coma
#	creusot/tests/should_succeed/iterators/08_collect_extend.coma
#	creusot/tests/should_succeed/iterators/09_empty.coma
#	creusot/tests/should_succeed/iterators/10_once.coma
#	creusot/tests/should_succeed/iterators/11_repeat.coma
#	creusot/tests/should_succeed/iterators/12_zip.coma
#	creusot/tests/should_succeed/iterators/13_cloned.coma
#	creusot/tests/should_succeed/iterators/14_copied.coma
#	creusot/tests/should_succeed/iterators/15_enumerate.coma
#	creusot/tests/should_succeed/iterators/16_take.coma
#	creusot/tests/should_succeed/knapsack.coma
#	creusot/tests/should_succeed/knapsack_full.coma
#	creusot/tests/should_succeed/lang/while_let.coma
#	creusot/tests/should_succeed/list_index_mut.coma
#	creusot/tests/should_succeed/list_reversal_lasso.coma
#	creusot/tests/should_succeed/mapping_test.coma
#	creusot/tests/should_succeed/mutex.coma
#	creusot/tests/should_succeed/red_black_tree.coma
#	creusot/tests/should_succeed/rusthorn/inc_max_repeat.coma
#	creusot/tests/should_succeed/rusthorn/inc_some_2_list.coma
#	creusot/tests/should_succeed/rusthorn/inc_some_list.coma
#	creusot/tests/should_succeed/selection_sort_generic.coma
#	creusot/tests/should_succeed/slices/01.coma
#	creusot/tests/should_succeed/sparse_array.coma
#	creusot/tests/should_succeed/sum.coma
#	creusot/tests/should_succeed/sum_of_odds.coma
#	creusot/tests/should_succeed/syntax/06_logic_function_contracts.coma
#	creusot/tests/should_succeed/syntax/12_ghost_code.coma
#	creusot/tests/should_succeed/take_first_mut.coma
#	creusot/tests/should_succeed/traits/16_impl_cloning.coma
#	creusot/tests/should_succeed/type_invariants/vec_inv.coma
#	creusot/tests/should_succeed/vecdeque.coma
#	creusot/tests/should_succeed/vector/01.coma
#	creusot/tests/should_succeed/vector/02_gnome.coma
#	creusot/tests/should_succeed/vector/03_knuth_shuffle.coma
#	creusot/tests/should_succeed/vector/05_binary_search_generic.coma
#	creusot/tests/should_succeed/vector/06_knights_tour.coma
#	creusot/tests/should_succeed/vector/07_read_write.coma
#	creusot/tests/should_succeed/vector/08_haystack.coma
#	creusot/tests/should_succeed/vector/09_capacity.coma
dewert99 and others added 5 commits August 5, 2024 15:53
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