-
Notifications
You must be signed in to change notification settings - Fork 22
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
Generalized coercions #420
base: master
Are you sure you want to change the base?
Conversation
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.
I have one question: do we really need the coercions to use unification to see if we are from/to a structure? I don't see the point of aliasing a structure.
If the clauses were of the form
pi bla bla bla\ coercion ctx v (app[Structure|RevArgs]) e r :-
they could be indexed much better (we have some deep indexing features in the pipes). This rule would trigger only if the type is syntactically the structure type constructor applied to the list of arguments. The body can still use unification if needed.
@CohenCyril LGTM. Can you double check and merge? |
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.
That's great @Tragicus thanks for pushing.
I have a general question: how does it compose with coq coercions?
e.g. reusing your test-suite file (coercion.v
), this is what happens if I have
Axioms (A B B' C : Type) (AtoB : A -> B) (BtoB' : B -> B').
Axioms (P : A -> Prop) (CtoAP : C -> sigType A P).
Coercion AtoB : A >-> B.
Coercion BtoB' : B >-> B'.
(* does postcompose automatically with Coq coercions*)
Check fun (x : sigType A P) => x : B.
Check fun (x : sigType A P) => x : B'.
(* testing a Coq coercion to sigType *)
Coercion CtoAP : C >-> sigType.
Check fun (x : C) => x : sigType A P.
(* does not precompose automatically with Coq coercions *)
Fail Check fun (x : C) => x : A.
Check fun (x : C) => (x : sigType A P) : A.
Check fun (x : C) => (x : sigType A P) : B.
HB.structure Definition Sig (T : Type) (P : T -> Prop) := {x of isSigma T P x}. | ||
|
||
Check fun (T : Type) (P : T -> Prop) (x : sigType T P) => x : T. | ||
Check fun (T : Type) (P : T -> Prop) (x : T) (Px : Sig T P x) => x : sigType T P. |
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.
This looks really nice! But...
- it is a bit more than generalized coercions, for which I would expect this coercion to work only when
(x : T)
has been endowed with aHB.instance
ofisSigma
. e.g.
Axioms (A : Type) (P : A -> Type) (x : A) (Px : Sig A P x).
(* should not work indeed *)
Fail Check (x : sigType A P).
(* this works though ...*)
Succeed Check (let Px' := Px in x : sigType A P).
(* This should works but does not *)
HB.instance Definition _ := Px.
Fail Check x : sigType A P.
In comparison, the analogous code, but on Type
works:
HB.mixin Record isSigmaT (P : Type -> Prop) (x : Type) := { _ : P x }.
#[short(type="sigTType"), verbose]
HB.structure Definition SigT (P : Type -> Prop) := {x of isSigmaT P x}.
Axioms (X : Type) (PT : Type -> Prop) (PX : SigT PT X).
(* should not work indeed *)
Fail Check (X : sigTType A PT).
(* this works though now ... cf my next point*)
Succeed Check (let PX' := PX in X : sigTType PT).
(* This is what should work and does *)
HB.instance Definition _ := PX.
Succeed Check X : sigTType PT.
- I cannot find what exactly in the code makes this work, I was wondering what makes it work even when the coercion is not handled by elpi and but by Coq. In comparison, on master the following fails:
From Coq Require Import ssreflect.
From HB Require Import structures.
HB.mixin Record isSigmaT (P : Type -> Prop) (x : Type) := { _ : P x }.
#[short(type="sigTType"), verbose]
HB.structure Definition SigT (P : Type -> Prop) := {x of isSigmaT P x}.
Fail Check fun (P : Type -> Prop) (x : Type) (Px : SigT P x) => x : sigTType P.
Fail Check (let PX' := PX in X : sigTType PT).
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.
I think the code just above does not work because the whole business does not kick in since the coercion sort
is handled by Coq (the tgt is a supported one in this case), so nobody triggers the TC search.
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.
I am extremely confused as to why it works in Type
but not in Prop
. In both cases, HB.instance
declares canonical structures and not typeclasses, so it should not work.
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.
Your tests with a let
work and not the previous ones because (afaict) the elaborator sees the local context but not the global context.
As your tests show, the sort coercion postcomposes and does not precompose with Coq's coercions. This is because when the algorithm sees that the type of the input term is a structure, it applies the projection and then asks for a coercion to the target type. |
About post composition, the code does not look at the expected type. If the inferred type is a structure it elaborates (sort t) against the expected type. So Coq can insert further coercions. If the inferred type is not a structure, then it does nothing. In your last examples I think that the first coercion from C to sigType is inserted by Coq, then elpi goes to A and then Coq goes to C. I'm not so sure I see a use case for something (that is not a structure) coercing to a structure. |
One way to test it on a large scale would be to systematically replace Coq coercions by elpi coercions even for structures in Type and see if/where it breaks. |
A use case for something that is not a structure coercing to a structure is when you have an object that satisfies properties that would promote it to a structure, e.g. morphisms and subobjects. This coercion allows you to skip the packaging phase. |
IMO, the only missing bit is a documentation, et least in the changelog. |
@CohenCyril can you look at the PR again and decide what to do? |
Changelog.md
Outdated
- **Change** `HB.structure` declares the class of a structure (`axioms_`) as a | ||
type class on the subject with all arguments in output mode but for the | ||
subject that is in input mode. | ||
|
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.
Isn't this completely ortogonal to the coercion part?
To me these raises yet unresolved nontrivial. I have nothing against merging it, but, just in case, I would prefer that it is a distinct commit/PR so that we can revert easily if something goes wrong.
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.
maybe an #[predicate_subtype]
default to false?
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.
Yes or rather #[typeclass]
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.
Sorry for the delay, I rewrote the history as suggested and added a typeclass
option.
structures.v
Outdated
@@ -15,7 +15,7 @@ Definition ignore {T} (x: T) := x. | |||
Definition ignore_disabled {T T'} (x : T) (x' : T') := x'. | |||
|
|||
(* ********************* structures ****************************** *) | |||
From elpi Require Import elpi. | |||
From elpi Require Import elpi coercion. |
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.
From elpi Require Import elpi coercion. | |
From elpi Require Import elpi. | |
From elpi.apps Require Import coercion. |
HB/structure.elpi
Outdated
mk-reverse-coercion-aux (prod _N _T Body) Structure G Args Clause :- | ||
Clause = (pi x\ C x), | ||
pi x\ mk-reverse-coercion-aux (Body x) Structure G [x | Args] (C x). |
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.
mk-reverse-coercion-aux (prod _N _T Body) Structure G Args Clause :- | |
Clause = (pi x\ C x), | |
pi x\ mk-reverse-coercion-aux (Body x) Structure G [x | Args] (C x). | |
mk-reverse-coercion-aux (prod _N _T Body) Structure G Args (pi x\ C x) :- | |
pi x\ mk-reverse-coercion-aux (Body x) Structure G [x | Args] (C x). |
HB/structure.elpi
Outdated
|
||
pred mk-reverse-coercion i:gref, o:prop. | ||
mk-reverse-coercion Structure Clause :- | ||
coq.typecheck (global Structure) T ok, |
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.
coq.typecheck (global Structure) T ok, | |
coq.env.typeof Structure T, |
HB/structure.elpi
Outdated
|
||
pred mk-sort-coercion i:term, i:term, o:prop. | ||
mk-sort-coercion Structure P Clause :- | ||
coq.typecheck Structure T ok, |
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.
coq.typecheck Structure T ok, | |
std.assert-ok! (coq.typecheck Structure T) "anomaly: mk-sort-coercion", |
I did a couple of suggestions, to be tested. Other than that it looks OK. @CohenCyril ? |
Taking a look this afternoon |
Could you squash and rebase? |
Do we have tools to do a benchmark? |
I think it would be enough to |
Ok thanks, I will do that tonight or tomorrow. |
It took forever, but the bench is done. Interestingly enough, the version with typeclasses by default is slightly slower and uses slightly less memory, though the difference is negligible. |
Well, I did it anyway, and |
Here is the new bench, |
Damn, I guess we will have to wait for some speedup in the bootstrap time of elpi... or disable coercion in apply: but I don't know if the api makes it easy. |
This PR adds better support for coercions:
elaborate-skeleton
to fill in the holes,sort
projection as an elpi coercion when there is no coercion target,