Bias the mapping from magic to version towards the current version

[shym]

When mapping from magic numbers to versions, first check whether it is the current version, which is probably the most common case It has the added benefit that, when the mapping is ambiguous (mostly between the latest stable release and the development version), this strategy makes the good choice for the test suite

[pitag-ha]

Thanks, @shym! Does it sound good to you to add a changelog entry? Alternatively, I can write it and it and push it to your branch.

[shym]

I just updated the changelog. I assumed its entries are not sorted.

[pitag-ha]

Perfect, LGTM! The only nitpick (see below): I'd specify in the changelog that the change is about the driver, since library and driver are mixed in the same package in ppxlib and most user-facing changes are on the library.

[pitag-ha]

Suggested change

	- 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) (#409, @shym)
	- 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) (#409, @shym)

[panglesd]

Just out of curiosity, is there a case where ppxlib is reading an AST which is not of magic number "Ocaml_current"? In other words, could (module OCaml_current : OCaml_version) :: all_versions be replaced with [(module OCaml_current : OCaml_version)]?

[shym]

It does happen at least in the test suite, for instance in test/driver/run_as_ppx_rewriter_preserve_version/test.t/406_binary_ast. (But I admit I was wondering about the use case)

[pitag-ha]

is there a case where ppxlib is reading an AST which is not of magic number "Ocaml_current"?

This is the case whenever the PPX executable is compiled with a different OCaml version than the project. So, in particular, that's never the case when people use dune. However, it's usually the case in the bucklescript/rescript ecosystem (although I'm not sure how much they still use PPXs). And it can also be the case for OCaml people who don't want to use dune and the standard Ppxlib driver workflow and instead produce their own PPX executables (with Ppxlib) and use a Makefile passing those PPX executable(s) to ocamlopt/ocamlc manually.

That being said, I don't think that's a problem for the following reason: on released Ppxlib versions, nothing has changed: released Ppxlib versions are still only compatible with stable compilers, among which the magic numbers are unique. So biasing or not doesn't make a difference in that context. And whenever we are in a context where we allow Ppxlib to be co-installable with trunk (such as the trunk-support branch), it is clear that we don't guarantee that things will work. In that context, we do our best. Thank to @shym's idea, in that context, Ppxlib can now be cross-compiler compatible whenever people use dune. When people use a manual set-up, they might have to make sure that things work manually.