search

ocaml-ppx / ppxlib

Base library and tools for ppx rewriters
MIT License
244 stars 94 forks source link

Add support for latest trunk (5.03) #487

Closed NathanReb closed 1 week ago

NathanReb commented 2 months ago

Most of the work comes from @hhugo 's trunk-support-53 branch.

I rebased it all on top of main and reverted some of the formatting changes to instead use the original fromatting from the compiler modules to avoid large diffs if when we update the Ast_503 module in the future.

I also fixed part of the migrations that had the same Pmod_apply_unit migration bug we had on the 5.2 support branch so I suspect the migrations were copied from the 5.02 <-> 5.01 ones. I think this makes for another argument in favor in importing the omp tools to generate deep copy functions to use as a base for the migration modules. That would prevent this kind of bug.

I haven't reviewed the whole migrations extensively so I'm leaving this as draft for now. The branch can still be useful to people that wish to test trunk.

NathanReb commented 2 months ago

I tried this with ocaml-variants.5.3.0+trunk and it builds, most of the tests pass except for some slight changes in some expect tests output but nothing meaningful there. E.g. in error messages Ppxlib.Ast.expression is now reported as Ppxlib.expression.

I need to look into those more to better understand them but it just looks like the error reporting got better on the compiler side and it picks up the global include Ast in ppxlib.ml.

There is also a change in the quoter text which I'm not sure how to interpret yet so I'll look into it.

kit-ty-kate commented 2 months ago

shouldn't this be going into the trunk-support branch instead of main ?

NathanReb commented 1 month ago

shouldn't this be going into the trunk-support branch instead of main ?

No, we're trying to add support gradually on the main branch instead. We'll exclude 5.03 when releasing but main will stay compatible with trunk as much as possible.

kit-ty-kate commented 1 month ago

ah cool!

NathanReb commented 1 month ago

It would be nice to add trunk builds in the CI from there onward. If they could be flagged as optional (as in don't have to be green to merge) that would be even better but I'll take anything.

What's the best way to add that?

NathanReb commented 1 month ago

I guess we could reuse https://github.com/ocaml-ppx/ppxlib/pull/410

NathanReb commented 1 month ago

Just rebased on top of the new trunk CI build addition, let's see how it goes!

NathanReb commented 1 month ago

The trunk build seems to work as expected and even picked up an error, I'll look into it!

NathanReb commented 4 weeks ago

I pushed support for the latest changes to location.

Our limited preprocessor to use different code branches based on compiler versions is reaching its limits I think but is still holding up.

I think we can go ahead and review this. @Octachron if you want to look at the latest commit that adds compat for your latest changes to the compiler, there might be room for improvement there!