Convert type classes to LF type synonyms #4023
Conversation
cocreature
force-pushed
the
typeclass-synonym
branch
4 times, most recently
from
0790916 to
dd64798
Compare
cocreature
force-pushed
the
typeclass-synonym
branch
from
dd64798 to
f38b234
Compare
|
@associahedron Both good ideas, I’ve incorporated your suggestions. Thanks! |
cocreature
force-pushed
the
typeclass-synonym
branch
2 times, most recently
from
26a88f4 to
7306e3f
Compare
| @@ -1,5 +1,3 @@ | |||
| {-# LANGUAGE ConstrainedClassMethods #-} | |||
There was a problem hiding this comment.
This is enabled by default.
There was a problem hiding this comment.
Actually it isn’t. The GHC docs lied to me by claiming that MultiParamTypeclasses imply ConstrainedClassMethods.
| @@ -9,5 +7,8 @@ class A t where | |||
| foo : t -> t | |||
| bar : Eq t => t -> t | |||
|
|
|||
| class B t where | |||
| baz : B b => b -> t | |||
| -- This would create a recursive type synonym. | |||
There was a problem hiding this comment.
We decided to just disable this for now. We don’t use this on our large internal DAML codebase. There is a reasonable workaround with manual dictionary passing and if it does become a record we could either handle this in the LF conversion or support infinite types (something like OCaml’s -rectypes).
There was a problem hiding this comment.
Seems reasonable to me. I don't know of any natural examples of a class that requires itself as a constraint.
| @@ -32,7 +32,6 @@ for LF_VERSION in $PKG_DB/*; do | |||
| stdlib=$LF_VERSION/daml-stdlib-*.dalf | |||
| prim=$LF_VERSION/daml-prim.dalf | |||
| $DIFF -b -u <(get_serializable_types $stdlib) <(cat <<EOF | |||
| "DA.Upgrade:MetaEquiv" | |||
There was a problem hiding this comment.
This is a typeclass with no methods which was therefore serializable but is now translated to Unit.
cocreature
force-pushed
the
typeclass-synonym
branch
from
5d10b13 to
0241d80
Compare
| pure $ ERecProj (fromTCon recTyp) fldName scrutinee' `ETmApp` EUnit | ||
| if envLfVersion env `supports` featureTypeSynonyms | ||
| then pure $ EStructProj fldName scrutinee' `ETmApp` EUnit | ||
| else pure $ ERecProj (fromTCon recTyp) fldName scrutinee' `ETmApp` EUnit |
There was a problem hiding this comment.
I think you can replace this if expression by using the helper function,
pure $ mkDictProj env (fromTCon recTyp) fldName scrutinee' `ETmApp` EUnitThere was a problem hiding this comment.
good catch, fixed. Thank you!
| @@ -9,5 +7,8 @@ class A t where | |||
| foo : t -> t | |||
| bar : Eq t => t -> t | |||
|
|
|||
| class B t where | |||
| baz : B b => b -> t | |||
| -- This would create a recursive type synonym. | |||
There was a problem hiding this comment.
Seems reasonable to me. I don't know of any natural examples of a class that requires itself as a constraint.
changelog_begin changelog_end
cocreature
force-pushed
the
typeclass-synonym
branch
from
0241d80 to
7756a58
Compare
This is still a draft PR until type synonyms are ready across the stack (in particular the Scala side is still missing completely) but the conversion should work and I’m able to run
daml buildwith this on a large DAML codebase.changelog_begin
changelog_end
Pull Request Checklist
CHANGELOG_BEGINandCHANGELOG_ENDtagsNOTE: CI is not automatically run on non-members pull-requests for security
reasons. The reviewer will have to comment with
/AzurePipelines runtotrigger the build.