Skip to content

Commit

Permalink
add some tests, address some CRs
Browse files Browse the repository at this point in the history
  • Loading branch information
rtjoa committed Nov 5, 2024
1 parent 7d312e2 commit 70be641
Show file tree
Hide file tree
Showing 4 changed files with 314 additions and 9 deletions.
13 changes: 13 additions & 0 deletions chamelon/minimizer/removedeadcode.ml
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,8 @@ let rec var_from_pat pat_desc acc =
(List.map (fun (_, pat, _) -> pat) fields)
| O (Tpat_record (r, _)) ->
List.fold_left (fun l (_, _, pat) -> var_from_pat pat.pat_desc l) acc r
| O (Tpat_record_unboxed_product (r, _)) ->
List.fold_left (fun l (_, _, pat) -> var_from_pat pat.pat_desc l) acc r
| O (Tpat_or (p1, p2, _)) ->
var_from_pat p1.pat_desc (var_from_pat p2.pat_desc acc)
| O (Tpat_lazy pat) -> var_from_pat pat.pat_desc acc
Expand Down Expand Up @@ -150,6 +152,17 @@ let rec rem_in_pat str pat should_remove =
r,
a1 );
}
| O (Tpat_record_unboxed_product (r, a1)) ->
{
pat with
pat_desc =
Tpat_record_unboxed_product
( List.map
(fun (e1, e2, pat) ->
(e1, e2, rem_in_pat str pat should_remove))
r,
a1 );
}
| O (Tpat_or (p1, p2, a1)) ->
let p1 = rem_in_pat str p1 should_remove in
let p2 = rem_in_pat str p2 should_remove in
Expand Down
228 changes: 228 additions & 0 deletions testsuite/tests/typing-layouts-unboxed-records/recursive.ml
Original file line number Diff line number Diff line change
@@ -0,0 +1,228 @@
(* TEST
flambda2;
include stdlib_upstream_compatible;
flags = "-extension layouts_alpha";
{
expect;
}
*)

(* CR layouts 7.2: figure out the story for recursive unboxed products.
Consider that the following is allowed upstream:
type t = { t : t } [@@unboxed]
We should also give good errors for infinite-size unboxed records (see the test at the
bottom of this file with a depth-100 kind).
*)

(************************************)
(* Basic recursive unboxed products *)

type t : value = #{ t : t }
[%%expect{|
type t = #{ t : t; }
|}]

type t : float64 = #{ t : t }
[%%expect{|
type t = #{ t : t; }
|}]


type t : value = #{ t : t }
[%%expect{|
type t = #{ t : t; }
|}]

type bad = #{ bad : bad ; i : int}
[%%expect{|
Line 1, characters 0-34:
1 | type bad = #{ bad : bad ; i : int}
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Error:
The layout of bad is any & any
because it is an unboxed record.
But the layout of bad must be representable
because it is the type of record field bad.
|}]

type bad = #{ bad : bad }
[%%expect{|
Line 1, characters 0-25:
1 | type bad = #{ bad : bad }
^^^^^^^^^^^^^^^^^^^^^^^^^
Error:
The layout of bad is any
because a dummy kind of any is used to check mutually recursive datatypes.
Please notify the Jane Street compilers group if you see this output.
But the layout of bad must be representable
because it is the type of record field bad.
|}]

type a_bad = #{ b_bad : b_bad }
and b_bad = #{ a_bad : a_bad }
[%%expect{|
Line 1, characters 0-31:
1 | type a_bad = #{ b_bad : b_bad }
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Error:
The layout of a_bad is any
because a dummy kind of any is used to check mutually recursive datatypes.
Please notify the Jane Street compilers group if you see this output.
But the layout of a_bad must be representable
because it is the type of record field a_bad.
|}]

type bad : any = #{ bad : bad }
[%%expect{|
Line 1, characters 0-31:
1 | type bad : any = #{ bad : bad }
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Error:
The layout of bad is any
because of the annotation on the declaration of the type bad.
But the layout of bad must be representable
because it is the type of record field bad.
|}]

