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
Fixes #5481: support for unification of universe variables in congruence and f_equal #18106
Fixes #5481: support for unification of universe variables in congruence and f_equal #18106
Conversation
68d4f1d
to
631b962
Compare
@coqbot run full ci |
This is very hackish, and somewhat only side-stepping the real problem that we shouldn't ever rely on an ill-defined notion of "equality" for terms in an unification state... I guess this can qualify as a hotfix but I really don't like neither the original code nor the change. |
Can't we just say that congruence works with its own notion of quotient over terms and that it does not have to be a standard one? That is, from the moment that congruence is able to reconstruct a CIC proof that the terms it equates are equal, is it blamable that it uses its own quotient? [This being said, I don't like particularly the patch either, but I don't know what else to do.] |
Equality only makes sense when the state is around. If you change the values of evars for instance, you'd get a different term. You cannot even EConstr.kind these things without the state. |
The union-find part of In the PR, there is unification involved but only between universe levels. With the PR, any two universe levels are assumed unifiable by default and an attempt to actually unify them happens at proof reconstruction, when we are back in the econstr stateful world. (Note that I did not test what happens when it fails to reconstruct, so let's try, ah, it says "Not convertible", which maybe should be turned instead into "congruence failed"...). Back to evars, would we like to have congruence work also modulo evar-unification? At least, it is unclear to me how to implement union-find up to unification, if that's your point. |
@herbelin if you implement @SkySkimmer 's change I can merge. |
Also turn a tclFAIL into a NotConvertible so that it can be caught.
…ruence. We hash declarations without their universe instances while the universe constraints are reinferred at the time of building the proof. Also fixes coq#9979 (f_equal with universe polymorphism). Also now solves "Goal Type -> Type" by unifying universe levels. Co-authored-by: SkySkimmer <SkySkimmer@users.noreply.github.com>
…ils. We address here the new case of unification failure between universe variables (see previous commit).
631b962
to
2f2ea93
Compare
Done. I also added catching a typing error so that a proper error message is issued when unification of levels fails. |
@coqbot merge now |
We hash declarations without their universe instances while the universe constraints are (anyway) reinferred at the time of building the proof.
Giving two arguments to
Ccalgo.Symb
as I did is a bit hackish. The alternative would be to have a hash-consing function that discards universes but I did not find one.Fixes #5481
Fixes #9979
Now also added: unification of universe levels so that
congruence
works on e.g.Type -> Type
, the same way as it works for any non-TypeT -> T
.