Skip to content

Zero alloc fix assume payload parsing #2152

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

Merged
merged 6 commits into from
Dec 14, 2023
Merged

Zero alloc fix assume payload parsing #2152

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
68a6553
Fix bug in payload detection for [@zero_alloc assume ..]
gretay-js Dec 13, 2023
a800e0d
Update existing test output (error message text only)
gretay-js Dec 13, 2023
4ae372e
Add a test
gretay-js Dec 13, 2023
bbe3b0a
Improve tests: don't ignore warnings
gretay-js Dec 13, 2023
db1dbbc
fixup! Add a test
gretay-js Dec 14, 2023
371af13
Add a comment
gretay-js Dec 14, 2023
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
4 changes: 3 additions & 1 deletion ocaml/lambda/translattribute.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,8 @@ let get_ids_from_exp exp =
|> Result.map List.rev


(* [parse_ids_payload] requires that each element in [cases]
the first component (string list) is alphabetically sorted. *)
let parse_ids_payload txt loc ~default ~empty cases payload =
let[@local] warn () =
let ( %> ) f g x = g (f x) in
Expand Down Expand Up @@ -270,7 +272,7 @@ let parse_property_attribute attr property =
Assume { property; strict = true; never_returns_normally = false; loc; };
["assume"; "never_returns_normally"],
Assume { property; strict = false; never_returns_normally = true; loc; };
["assume"; "strict"; "never_returns_normally"],
["assume"; "never_returns_normally"; "strict";],
Assume { property; strict = true; never_returns_normally = true; loc; };
["ignore"], Ignore_assert_all property
]
Expand Down
16 changes: 8 additions & 8 deletions tests/backend/checkmach/dune.inc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,19 +3,19 @@
(alias runtest)
(enabled_if (= %{context_name} "main"))
(deps s.ml t.ml)
(action (run %{bin:ocamlopt.opt} %{deps} -g -c -zero-alloc-check default -dcse -dcheckmach -dump-into-file -O3)))
(action (run %{bin:ocamlopt.opt} %{deps} -g -c -zero-alloc-check default -dcse -dcheckmach -dump-into-file -O3 -warn-error +a)))

(rule
(alias runtest)
(enabled_if (= %{context_name} "main"))
(deps t5.ml test_assume.ml)
(action (run %{bin:ocamlopt.opt} %{deps} -g -c -zero-alloc-check default -dcse -dcheckmach -dump-into-file -O3)))
(action (run %{bin:ocamlopt.opt} %{deps} -g -c -zero-alloc-check default -dcse -dcheckmach -dump-into-file -O3 -warn-error +a)))

(rule
(alias runtest)
(enabled_if (and (= %{context_name} "main") %{ocaml-config:flambda}))
(deps test_flambda.ml)
(action (run %{bin:ocamlopt.opt} %{deps} -g -c -zero-alloc-check default -dcse -dcheckmach -dump-into-file -O3)))
(action (run %{bin:ocamlopt.opt} %{deps} -g -c -zero-alloc-check default -dcse -dcheckmach -dump-into-file -O3 -warn-error +a)))

(rule
(enabled_if (= %{context_name} "main"))
Expand Down Expand Up @@ -477,19 +477,19 @@
(alias runtest)
(enabled_if (= %{context_name} "main"))
(deps t7.ml)
(action (run %{bin:ocamlopt.opt} %{deps} -g -c -zero-alloc-check default -dcse -dcheckmach -dump-into-file -O3)))
(action (run %{bin:ocamlopt.opt} %{deps} -g -c -zero-alloc-check default -dcse -dcheckmach -dump-into-file -O3 -warn-error +a)))

(rule
(alias runtest)
(enabled_if (= %{context_name} "main"))
(deps test_stub_dep.ml test_stub.ml)
(action (run %{bin:ocamlopt.opt} %{deps} -g -c -zero-alloc-check default -dcse -dcheckmach -dump-into-file -O3)))
(action (run %{bin:ocamlopt.opt} %{deps} -g -c -zero-alloc-check default -dcse -dcheckmach -dump-into-file -O3 -warn-error +a)))

(rule
(alias runtest)
(enabled_if (and (= %{context_name} "main") %{ocaml-config:flambda}))
(deps t1.ml)
(action (run %{bin:ocamlopt.opt} %{deps} -g -c -zero-alloc-check default -dcse -dcheckmach -dump-into-file -O3)))
(action (run %{bin:ocamlopt.opt} %{deps} -g -c -zero-alloc-check default -dcse -dcheckmach -dump-into-file -O3 -warn-error +a)))

