Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Upgrade ocaml-migrate-parsetree to support latest ocaml #275

Merged
merged 4 commits into from
Jul 10, 2020
Merged

Upgrade ocaml-migrate-parsetree to support latest ocaml #275

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
97adc2c
upgrade ocaml-migrate-parsetree to 4.11 version, upgrade dune to 2.0,…
bikallem Jun 7, 2020
d3114f3
address reviewer comments
bikallem Jun 7, 2020
dc66c3b
add dune >= 2.0.0 to .opam files
bikallem Jun 8, 2020
a5a97de
install latest dune in drone ci
bikallem Jun 8, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion dune-project
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,2 +1,5 @@
(lang dune 1.0)
(lang dune 2.0)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs the ppx_cstruct.opam to have a corresponding dune {>= "2.0.0"} entry.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, just one last change needed here before merge. Since the dune-project affects all the associated projects, every opam package needs a corresponding dune >2 entry. e.g. the Drone CI is failing as its trying to install cstruct with dune 1.11 and that no longer works.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have now added dune {>= "2.0.0"} constraint to all .opam files in repo.


(allow_approximate_merlin)
avsm marked this conversation as resolved.
Show resolved Hide resolved

(name cstruct)
15 changes: 8 additions & 7 deletions fuzz/dune
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,11 @@
(name fuzz)
(libraries cstruct bigarray cstruct-sexp crowbar fmt))

(alias
(name fuzz)
(deps fuzz.exe (source_tree input))
(action (run
timeout --preserve-status 50m
bun -v --input=input --output=output
-- ./fuzz.exe)))
(rule
(alias fuzz)
(deps
fuzz.exe
(source_tree input))
(action
(run timeout --preserve-status 50m bun -v --input=input --output=output --
./fuzz.exe)))
7 changes: 5 additions & 2 deletions lib/dune
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,12 @@
(name cstruct)
(public_name cstruct)
(libraries bigarray-compat)
(c_names cstruct_stubs)
(foreign_stubs
(language c)
(names cstruct_stubs))
(wrapped false)
(js_of_ocaml (javascript_files cstruct.js))
(js_of_ocaml
(javascript_files cstruct.js))
(modules cstruct cstruct_cap))

(library
Expand Down
4 changes: 2 additions & 2 deletions lib_test/dune
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@
(libraries cstruct bigarray alcotest cstruct-sexp)
(name tests))

(alias
(name runtest)
(rule
(alias runtest)
(package cstruct-sexp)
(action
(run ./tests.exe -e)))
4 changes: 2 additions & 2 deletions ppx/dune
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,6 @@
(wrapped false)
(ppx_runtime_libraries cstruct stdlib-shims)
(preprocess
(pps ppx_tools_versioned.metaquot_404))
(pps ppx_tools_versioned.metaquot_411))
(libraries sexplib ocaml-migrate-parsetree ppx_tools_versioned
ppx_tools_versioned.metaquot_404 bigarray stdlib-shims))
ppx_tools_versioned.metaquot_411 bigarray stdlib-shims))
39 changes: 22 additions & 17 deletions ppx/ppx_cstruct.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,14 @@
open Migrate_parsetree
open Printf

open Ast_404
open Ast_411
open Longident
open Asttypes
open Parsetree
open Ast_helper
open Ast_mapper
module Loc = Location
module Ast = Ast_convenience_404
module Ast = Ast_convenience_411

type mode = Big_endian | Little_endian | Host_endian | Bi_endian

Expand Down Expand Up @@ -331,10 +331,10 @@ let output_struct _loc s =
and expr_le = Mod.structure (output_struct_one_endian _loc {s with endian = Little_endian})

