-
Notifications
You must be signed in to change notification settings - Fork 32
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
Stop to migrate and omit < 4.08 support #126
Conversation
I found out that we don't need to use ppxlib to just generate AST, so I modified to use only ocaml-compiler-libs to do. |
92fb65c
to
f034f09
Compare
src/syntax.ml
Outdated
@@ -182,17 +182,23 @@ let module_ nm vs = Str.module_ (Mb.mk (strloc (Some nm)) (Mod.structure vs)) | |||
(* try body with _ -> with_ *) | |||
let try_ body with_ = Exp.try_ body [ Exp.case (Pat.any ()) with_ ] | |||
|
|||
let construct lid p = | |||
#if OCAML_VERSION >= (4, 13, 0) | |||
let p = Option.map (fun p -> ([], p)) p in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to pass empty list not to use existensial type variables (<= 4.13.0).
Looks good @Nymphium. Could you address the 2 minor comments and ensure the commits are split into; 1 changes to the generator to add functionality (either squashing into a single commit or multiple commits if there is a good reason to) and 2 running codegen/fmt via make gen? |
17eeba5
to
1ce2126
Compare
This PR migrates from ocaml-migrate-parsetree to ppxlib and removes version migration from the current compiler to 4.04.
All the services has an constraint
ocaml {>= "4.08"}
and generating AST doesn't use new syntactic features, such as let-operator, so there was no reason to support < 4.08.