(rule
(enabled_if (and (= %{context_name} "main") %{ocaml-config:flambda}))
Expand Down Expand Up @@ -571,13 +571,13 @@
(alias runtest)
(enabled_if (= %{context_name} "main"))
(deps test_zero_alloc_opt1.ml)
(action (run %{bin:ocamlopt.opt} %{deps} -g -c -zero-alloc-check opt -dcse -dcheckmach -dump-into-file -O3)))
(action (run %{bin:ocamlopt.opt} %{deps} -g -c -zero-alloc-check opt -dcse -dcheckmach -dump-into-file -O3 -warn-error +a)))

(rule
(alias runtest)
(enabled_if (= %{context_name} "main"))
(deps test_zero_alloc_opt2.ml)
(action (run %{bin:ocamlopt.opt} %{deps} -g -c -zero-alloc-check opt -dcse -dcheckmach -dump-into-file -O3)))
(action (run %{bin:ocamlopt.opt} %{deps} -g -c -zero-alloc-check opt -dcse -dcheckmach -dump-into-file -O3 -warn-error +a)))

(rule
(enabled_if (= %{context_name} "main"))
Expand Down
2 changes: 1 addition & 1 deletion tests/backend/checkmach/gen/gen_dune.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ let () =
(alias runtest)
${enabled_if}
(deps ${deps})
(action (run %{bin:ocamlopt.opt} %{deps} -g -c ${extra_flags} -dcse -dcheckmach -dump-into-file -O3)))
(action (run %{bin:ocamlopt.opt} %{deps} -g -c ${extra_flags} -dcse -dcheckmach -dump-into-file -O3 -warn-error +a)))
|};
Buffer.output_buffer Out_channel.stdout buf
in
Expand Down
3 changes: 3 additions & 0 deletions tests/backend/checkmach/t.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -258,3 +258,6 @@ module Opt = struct
let[@zero_alloc opt] test x = x,x
let[@zero_alloc opt strict] test x = x,x
end

let[@zero_alloc assume never_returns_normally strict] test60 x = (x,x)
let[@zero_alloc assume strict never_returns_normally] test61 x = (x,x)
6 changes: 3 additions & 3 deletions tests/backend/checkmach/test_attribute_error_duplicate.output
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,15 @@ Warning 54 [duplicated-attribute]: the "zero_alloc" attribute is used more than

File "test_attribute_error_duplicate.ml", line 3, characters 5-15:
Warning 47 [attribute-payload]: illegal payload for attribute 'zero_alloc'.
It must be either 'assume', 'strict', 'opt', 'opt strict', 'assume strict', 'assume never_returns_normally', 'assume strict never_returns_normally', 'ignore' or empty
It must be either 'assume', 'strict', 'opt', 'opt strict', 'assume strict', 'assume never_returns_normally', 'assume never_returns_normally strict', 'ignore' or empty

File "test_attribute_error_duplicate.ml", line 4, characters 5-15:
Warning 47 [attribute-payload]: illegal payload for attribute 'zero_alloc'.
It must be either 'assume', 'strict', 'opt', 'opt strict', 'assume strict', 'assume never_returns_normally', 'assume strict never_returns_normally', 'ignore' or empty
It must be either 'assume', 'strict', 'opt', 'opt strict', 'assume strict', 'assume never_returns_normally', 'assume never_returns_normally strict', 'ignore' or empty

File "test_attribute_error_duplicate.ml", line 5, characters 5-15:
Warning 47 [attribute-payload]: illegal payload for attribute 'zero_alloc'.
It must be either 'assume', 'strict', 'opt', 'opt strict', 'assume strict', 'assume never_returns_normally', 'assume strict never_returns_normally', 'ignore' or empty
It must be either 'assume', 'strict', 'opt', 'opt strict', 'assume strict', 'assume never_returns_normally', 'assume never_returns_normally strict', 'ignore' or empty

File "test_attribute_error_duplicate.ml", line 1, characters 5-15:
Error: Annotation check for zero_alloc strict failed on function Test_attribute_error_duplicate.test1 (camlTest_attribute_error_duplicate.test1_HIDE_STAMP)