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

Fix generated html for conjunctive types #353

Closed
wants to merge 1 commit into from

Conversation

Octachron
Copy link
Member

This PR fixes the html generated for conjunctive types, for instance

type 'a t = [< `X of & int & float ] as 'a

which are currently printed as

type 'a t = [< `X of int * float ] as 'a

because the list of arguments of the polymorphic variant constructor is interpreted as a product instead of a conjunction.

@Octachron Octachron force-pushed the conjunctive_types branch 2 times, most recently from 2664304 to 8c68485 Compare April 26, 2019 20:54
@jonludlam
Copy link
Member

Well, you learn something new every day :-) I'd not come across these types before.



(* Conjunctive types: dune compilation scheme exposes a bug in old
versions of the compiler *)
Copy link
Member

Choose a reason for hiding this comment

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

What does this refer to?

Copy link
Member Author

Choose a reason for hiding this comment

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

I was thinking of ocaml/ocaml#7629 , but there were other issues with conjunctive types in older version of ocaml : ocaml/ocaml#7496 for instance. The short explanation is that conjunctive types are intended as type-checking artefacts and using them directly can make compilation break in unexpected way.

@Octachron
Copy link
Member Author

Indeed, conjunctive types are not really intended to be used directly. Honestly, the value of this PR is more in the documentation of the unusal interpration of constructor arguments for polymorphic variants than in the fix itself.

@jonludlam
Copy link
Member

Right. Thanks very much for this!

@jonludlam
Copy link
Member

Rebased in #358

@jonludlam jonludlam closed this May 23, 2019
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.

2 participants