type 'a id = #{ a : 'a }
type bad = bad id
[%%expect{|
type 'a id = #{ a : 'a; }
Line 2, characters 0-17:
2 | type bad = bad id
^^^^^^^^^^^^^^^^^
Error: The type abbreviation "bad" is cyclic:
"bad" = "bad id",
"bad id" contains "bad"
|}]


type 'a bad = #{ bad : 'a bad ; u : 'a}
[%%expect{|
Line 1, characters 0-39:
1 | type 'a bad = #{ bad : 'a bad ; u : 'a}
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Error:
The layout of 'a bad is any & any
because it is an unboxed record.
But the layout of 'a bad must be representable
because it is the type of record field bad.
|}]

type 'a bad = { bad : 'a bad ; u : 'a}
[%%expect{|
type 'a bad = { bad : 'a bad; u : 'a; }
|}]


(***************************************)
(* Recursive unboxed records with void *)

type t_void : void

type ('a : void) t = #{ x : 'a ; y : t_void }
[%%expect{|
type t_void : void
type ('a : void) t = #{ x : 'a; y : t_void; }
|}]

type t = { x : t_void } [@@unboxed]
[%%expect{|
type t = { x : t_void; } [@@unboxed]
|}]

type bad : void = #{ bad : bad }
[%%expect{|
type bad = #{ bad : bad; }
|}]