in [{pstr_desc = Pstr_module
{pmb_name = {txt = "BE"; loc = _loc}; pmb_expr = expr_be;
{pmb_name = {txt = Some "BE"; loc = _loc}; pmb_expr = expr_be;
pmb_attributes = []; pmb_loc = _loc;}; pstr_loc = _loc;};
{pstr_desc = Pstr_module
{pmb_name = {txt = "LE"; loc = _loc}; pmb_expr = expr_le;
{pmb_name = {txt = Some "LE"; loc = _loc}; pmb_expr = expr_le;
pmb_attributes = []; pmb_loc = _loc;}; pstr_loc = _loc;}
]
| _ -> output_struct_one_endian _loc s
Expand Down Expand Up @@ -511,8 +511,11 @@ let output_enum_sig loc (cenum:cenum) =
let constr_enum = function
| {pcd_name = f; pcd_args = Pcstr_tuple []; pcd_attributes = attrs; _} ->
let id = match attrs with
| [{txt = "id"; _}, PStr
[{pstr_desc = Pstr_eval ({pexp_desc = Pexp_constant cst; pexp_loc = loc; _}, _); _}]] ->
| [{attr_name = {txt = "id";_};
attr_payload =
PStr [{ pstr_desc =
Pstr_eval ({pexp_desc = Pexp_constant cst; pexp_loc = loc; _}, _); _}]
;_ }] ->
let cst = match cst with
| Pconst_integer(i, _) -> Int64.of_string i
| _ ->
Expand All @@ -527,18 +530,19 @@ let constr_enum = function
loc_err loc "invalid cenum variant"

let get_len = function
| [ ({txt = "len"; loc},
PStr
[{pstr_desc =
Pstr_eval ({pexp_desc = Pexp_constant (Pconst_integer (sz, None)); _}, _)
; _}])]
| [{attr_name = {txt = "len"; loc};
attr_payload = PStr
[{pstr_desc =
Pstr_eval ({pexp_desc = Pexp_constant (Pconst_integer (sz, None)); _}, _);
_}]
; _}]
->
let n = int_of_string sz in
if n > 0 then
Some n
else
loc_err loc "[@len] argument should be > 0"
| [{txt = "len"; loc}, _ ] ->
| [{attr_name = {txt = "len"; loc}; _} ] ->
loc_err loc "[@len] argument should be an integer"
| _ ->
None
Expand All @@ -565,8 +569,8 @@ let cstruct decl =
| _ -> loc_err loc "record type declaration expected"
in
let endian = match attrs with
| [{txt = endian; _}, PStr []] -> endian
| [_, _] -> loc_err loc "no attribute payload expected"
| [{attr_name = {txt = endian; _}; attr_payload = PStr []; _}] -> endian
| [_] -> loc_err loc "no attribute payload expected"
| _ -> loc_err loc "too many attributes"
in
create_struct loc endian name fields
Expand All @@ -581,9 +585,10 @@ let cenum decl =
in
let width, sexp =
match attrs with
| ({txt = width; _}, PStr []) :: ({txt = "sexp"; _}, PStr []) :: [] ->
| ({attr_name = {txt = width; _};attr_payload= PStr [];_})
:: ({attr_name = {txt = "sexp"; _};attr_payload = PStr []; _}) :: [] ->
width, true
| ({txt = width; _}, PStr []) :: [] ->
| ({attr_name = {txt = width; _};attr_payload= PStr [];_}) :: [] ->
width, false
| _ ->
loc_err loc "invalid cenum attributes"
Expand Down Expand Up @@ -637,5 +642,5 @@ let structure mapper s =
List.concat (List.map (structure_item' mapper) s)

let () =
Driver.register ~name:"ppx_cstruct" Versions.ocaml_404
Driver.register ~name:"ppx_cstruct" Versions.ocaml_411
(fun _config _cookies -> {default_mapper with structure; signature})
32 changes: 18 additions & 14 deletions ppx_test/errors/dune
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,23 +1,27 @@
(executable
(name pp)
(modules pp)
(preprocess (action
(run %{bin:cppo} -V OCAML:%{ocaml_version} %{input-file})))
(libraries
ppx_cstruct
ocaml-migrate-parsetree))
(name pp)
(modules pp)
(preprocess
(action
(run %{bin:cppo} -V OCAML:%{ocaml_version} %{input-file})))
(libraries ppx_cstruct ocaml-migrate-parsetree))

(executable
(name gen_tests)
(modules gen_tests))
(name gen_tests)
(modules gen_tests))

(include dune.inc)

(rule
(targets dune.inc.gen)
(deps (source_tree .))
(action (with-stdout-to %{targets} (run ./gen_tests.exe))))
(deps
(source_tree .))
(action
(with-stdout-to
%{targets}
(run ./gen_tests.exe))))

(alias
(name runtest)
(action (diff dune.inc dune.inc.gen)))
(rule
(alias runtest)
(action
(diff dune.inc dune.inc.gen)))
64 changes: 32 additions & 32 deletions ppx_test/errors/dune.inc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@
%{targets}
(run ./pp.exe --impl %{input}))))

(alias
(name runtest)
(rule
(alias runtest)
(package ppx_cstruct)
(action
(diff cenum_id_payload.ml.expected cenum_id_payload.ml.errors)))
Expand All @@ -21,8 +21,8 @@
%{targets}
(run ./pp.exe --impl %{input}))))

(alias
(name runtest)
(rule
(alias runtest)
(package ppx_cstruct)
(action
(diff cenum_invalid_type.ml.expected cenum_invalid_type.ml.errors)))
Expand All @@ -35,8 +35,8 @@
%{targets}
(run ./pp.exe --impl %{input}))))

(alias
(name runtest)
(rule
(alias runtest)
(package ppx_cstruct)
(action
(diff cenum_no_attribute.ml.expected cenum_no_attribute.ml.errors)))
Expand All @@ -49,8 +49,8 @@
%{targets}
(run ./pp.exe --impl %{input}))))

(alias
(name runtest)
(rule
(alias runtest)
(package ppx_cstruct)
(action
(diff cenum_not_a_variant.ml.expected cenum_not_a_variant.ml.errors)))
Expand All @@ -63,8 +63,8 @@
%{targets}
(run ./pp.exe --impl %{input}))))

