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

Add support for OCaml 5.1 #428

Merged
merged 1 commit into from
Jun 20, 2023
Merged

Conversation

tmattio
Copy link
Collaborator

@tmattio tmattio commented Jun 16, 2023

  • Update trunk AST for pvb_constraint
  • Add migration from 501 to 500
  • Bump the 501 AST magic numbers
  • Binary AST reader: use latest possible AST version

When the magic numbers aren't unique, before this commit, the binary AST reader would read into the first AST version matching the magic number. With this commit, it takes the last such version. That's important for trunk support.

  • Fix inline tests' dependency on OCaml version

Done by making the expected output also dependent on the OCaml version


This is cherry-picking 4ebc7d4 to 9d95d42. (with a small difference from 4ebc7d4 to remove the README update)

I've squashed the commits to make the history cleaner as most of the commits seemed to be fixes of the intial commit, but I can keep the history intact if you prefer.

I'll rebase trunk-support on top of main once merged.

@tmattio tmattio force-pushed the support-510-squashed branch 2 times, most recently from 6423a07 to 23b43c6 Compare June 16, 2023 14:34
Copy link
Member

@pitag-ha pitag-ha left a comment

Choose a reason for hiding this comment

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

Thanks for taking this on! There seem to be a few important commits from trunk-support missing (see below).

CHANGES.md Outdated Show resolved Hide resolved
Comment on lines 1034 to 1044
and poly_constraint (*IF_CURRENT = Parsetree.poly_constraint *) =
{
locally_abstract_univars:string loc list;
typ:core_type;
}

