Skip to content

Commit b6e3c58

Browse files
authored
flambda-backend: Improve mode checking related to allocation (#2366)
* remove mode_subcomponent * refactor option_some * improve Texp_record * improve Texp_field * improve Texp_field for unique analysis * better story of field projection with boxing * address comments
1 parent f678999 commit b6e3c58

File tree

15 files changed

+215
-150
lines changed

15 files changed

+215
-150
lines changed

lambda/translcore.ml

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -539,7 +539,7 @@ and transl_exp0 ~in_new_scope ~scopes sort e =
539539
transl_record ~scopes e.exp_loc e.exp_env
540540
(Option.map transl_alloc_mode_r alloc_mode)
541541
fields representation extended_expression
542-
| Texp_field(arg, id, lbl, _, alloc_mode) ->
542+
| Texp_field(arg, id, lbl, float) ->
543543
let targ = transl_exp ~scopes Jkind.Sort.for_record arg in
544544
let sem =
545545
match lbl.lbl_mut with
@@ -554,7 +554,12 @@ and transl_exp0 ~in_new_scope ~scopes sort e =
554554
of_location ~scopes e.exp_loc)
555555
| Record_unboxed | Record_inlined (_, Variant_unboxed) -> targ
556556
| Record_float ->
557-
let mode = transl_alloc_mode_r (Option.get alloc_mode) in
557+
let alloc_mode =
558+
match float with
559+
| Boxing (alloc_mode, _) -> alloc_mode
560+
| Non_boxing _ -> assert false
561+
in
562+
let mode = transl_alloc_mode_r alloc_mode in
558563
Lprim (Pfloatfield (lbl.lbl_pos, sem, mode), [targ],
559564
of_location ~scopes e.exp_loc)
560565
| Record_ufloat ->

testsuite/tests/typing-local/local.ml

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2945,3 +2945,15 @@ let () = foo (local_ M_constructor)
29452945
let () = foo_f (local_ (fun M_constructor -> ()))
29462946
[%%expect{|
29472947
|}]
2948+
2949+
type r = {global_ x : string; y : string}
2950+
2951+
let foo () =
2952+
let local_ y = "world" in
2953+
let local_ r = {x = "hello"; y} in
2954+
(* Only using r.x, which is global. So the whole return is global and OK. *)
2955+
{r with y = "foo!" }
2956+
[%%expect{|
2957+
type r = { global_ x : string; y : string; }
2958+
val foo : unit -> r = <fun>
2959+
|}]

testsuite/tests/typing-modal-kinds/basics.ml

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -181,6 +181,22 @@ Line 2, characters 45-46:
181181
Error: This value escapes its region
182182
|}]
183183

184+
type r = {x : float; y : float}
185+
186+
let foo () =
187+
let local_ r = {x = 3.0; y = 4.0} in
188+
(* [r.x] is allocated global and can escape. *)
189+
r.x
190+
191+
(* CR layouts v2.8: this should succeed *)
192+
[%%expect{|
193+
type r = { x : float; y : float; }
194+
Line 6, characters 2-5:
195+
6 | r.x
196+
^^^
197+
Error: This value escapes its region
198+
|}]
199+
184200
let function_escape = let local_ x : int -> int = fun y -> y in x
185201

186202
[%%expect{|

testsuite/tests/typing-unique/unique_analysis.ml

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -781,3 +781,47 @@ Line 4, characters 20-21:
781781

782782
|}]
783783