type ('a : void) bad = #{ bad : 'a bad ; u : 'a}
[%%expect{|
Line 1, characters 0-49:
1 | type ('a : void) bad = #{ bad : 'a bad ; u : 'a}
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Error:
The layout of 'a bad is any & any
because it is an unboxed record.
But the layout of 'a bad must be representable
because it is the type of record field bad.
|}]


(****************************)
(* A particularly bad error *)

type bad : float64 = #{ bad : bad ; i : int}
[%%expect{|
Line 1, characters 0-44:
1 | type bad : float64 = #{ bad : bad ; i : int}
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Error: The layout of type "bad" is ((((((((((((((((((((((((((((((((((((
(
(
(
(
(
(
(
(
(
(
(
(
(
(
(
(
(
(
(
(
(
(
(
(
(
(
(
(
(
(
(
(
(
(
(
(
(
(
(
(
(
(
(
(
(
(
(
(
(
(
(
(
(
(
(
(
(
(
(
(
(
(
(
(float64 & value) & value) & value) & value) & value) & value) & value) & value) & value) & value) & value) & value) & value) & value) & value) & value) & value) & value) & value) & value) & value) & value) & value) & value) & value) & value) & value) & value) & value) & value) & value) & value) & value) & value) & value) & value) & value) & value) & value) & value) & value) & value) & value) & value) & value) & value) & value) & value) & value) & value) & value) & value) & value) & value) & value) & value) & value) & value) & value) & value) & value) & value) & value) & value) & value) & value) & value) & value) & value) & value) & value) & value) & value) & value) & value) & value) & value) & value) & value) & value) & value) & value) & value) & value) & value) & value) & value) & value) & value) & value) & value) & value) & value) & value) & value) & value) & value) & value) & value) & value) & value
because it is an unboxed record.
But the layout of type "bad" must be a sublayout of float64
because of the annotation on the declaration of the type bad.
|}]
65 changes: 65 additions & 0 deletions testsuite/tests/typing-layouts-unboxed-records/unique.ml
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
(* TEST
flambda2;
include stdlib_upstream_compatible;
flags = "-extension layouts_alpha -extension unique";
{
expect;
}
*)

(* Uniqueness tests *)

type t = #{ x : string ; y : string }
let use_string (_s @ unique) = ()
let mk : unit -> t @ unique = fun () -> #{ x = "hi"; y = "hi" }
[%%expect{|
type t = #{ x : string; y : string; }
val use_string : ('a : value_or_null). 'a @ unique -> unit = <fun>
val mk : unit -> t @ unique = <fun>
|}]

(* Can access different fields *)
let () =
let t = mk () in
use_string t.#x;
use_string t.#y
[%%expect{|
|}]

let () =
let #{ x ; y } = mk () in
use_string x;
use_string y
[%%expect{|
|}]

(* Cannot access the same field twice *)
let () =
let t = mk () in
use_string t.#x;
use_string t.#x
[%%expect{|
Line 4, characters 13-17:
4 | use_string t.#x
^^^^
Error: This value is used here, but it has already been used as unique:
Line 3, characters 13-17:
3 | use_string t.#x;
^^^^

|}]

let () =
let #{ x ; y = _ } = mk () in
use_string x;
use_string x
[%%expect{|
Line 4, characters 13-14:
4 | use_string x
^
Error: This value is used here, but it has already been used as unique:
Line 3, characters 13-14:
3 | use_string x;
^

|}]
17 changes: 8 additions & 9 deletions typing/typedecl.ml
Original file line number Diff line number Diff line change
Expand Up @@ -779,10 +779,10 @@ let transl_declaration env sdecl (id, uid) =
| Ptype_variant [{pcd_args = Pcstr_tuple [_]; _}]
| Ptype_variant [{pcd_args = Pcstr_record [{pld_mutable=Immutable; _}]; _}]
| Ptype_record [{pld_mutable=Immutable; _}] ->
lazy (Option.value unboxed_attr ~default:!Clflags.unboxed_types),
Option.value unboxed_attr ~default:!Clflags.unboxed_types,
Option.is_none unboxed_attr
| Ptype_record_unboxed_product _ -> lazy (assert false), true
| _ -> lazy false, false (* Not unboxable, mark as boxed *)
| Ptype_record_unboxed_product _ -> false, false
| _ -> false, false (* Not unboxable, mark as boxed *)
in
verify_unboxed_attr unboxed_attr sdecl;
let jkind_from_annotation, jkind_annotation =
Expand Down Expand Up @@ -857,7 +857,7 @@ let transl_declaration env sdecl (id, uid) =
let name = Ident.create_local scstr.pcd_name.txt in
let attributes = scstr.pcd_attributes in
let tvars, targs, tret_type, args, ret_type =
make_constructor ~unboxed:(Lazy.force unbox) env scstr.pcd_loc
make_constructor ~unboxed:unbox env scstr.pcd_loc
~cstr_path:(Path.Pident name) ~type_path:path params
scstr.pcd_vars scstr.pcd_args scstr.pcd_res
in
Expand Down Expand Up @@ -887,7 +887,7 @@ let transl_declaration env sdecl (id, uid) =
in
let tcstrs, cstrs = List.split (List.map make_cstr scstrs) in
let rep, jkind =
if (Lazy.force unbox) then
if unbox then
Variant_unboxed, any
else
(* We mark all arg jkinds "any" here. They are updated later,
Expand All @@ -914,15 +914,15 @@ let transl_declaration env sdecl (id, uid) =
let lbls, lbls' =
(* CR layouts: we forbid [@@unboxed] records from being
non-value, see comment in [check_representable]. *)
transl_labels ~new_var_jkind:Any ~allow_unboxed:(not (Lazy.force unbox))
env None true lbls (Record { unboxed = (Lazy.force unbox) })
transl_labels ~new_var_jkind:Any ~allow_unboxed:(not unbox)
env None true lbls (Record { unboxed = unbox })
in
let rep, jkind =
(* Note this is inaccurate, using `Record_boxed` in cases where the
correct representation is [Record_float], [Record_ufloat], or
[Record_mixed]. Those cases are fixed up after we can get
accurate jkinds for the fields, in [update_decl_jkind]. *)
if (Lazy.force unbox) then
if unbox then
Record_unboxed, any
else
Record_boxed (Array.make (List.length lbls) any),
Expand Down Expand Up @@ -2936,7 +2936,6 @@ let check_unboxable env loc ty =
try match get_desc ty with
| Tconstr (p, _, _) ->
let tydecl = Env.find_type p env in
(* CR rtjoa: understand what type_unboxed_default of unboxed records should be *)
if tydecl.type_unboxed_default then
Path.Set.add p acc
else acc
Expand Down

0 comments on commit 70be641

Please sign in to comment.