Update for 5.4 #100
Update for 5.4 #100
Conversation
gasche
left a comment
gasche
left a comment
There was a problem hiding this comment.
Thanks a lot! This looks very reasonable to me, and extending the tests is a nice touch.
| let string_of_lid lid = Format.asprintf "%a" Astlib.Pprintast.longident lid | ||
|
|
||
| (* Remove locations *) | ||
| let rec longident_without_loc = |
There was a problem hiding this comment.
Is the point of this function really to remove the location, or is it to convert to the Astlib type? If the latter, then the name should reflect that.
There was a problem hiding this comment.
Both? In particular, ppx_import uses List.assoc on Longident.t, which sounds dangerous on variants of Longident.t with a location.
There was a problem hiding this comment.
In issue #99, @NathanReb commented:
I exposed
Astlib.Longident.to/from_compilerin ppxlib.0.37.0 which you should be able to use if you'd like!
(My impression is that it would be nice to merge a fix relatively quickly to unblock users, so I would be okay with merging the current approach and cleaning things up later or not at all. @Octachron, a preference?)
|
The CI appears to be broken on this repository due to versions of github actions that are too old, I will send a separate PR to CI-golf this. => #101 |
[v1.x] Backport of "#100: Update for 5.4"
CHANGES: * Support for OCaml 5.4 (ocaml-ppx/ppx_import#100, @Octachron, backport ocaml-ppx/ppx_import#104, @egallego) * Support for OCaml 4.5, 4.6, and 4.7 has been removed (ocaml-ppx/ppx_import#104, @egallego)
2 participants
This PR adds support for the split world of OCaml 5.4, where the compiler has a richer
Longident.tandTupletypes than the Astlib version.