and value_binding (*IF_CURRENT = Parsetree.value_binding *) =
{
pvb_pat: pattern;
pvb_expr: expression;
pvb_constraint: poly_constraint option;
Copy link
Member

Choose a reason for hiding this comment

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

This is the version from before the compiler Parsetree fix for coercions, isn't it?

ppxlib.opam Outdated
@@ -21,7 +21,7 @@ doc: "https://ocaml-ppx.github.io/ppxlib/"
bug-reports: "https://github.com/ocaml-ppx/ppxlib/issues"
depends: [
"dune" {>= "2.7"}
"ocaml" {>= "4.04.1" & < "5.1.0"}
"ocaml" {>= "4.04.1" & < "5.2.0"}
Copy link
Member

Choose a reason for hiding this comment

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

We also need a "not compatible with ocaml.5.1.0~alpha1" constraint here.

@tmattio
Copy link
Collaborator Author

tmattio commented Jun 17, 2023

Thanks for the review @pitag-ha and you're right I had omitted commits that came after the support for 5.2 that fixed 5.1. I added e072b1c and 1966c31.

So the only commits from trunk-support that are included now should be:

Do any of these seem relevant for 5.1?

Copy link
Member

@pitag-ha pitag-ha left a comment

Choose a reason for hiding this comment

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

Looks almost good, thanks! We just need to run and promote the tests to fix them on 5.1. Could you do that?

CHANGES.md Outdated Show resolved Hide resolved
CHANGES.md Outdated Show resolved Hide resolved
astlib/migrate_501_500.ml Outdated Show resolved Hide resolved
CHANGES.md Show resolved Hide resolved
* Update trunk AST for pvb_constraint
* Add migration from 501 to 500
* Bump the 501 AST magic numbers
* Binary AST reader: use latest possible AST version

When the magic numbers aren't unique, before this commit, the binary
AST reader would read into the first AST version matching the magic
number. With this commit, it takes the last such version. That's important
for trunk support.

* Fix inline tests' dependency on OCaml version

Done by making the expected output also dependent on the OCaml version

* Add a add tests to test both round-trips as well as a test comparing
parsing on 5.0.0 with parsing on 5.1.0 and then down migrating. This last
test will need to be updated when bumping the AST.

Co-authored-by: Samuel Hym <samuel.hym@rustyne.lautre.net>
Co-authored-by: Florian Angeletti <florian.angeletti@inria.fr>
Co-authored-by: Sonja Heinze <sonjaleaheinze@gmail.com>
Co-authored-by: Paul-Elliot <peada@free.fr>
Signed-off-by: Thibaut Mattio <thibaut.mattio@gmail.com>
@tmattio
Copy link
Collaborator Author

tmattio commented Jun 19, 2023

Looks almost good, thanks! We just need to run and promote the tests to fix them on 5.1. Could you do that?

Done

Copy link
Member

@pitag-ha pitag-ha left a comment

Choose a reason for hiding this comment

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

Thank you, looks all good (up to the still running CI)! Once the CI finishes and passes, let's merge this and then cut a release right away.

@tmattio
Copy link
Collaborator Author

tmattio commented Jun 20, 2023

The cluster is having a hard time running the jobs - macOS x86 and Debian s390x have been hanging since yesterday.

Seeing all the other jobs are passing and we don't have an immediate solution for unlocking the hanging jobs, I'm merging now.

@tmattio tmattio merged commit f0320c4 into ocaml-ppx:main Jun 20, 2023
4 of 5 checks passed
tmattio added a commit to tmattio/opam-repository that referenced this pull request Jun 20, 2023
CHANGES:

- Adopt the OCaml Code of Conduct on the repo (ocaml-ppx/ppxlib#426, @pitag-ha)

- Clean up misleading attribute hints when declared for proper context. (ocaml-ppx/ppxlib#425, @ceastlund)

- Ast_pattern now has ebool, pbool helper, and a new map.(ocaml-ppx/ppxlib#402, @Burnleydev1)

- multiple errors are now reported in `metaquot`. (ocaml-ppx/ppxlib#397, @Burnleydev1)

- Add `Attribute.declare_with_attr_loc` (ocaml-ppx/ppxlib#396, @dvulakh)

- Add "ns" and "res" as reserved namespaces(ocaml-ppx/ppxlib#388, @davesnx)

- Make quoter `let` binding non-recursive (ocaml-ppx/ppxlib#401, @sim642)

- Fix failure of 'lift_map_with_context' in traverse by compile-time
  evaluation of 'fst' and 'snd' (ocaml-ppx/ppxlib#390, @smuenzel)

- Driver: Bias the mapping from magic to version towards the current version,
  as it is usually the common case and it helps when magic numbers are
  ambiguous (such as on development versions) (ocaml-ppx/ppxlib#409, @shym)

- Remove unnecessary test dependencies towards base and stdio (ocaml-ppx/ppxlib#421, @kit-ty-kate)

- Update description to reflect that `ppxlib` contains more than a library
  (ocaml-ppx/ppxlib#422, @pitag-ha)

- Add support for OCaml 5.1, excluding OCaml `5.1.0~alpha1` (ocaml-ppx/ppxlib#428, @shym, @Octachron , @pitag-ha, @panglesd)
- Driver: Fix `-locations-check` option for coercions with ground  (ocaml-ppx/ppxlib#428, @Octachron)
tmattio added a commit to tmattio/opam-repository that referenced this pull request Jun 20, 2023
CHANGES:

- Adopt the OCaml Code of Conduct on the repo (ocaml-ppx/ppxlib#426, @pitag-ha)

- Clean up misleading attribute hints when declared for proper context. (ocaml-ppx/ppxlib#425, @ceastlund)

- Ast_pattern now has ebool, pbool helper, and a new map.(ocaml-ppx/ppxlib#402, @Burnleydev1)

- multiple errors are now reported in `metaquot`. (ocaml-ppx/ppxlib#397, @Burnleydev1)

- Add `Attribute.declare_with_attr_loc` (ocaml-ppx/ppxlib#396, @dvulakh)

- Add "ns" and "res" as reserved namespaces(ocaml-ppx/ppxlib#388, @davesnx)

- Make quoter `let` binding non-recursive (ocaml-ppx/ppxlib#401, @sim642)

- Fix failure of 'lift_map_with_context' in traverse by compile-time
  evaluation of 'fst' and 'snd' (ocaml-ppx/ppxlib#390, @smuenzel)

- Driver: Bias the mapping from magic to version towards the current version,
  as it is usually the common case and it helps when magic numbers are
  ambiguous (such as on development versions) (ocaml-ppx/ppxlib#409, @shym)

- Remove unnecessary test dependencies towards base and stdio (ocaml-ppx/ppxlib#421, @kit-ty-kate)

- Update description to reflect that `ppxlib` contains more than a library
  (ocaml-ppx/ppxlib#422, @pitag-ha)

- Add support for OCaml 5.1, excluding OCaml `5.1.0~alpha1` (ocaml-ppx/ppxlib#428, @shym, @Octachron , @pitag-ha, @panglesd)
- Driver: Fix `-locations-check` option for coercions with ground  (ocaml-ppx/ppxlib#428, @Octachron)
tmattio added a commit to tmattio/opam-repository that referenced this pull request Jun 20, 2023
CHANGES:

- Adopt the OCaml Code of Conduct on the repo (ocaml-ppx/ppxlib#426, @pitag-ha)

- Clean up misleading attribute hints when declared for proper context. (ocaml-ppx/ppxlib#425, @ceastlund)

- Ast_pattern now has ebool, pbool helper, and a new map.(ocaml-ppx/ppxlib#402, @Burnleydev1)

- multiple errors are now reported in `metaquot`. (ocaml-ppx/ppxlib#397, @Burnleydev1)

- Add `Attribute.declare_with_attr_loc` (ocaml-ppx/ppxlib#396, @dvulakh)

- Add "ns" and "res" as reserved namespaces(ocaml-ppx/ppxlib#388, @davesnx)

- Make quoter `let` binding non-recursive (ocaml-ppx/ppxlib#401, @sim642)

- Fix failure of 'lift_map_with_context' in traverse by compile-time
  evaluation of 'fst' and 'snd' (ocaml-ppx/ppxlib#390, @smuenzel)

- Driver: Bias the mapping from magic to version towards the current version,
  as it is usually the common case and it helps when magic numbers are
  ambiguous (such as on development versions) (ocaml-ppx/ppxlib#409, @shym)

- Remove unnecessary test dependencies towards base and stdio (ocaml-ppx/ppxlib#421, @kit-ty-kate)

- Update description to reflect that `ppxlib` contains more than a library
  (ocaml-ppx/ppxlib#422, @pitag-ha)

- Add support for OCaml 5.1, excluding OCaml `5.1.0~alpha1` (ocaml-ppx/ppxlib#428, @shym, @Octachron , @pitag-ha, @panglesd)
- Driver: Fix `-locations-check` option for coercions with ground  (ocaml-ppx/ppxlib#428, @Octachron)
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.

None yet

2 participants