-
Notifications
You must be signed in to change notification settings - Fork 389
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
Add support for OCaml 4.13 #4352
Conversation
Signed-off-by: Kate <kit.ty.kate@disroot.org>
I'm sorry, I noted this change here but failed to make sure that it was seen by anyone working on dune. |
No worries and thanks @kit-ty-kate for the patch. Nice to know about this OCaml PR BTW, seems like we might finally be able to get rid of BTW, I'm curious how the OCaml PR relates to this change. I'm guessing this is because of the |
@jeremiedimino there was some discussion of the need to add this annotation in the ocaml PR comments, see e.g. ocaml/ocaml#10081 (comment) . TL;DR: it seems that this code was relying on (non-principal) horizontal type propagation. |
i forgot to specify, this probably needs a cherry-pick in main as well |
Signed-off-by: Kate <kit.ty.kate@disroot.org>
@jberdine thanks for the pointer. Without digging too much into this, my initial interpretation is: "where type annotations are needed depend on implementation details in the compiler". Am I far from the truth? :) @kit-ty-kate, cherry-picked in main. |
Yeah, that's right. Except that if you compile with |
Record disambiguation behaviour changed when using
|>
in ocaml/ocaml#10081cc @alainfrisch