-
Notifications
You must be signed in to change notification settings - Fork 89
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
Fix ord on variants conflicting with Result
#260
base: master
Are you sure you want to change the base?
Conversation
I'm guessing my fix only works with OCaml 4.11 and up due to the following point in OCaml 4.11.0 release notes:
I tried slapping Changing that to avoid the
I hopefully managed to make it all work by also inserting |
Closes #254. Besides
Error
also applies toOk
, coming fromPpx_deriving_runtime
.Conceptually the fix is simple: just add a type constraint to the
to_int
function generated into thewildcard_case
of variant comparison. This forces type-directed construction resolution to correctly use the constructor from our type instead ofResult
.The tricky part was not breaking other tests, where a new variant type is defined with the same name as a standard one, e.g.
bool
. In that case theto_int
type constraint inside the comparator must still refer to the outerbool
type being declared, not the one fromPpx_deriving_runtime
, which happens to be opened at that point due tosanitize
.A local module definition
Ppx_deriving_ord_helper
is used to capture the type being declared outside ofsanitize
, such that inside it, we can refer to the correctbool
. Hopefully such type-only local module doesn't have any runtime cost (?), otherwise I'll have to make it conditional to only happen for variant types (currently it's generated unconditionally and otherwise just unused).