784+
type r = {x : float; y : float}
785+
786+
(* CR zqian: The following should pass but doesn't, because the uniqueness
787+
analysis doesn't support mode crossing. The following involes sequencing the
788+
maybe_unique usage of [r.x] and the maybe_unique usage of [r] as a whole.
789+
Sequencing them will force both to be shared and many. The [unique_use] in
790+
[r.x] is mode-crossed (being an unboxed float) so is fine. The [unique_use]
791+
in [r] cannot cross mode, and forcing it causes error. *)
792+
793+
let foo () =
794+
let r = {x = 3.0; y = 5.0} in
795+
let x = r.x in
796+
ignore (unique_id r);
797+
(* [x] is allocated fresh, unrelated to [r]. *)
798+
ignore (unique_id x)
799+
[%%expect{|
800+
type r = { x : float; y : float; }
801+
Line 13, characters 20-21:
802+
13 | ignore (unique_id r);
803+
^
804+
Error: This value is used here,
805+
but part of it has already been used as unique:
806+
Line 12, characters 10-13:
807+
12 | let x = r.x in
808+
^^^
809+
810+
|}]
811+
812+
let foo () =
813+
let r = {x = 3.0; y = 5.0} in
814+
ignore (unique_id r);
815+
(* but projection still uses [r]'s mem block, of course *)
816+
let x = r.x in
817+
ignore (unique_id x)
818+
[%%expect{|
819+
Line 5, characters 10-11:
820+
5 | let x = r.x in
821+
^
822+
Error: This value is read from here, but it has already been used as unique:
823+
Line 3, characters 20-21:
824+
3 | ignore (unique_id r);
825+
^
826+
827+
|}]

typing/printtyped.ml

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -467,9 +467,8 @@ and expression i ppf x =
467467
record_representation (i+1) ppf representation;
468468
line i ppf "extended_expression =\n";
469469
option (i+1) expression ppf extended_expression;
470-
| Texp_field (e, li, _, _, am) ->
470+
| Texp_field (e, li, _, _) ->
471471
line i ppf "Texp_field\n";
472-
alloc_mode_option i ppf am;
473472
expression i ppf e;
474473
longident i ppf li;
475474
| Texp_setfield (e1, am, li, _, e2) ->

typing/rec_check.ml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -720,7 +720,7 @@ let rec expression : Typedtree.expression -> term_judg =
720720
join [
721721
expression e1 << Dereference
722722
]
723-
| Texp_field (e, _, _, _, _) ->
723+
| Texp_field (e, _, _, _) ->
724724
(*
725725
G |- e: m[Dereference]
726726
-----------------------

typing/tast_iterator.ml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -329,7 +329,7 @@ let expr sub {exp_loc; exp_extra; exp_desc; exp_env; exp_attributes; _} =
329329
| _, Overridden (lid, exp) -> iter_loc sub lid; sub.expr sub exp)
330330
fields;
331331
Option.iter (sub.expr sub) extended_expression;
332-
| Texp_field (exp, lid, _, _, _) ->
332+
| Texp_field (exp, lid, _, _) ->
333333
iter_loc sub lid;
334334
sub.expr sub exp
335335
| Texp_setfield (exp1, _, lid, _, exp2) ->

typing/tast_mapper.ml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -477,8 +477,8 @@ let expr sub x =
477477
extended_expression = Option.map (sub.expr sub) extended_expression;
478478
alloc_mode
479479
}
480-
| Texp_field (exp, lid, ld, mode, am) ->
481-
Texp_field (sub.expr sub exp, map_loc sub lid, ld, mode, am)
480+
| Texp_field (exp, lid, ld, float) ->
481+
Texp_field (sub.expr sub exp, map_loc sub lid, ld, float)
482482
| Texp_setfield (exp1, am, lid, ld, exp2) ->
483483
Texp_setfield (
484484
sub.expr sub exp1,

typing/typeclass.ml

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1303,17 +1303,14 @@ and class_expr_aux cl_num val_env met_env virt self_scope scl =
13031303
let arg = Typecore.type_argument val_env sarg ty ty0 in
13041304
arg, Jkind.Sort.value
13051305
else
1306-
let ty' = Typecore.extract_option_type val_env ty
1307-
and ty0' = Typecore.extract_option_type val_env ty0 in
1308-
let arg = Typecore.type_argument val_env sarg ty' ty0' in
1309-
Typecore.option_some val_env arg Mode.Value.legacy,
1306+
Typecore.type_option_some val_env sarg ty ty0,
13101307
(* CR layouts v5: Change the sort when options can hold
13111308
non-values. *)
13121309
Jkind.Sort.value
13131310
)
13141311
in
13151312
let eliminate_optional_arg () =
1316-
Arg (Typecore.option_none val_env ty0 Location.none,
1313+
Arg (Typecore.type_option_none val_env ty0 Location.none,
13171314
(* CR layouts v5: Change the sort when options can hold
13181315
non-values. *)
13191316
Jkind.Sort.value

0 commit comments

Comments
 (0)