Skip to content

Comments

Migrate Ptyp_open from 502 to 501#625

Open
patricoferris wants to merge 1 commit intoocaml-ppx:mainfrom
patricoferris:migrate-module-opens
Open

Migrate Ptyp_open from 502 to 501#625
patricoferris wants to merge 1 commit intoocaml-ppx:mainfrom
patricoferris:migrate-module-opens

Conversation

@patricoferris
Copy link
Collaborator

A bug fix to support migration syntax like type t = M.(t) to the 501 AST via, our now, standard approach of using extensions. This really only impacts those who have not upgraded ppxlib to 0.36.0.

So, to help people, perhaps we backport this to a 0.35.1 release?

A bug fix to support migration syntax like `type t = M.(t)` to the 501
AST via, our now, standard approach of using extensions. This really
only impacts those who have not upgraded ppxlib to 0.36.0.

Signed-off-by: Patrick Ferris <patrick@sirref.org>
@NathanReb
Copy link
Collaborator

I'm not sure this would improve the situation much, especially since as you point out, our AST will not go back to 5.1. Now the error would be a Uninterpreted Extension .... The extension name would hint at ppxlib which is good but I still feel like it wouldn't beat a good error message there.
I think the quality of the uninterpreted extension error is balanced by the improved compatibility that we gain from allowing new features to be used alongside ppx-es but that won't be the case here since it would only happen when using older compilers.

How about simply improving the error message and backport it to 0.35 as you suggest?

@patricoferris
Copy link
Collaborator Author

ppxlib.0.35.0 is installable with OCaml 5.2 and 5.3 so it is possible (as was the case here: https://mastodon.social/@jonmsterling@mathstodon.xyz/116085163199163273) that you could use type t = M.(t) and expect to be preserved?

@NathanReb
Copy link
Collaborator

NathanReb commented Feb 17, 2026

Yeah I guess that's right, I didn't really expect to backport this kind of support but given how hard it can be to upgrade from 0.35 to 0.36, I'd say it's fine indeed!

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