(alias
(name runtest)
(rule
(alias runtest)
(package ppx_cstruct)
(action
(diff cenum_unknown_attribute.ml.expected cenum_unknown_attribute.ml.errors)))
Expand All @@ -77,8 +77,8 @@
%{targets}
(run ./pp.exe --impl %{input}))))

(alias
(name runtest)
(rule
(alias runtest)
(package ppx_cstruct)
(action
(diff cstruct_attribute_payload.ml.expected cstruct_attribute_payload.ml.errors)))
Expand All @@ -91,8 +91,8 @@
%{targets}
(run ./pp.exe --impl %{input}))))

(alias
(name runtest)
(rule
(alias runtest)
(package ppx_cstruct)
(action
(diff cstruct_duplicate_field.ml.expected cstruct_duplicate_field.ml.errors)))
Expand All @@ -105,8 +105,8 @@
%{targets}
(run ./pp.exe --impl %{input}))))

(alias
(name runtest)
(rule
(alias runtest)
(package ppx_cstruct)
(action
(diff cstruct_len_int32.ml.expected cstruct_len_int32.ml.errors)))
Expand All @@ -119,8 +119,8 @@
%{targets}
(run ./pp.exe --impl %{input}))))

(alias
(name runtest)
(rule
(alias runtest)
(package ppx_cstruct)
(action
(diff cstruct_len_not_int.ml.expected cstruct_len_not_int.ml.errors)))
Expand All @@ -133,8 +133,8 @@
%{targets}
(run ./pp.exe --impl %{input}))))

(alias
(name runtest)
(rule
(alias runtest)
(package ppx_cstruct)
(action
(diff cstruct_len_zero.ml.expected cstruct_len_zero.ml.errors)))
Expand All @@ -147,8 +147,8 @@
%{targets}
(run ./pp.exe --impl %{input}))))

(alias
(name runtest)
(rule
(alias runtest)
(package ppx_cstruct)
(action
(diff cstruct_multiple_len.ml.expected cstruct_multiple_len.ml.errors)))
Expand All @@ -161,8 +161,8 @@
%{targets}
(run ./pp.exe --impl %{input}))))

(alias
(name runtest)
(rule
(alias runtest)
(package ppx_cstruct)
(action
(diff cstruct_not_a_record.ml.expected cstruct_not_a_record.ml.errors)))
Expand All @@ -175,8 +175,8 @@
%{targets}
(run ./pp.exe --impl %{input}))))

(alias
(name runtest)
(rule
(alias runtest)
(package ppx_cstruct)
(action
(diff cstruct_not_an_identifier.ml.expected cstruct_not_an_identifier.ml.errors)))
Expand All @@ -189,8 +189,8 @@
%{targets}
(run ./pp.exe --impl %{input}))))

(alias
(name runtest)
(rule
(alias runtest)
(package ppx_cstruct)
(action
(diff cstruct_several_attributes.ml.expected cstruct_several_attributes.ml.errors)))
Expand All @@ -203,8 +203,8 @@
%{targets}
(run ./pp.exe --impl %{input}))))

(alias
(name runtest)
(rule
(alias runtest)
(package ppx_cstruct)
(action
(diff cstruct_unknown_endian.ml.expected cstruct_unknown_endian.ml.errors)))
Expand All @@ -217,8 +217,8 @@
%{targets}
(run ./pp.exe --impl %{input}))))

(alias
(name runtest)
(rule
(alias runtest)
(package ppx_cstruct)
(action
(diff cstruct_unknown_type.ml.expected cstruct_unknown_type.ml.errors)))
4 changes: 2 additions & 2 deletions ppx_test/errors/gen_tests.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ let output_stanzas name =
%%{targets}
(run ./pp.exe --impl %%{input}))))

(alias
(name runtest)
(rule
(alias runtest)
(package ppx_cstruct)
(action
(diff %s.expected %s.errors)))
Expand Down
2 changes: 1 addition & 1 deletion ppx_test/with-lwt/ppx_cstruct_and_lwt.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,5 +10,5 @@ type foo64 =
[@@uint64_t]
]

let%lwt foo = Lwt.return ()
let foo = Lwt_main.run (Lwt.return ())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why change this? That use of let%lwt was there in the test suite was there to check that ppx_cstruct and ppx_lwt work together.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The recent version of ppx_lwt gives the warning below,

File "ppx_test/with-lwt/ppx_cstruct_and_lwt.ml", line 13, characters 0-27:
Error (warning 22): let%lwt should not be used at the module item level.
Replace let%lwt x = e by let x = Lwt_main.run (e)

I have now modified the test to use let%lwt inside rather than at the module toplevel to get around the warning.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call; there should be no interop problems with modern ppx, but there were in the past, so its good to keep the test around.


Loading