ocaml-ppx / ppxlib

Base library and tools for ppx rewriters
MIT License
246 stars 98 forks source link

OCaml 4.08 and Parsetree #82

Open psteckler opened 5 years ago

psteckler commented 5 years ago

In ppxlib 0.8.0, the file ast/import.ml contains the line:

module Js    = Migrate_parsetree.OCaml_407

When building in an OCaml 4.08 setup, that seems to make Ppxlib.Parsetree refer to the Parsetree in Ast_407 from ocaml-migrate-parsetree.

That seems wrong, and makes code fail to compile that compiled fine with OCaml 4.07.1.

Shouldn't that line use the current version of OCaml, rather than a fixed version?

ghost commented 5 years ago

That's intentional. If it was based on the current version of the AST, then at every new compiler release a large number of ppx rewriters based on ppxlib would immediately break. In a way, ppxlib provides a protective layer on top of the compiler libraries.

This is not ideal and indeed it means that the ppx world tends to lag behind. However, there is an effort to improve it: https://discuss.ocaml.org/t/the-future-of-ppx/3766

psteckler commented 5 years ago

The compilation issue I see is because the latest OPAM package ppx_deriving (released 9 July) uses 4.08 parse trees, while ppxlib still uses 4.07 parse trees.

I can pin the branch in #80, which resolves the parse tree mismatch issue, but then other ppx packages won't install.

I'll have to wait until the ppx ecosystem has caught up to 4.08 before migrating my code.

ghost commented 5 years ago

I'll have to wait until the ppx ecosystem has caught up to 4.08 before migrating my code.

Unfortunately yes. That's the current story and it's not a good one. That's why we want to change it.

Kakadu commented 5 years ago

During migration from ppxlib 0.8.1 to 0.9.0 I needed to fix my code that converts current OCaml parsetree to ppxlib's one

 module Migr =
-  Migrate_parsetree.Convert(Migrate_parsetree.OCaml_current)(Migrate_parsetree.OCaml_407)
+  Migrate_parsetree.Convert(Migrate_parsetree.OCaml_current)(Migrate_parsetree.OCaml_408)

Is there any way change OCaml_408 by some other module name to make code unbreakable by next AST upgrade in ppxlib? I imagine ppxlib exporting right migration module but today I couldn't find the one.

ghost commented 5 years ago

Yep, there is already something like this. IIRC, it's Ppxlib.Selected_ast.Ast