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.

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

Unboxed tuples #2879

Merged
merged 4 commits into from
Sep 5, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
Prev Previous commit
Next Next commit
Improve some comments
  • Loading branch information
ccasin committed Sep 5, 2024
commit 284db5ede2bb13e7dd85fd709e5328b8342d02e5
19 changes: 10 additions & 9 deletions ocaml/testsuite/tests/typing-layouts-products/basics.ml
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ Error: This type #(int * int) should be an instance of type
But the layout of #(int * int) must be a sublayout of value & bits64
because of the definition of t3 at line 1, characters 0-34.
|}]
(* CR layouts 7.1: The above error should identify the component of the product
(* CR layouts v7.1: The above error should identify the component of the product
that is problematic. *)

(* some mutually recusive types *)
Expand Down Expand Up @@ -122,7 +122,7 @@ Error: This type #(int * int64) should be an instance of type
because of the annotation on 'a in the declaration of the type
t6_wrong.
|}]
(* CR layouts 7.1: The above error should identify the component of the product
(* CR layouts v7.1: The above error should identify the component of the product
that is problematic. *)

(* Just like t6/t7, but with the annotation on the other (the order doesn't
Expand Down Expand Up @@ -600,7 +600,7 @@ Error: This value escapes its region.
(*********************)
(* Test 9: externals *)

(* CR layouts 7.1: Unboxed products should be allowed for some primitives, like
(* CR layouts v7.1: Unboxed products should be allowed for some primitives, like
%identity *)

type t_product : value & value
Expand Down Expand Up @@ -685,7 +685,7 @@ Error: Unboxed product layouts are not yet supported as arguments to
The layout of this argument is value & value.
|}]

(* CR layouts 7.1: Unboxed products should be allowed for some primitives, like
(* CR layouts v7.1: Unboxed products should be allowed for some primitives, like
%identity *)

(***********************************)
Expand Down Expand Up @@ -778,12 +778,13 @@ type t3 = #(int * bool) array
type t4 = #(string * #(float# * bool option)) array
|}]

(* CR layouts 7.1: This example demonstrates a bug in type inference that we
(* CR layouts v7.1: This example demonstrates a bug in type inference that we
should fix. The issue is the way typing of tuple expressions works - we
create type variables at generic level (I'm confused about why not current
level) and then constrain them by the expected type. This will fail if it
requires to refine the kind, because we don't allow refining kinds at generic
level. Further thinking required. *)
create type variables at generic level and then constrain them by the
expected type. This will fail if it requires to refine the kind, because we
don't allow refining kinds at generic level. We need to remove the
restriction that kinds of things at generic level can't be modified, but that
is orthogonal to unboxed tuples so we leave in this sad bug for now. *)
let _ = [| #(1,2) |]
[%%expect{|
Line 1, characters 11-17:
Expand Down
2 changes: 1 addition & 1 deletion ocaml/typing/jkind.ml
Original file line number Diff line number Diff line change
Expand Up @@ -1831,7 +1831,7 @@ let is_nary_product n t =
in
if Misc.Le_result.is_le (Jkind_desc.sub t.jkind bound)
then
(* CR layouts 7.1: The histories here are wrong (we are giving each
(* CR layouts v7.1: The histories here are wrong (we are giving each
component the history of the whole product). They don't show up in
errors, so it's fine for now, but we'll probably need to fix this as
part of improving errors around products. A couple options: re-work the
Expand Down
2 changes: 1 addition & 1 deletion ocaml/typing/typedecl.ml
Original file line number Diff line number Diff line change
Expand Up @@ -2947,7 +2947,7 @@ let unexpected_layout_any_check prim env cty ty =
[@layout_poly].

An exception is raised if any of these checks fails. *)
(* CR layouts 7.1: additionally, we do not allow externals to have unboxed
(* CR layouts v7.1: additionally, we do not allow externals to have unboxed
product args/returns. Right now this restriction is in place for all
externals, but we should be able to relax it for some primitives that are
implemented by the compiler, like %identity. Enforcement for [@layout_poly]
Expand Down
2 changes: 1 addition & 1 deletion ocaml/typing/typeopt.ml
Original file line number Diff line number Diff line change
Expand Up @@ -714,7 +714,7 @@ let[@inline always] rec layout_of_const_sort_generic ~value_kind ~error
Language_extension.(is_enabled Small_numbers) ->
Lambda.Punboxed_float Pfloat32
| Product consts when Language_extension.(is_at_least Layouts Beta) ->
(* CR layouts 7.1: assess whether it is important for performance to support
(* CR layouts v7.1: assess whether it is important for performance to support
deep value_kinds here *)
Lambda.Punboxed_product
(List.map (layout_of_const_sort_generic ~value_kind:(lazy Pgenval) ~error)
Expand Down
Loading