Skip to content

Commit 937f821

Browse files
authored
flambda-backend: Fix unsound modes in typecore (#2699)
* fix modes in typecore * fix tests
1 parent 79bb2d8 commit 937f821

File tree

6 files changed

+132
-72
lines changed

6 files changed

+132
-72
lines changed

testsuite/tests/typing-modes/class.ml

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -172,3 +172,25 @@ Line 3, characters 17-20:
172172
^^^
173173
Error: This value is nonportable but expected to be portable.
174174
|}]
175+
176+
let foo () =
177+
let x = object end in
178+
portable_use x
179+
[%%expect{|
180+
Line 3, characters 17-18:
181+
3 | portable_use x
182+
^
183+
Error: This value is nonportable but expected to be portable.
184+
|}]
185+
186+
class cla = object
187+
method m =
188+
let o = {< >} in
189+
portable_use o
190+
end
191+
[%%expect{|
192+
Line 4, characters 21-22:
193+
4 | portable_use o
194+
^
195+
Error: This value is nonportable but expected to be portable.
196+
|}]

testsuite/tests/typing-modes/lazy.ml

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,11 @@
66
let u =
77
let _x @ portable = lazy "hello" in
88
()
9-
(* CR zqian: this should fail. *)
109
[%%expect{|
11-
val u : unit = ()
10+
Line 2, characters 24-36:
11+
2 | let _x @ portable = lazy "hello" in
12+
^^^^^^^^^^^^
13+
Error: This value is nonportable but expected to be portable.
1214
|}]
1315

1416
(* lazy body is legacy *)

testsuite/tests/typing-modes/letop.ml

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
(* TEST
2+
expect;
3+
*)
4+
5+
let portable_use : _ @ portable -> unit = fun _ -> ()
6+
7+
let ( let* ) o f =
8+
match o with
9+
| None -> None
10+
| Some x -> f x
11+
12+
let ( and* ) a b =
13+
match a, b with
14+
| Some a, Some b -> Some (a, b)
15+
| _ -> None
16+
17+
[%%expect{|
18+
val portable_use : 'a @ portable -> unit = <fun>
19+
val ( let* ) : 'a option -> ('a -> 'b option) -> 'b option = <fun>
20+
val ( and* ) : 'a option -> 'b option -> ('a * 'b) option = <fun>
21+
|}]
22+
23+
(* bindings are required to be legacy *)
24+
let foo () =
25+
let* a = local_ "hello" in
26+
()
27+
[%%expect{|
28+
Line 2, characters 13-27:
29+
2 | let* a = local_ "hello" in
30+
^^^^^^^^^^^^^^
31+
Error: This value escapes its region.
32+
|}]
33+
34+
let foo () =
35+
let* a = Some "hello"
36+
and* b = local_ "hello" in
37+
()
38+
[%%expect{|
39+
Line 3, characters 13-27:
40+
3 | and* b = local_ "hello" in
41+
^^^^^^^^^^^^^^
42+
Error: This value escapes its region.
43+
|}]
44+
45+
(* Bindings are avialable as legacy *)
46+
let foo () =
47+
let* a = Some (fun x -> x)
48+
and* b = Some (fun x -> x) in
49+
portable_use a
50+
[%%expect{|
51+
Line 4, characters 17-18:
52+
4 | portable_use a
53+
^
54+
Error: This value is nonportable but expected to be portable.
55+
|}]
56+
57+
let foo () =
58+
let* a = Some (fun x -> x)
59+
and* b = Some (fun x -> x) in
60+
portable_use b
61+
[%%expect{|
62+
Line 4, characters 17-18:
63+
4 | portable_use b
64+
^
65+
Error: This value is nonportable but expected to be portable.
66+
|}]
67+
68+
(* Body required to be legacy *)
69+
let foo () =
70+
let _ =
71+
let* a = Some (fun x -> x) in
72+
local_ "hello"
73+
in
74+
()
75+
[%%expect{|
76+
Line 4, characters 8-22:
77+
4 | local_ "hello"
78+
^^^^^^^^^^^^^^
79+
Error: This value escapes its region.
80+
|}]
81+
82+
(* The whole letop is available as legacy *)
83+
let foo () =
84+
portable_use (
85+
let* a = Some (fun x -> x) in
86+
fun x -> x
87+
)
88+
[%%expect{|
89+
Lines 2-5, characters 17-5:
90+
2 | .................(
91+
3 | let* a = Some (fun x -> x) in
92+
4 | fun x -> x
93+
5 | )
94+
Error: This value is nonportable but expected to be portable.
95+
|}]

testsuite/tests/typing-modes/module.ml

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -117,9 +117,11 @@ val u : unit = ()
117117

118118
(* first class modules are produced at legacy *)
119119
let x = ((module M : SL) : _ @@ portable)
120-
(* CR zqian: this should fail *)
121120
[%%expect{|
122-
val x : (module SL) = <module>
121+
Line 1, characters 9-24:
122+
1 | let x = ((module M : SL) : _ @@ portable)
123+
^^^^^^^^^^^^^^^
124+
Error: This value is nonportable but expected to be portable.
123125
|}]
124126

125127
(* first class modules are consumed at legacy *)

testsuite/tests/typing-unique/unique_analysis.ml

Lines changed: 0 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -555,62 +555,6 @@ let foo () =
555555
val foo : unit -> unit = <fun>
556556
|}]
557557

558-
559-
(* testing Tpat_lazy *)
560-
let foo () =
561-
match lazy (unique_ "hello") with
562-
| (lazy y) as x -> ignore (shared_id x)
563-
[%%expect{|
564-
val foo : unit -> unit = <fun>
565-
|}]
566-
567-
568-
let foo () =
569-
match lazy (unique_ "hello") with
570-
| (lazy y) as x -> ignore (unique_id x)
571-
572-
[%%expect{|
573-
Line 3, characters 37-38:
574-
3 | | (lazy y) as x -> ignore (unique_id x)
575-
^
576-
Error: This value is used here as unique, but it has already been used:
577-
Line 3, characters 2-10:
578-
3 | | (lazy y) as x -> ignore (unique_id x)
579-
^^^^^^^^
580-
581-
|}]
582-
583-
type 'a r_lazy = {x_lazy : 'a Lazy.t; y : string}
584-
585-
let foo () =
586-
match {x_lazy = lazy (unique_ "hello"); y = "world"} with
587-
| {x_lazy = lazy y} as r -> ignore (unique_id r.x_lazy)
588-
[%%expect{|
589-
type 'a r_lazy = { x_lazy : 'a Lazy.t; y : string; }
590-
Line 5, characters 48-56:
591-
5 | | {x_lazy = lazy y} as r -> ignore (unique_id r.x_lazy)
592-
^^^^^^^^
593-
Error: This value is used here as unique, but it has already been used:
594-
Line 5, characters 14-20:
595-
5 | | {x_lazy = lazy y} as r -> ignore (unique_id r.x_lazy)
596-
^^^^^^
597-
598-
|}]
599-
600-
let foo () =
601-
match {x_lazy = lazy (unique_ "hello"); y = "world"} with
602-
| {x_lazy = lazy y} as r -> ignore (shared_id r.x_lazy)
603-
[%%expect{|
604-
val foo : unit -> unit = <fun>
605-
|}]
606-
607-
let foo () =
608-
match {x_lazy = lazy (unique_ "hello"); y = "world"} with
609-
| {x_lazy = lazy y} as r -> ignore (unique_id r.y)
610-
[%%expect{|
611-
val foo : unit -> unit = <fun>
612-
|}]
613-
614558
(* Testing modalities in records *)
615559
type r_shared = {x : string; y : string @@ shared many}
616560
[%%expect{|

typing/typecore.ml

Lines changed: 7 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -427,9 +427,6 @@ let meet_regional mode =
427427
let mode = Value.disallow_left mode in
428428
Value.meet [mode; (Value.max_with (Comonadic Areality) Regionality.regional)]
429429

430-
let meet_global mode =
431-
Value.meet [mode; (Value.max_with (Comonadic Areality) Regionality.global)]
432-
433430
let value_regional_to_local mode =
434431
mode
435432
|> value_to_alloc_r2l
@@ -477,10 +474,6 @@ let mode_with_position mode position =
477474
let mode_max_with_position position =
478475
{ mode_max with position }
479476

480-
let mode_global expected_mode =
481-
let mode = meet_global expected_mode.mode in
482-
{expected_mode with mode}
483-
484477
let mode_exclave expected_mode =
485478
let mode =
486479
Value.join_with (Comonadic Areality)
@@ -550,10 +543,6 @@ let mode_argument ~funct ~index ~position_and_mode ~partial_app marg =
550543
mode_tailcall_argument vmode, vmode
551544
end
552545

553-
let mode_lazy expected_mode =
554-
{ (mode_global expected_mode) with
555-
position = RTail (Regionality.disallow_left Regionality.global, FTail) }
556-
557546
(* expected_mode.closure_context explains why expected_mode.mode is low;
558547
shared_context explains why mode.uniqueness is high *)
559548
let submode ~loc ~env ?(reason = Other) ?shared_context mode expected_mode =
@@ -6045,6 +6034,7 @@ and type_expect_
60456034
raise(Error(loc, env, Instance_variable_not_mutable lab.txt))
60466035
end
60476036
| Pexp_override lst ->
6037+
submode ~loc ~env Value.legacy expected_mode;
60486038
let _ =
60496039
List.fold_right
60506040
(fun (lab, _) l ->
@@ -6177,13 +6167,14 @@ and type_expect_
61776167
exp_env = env;
61786168
}
61796169
| Pexp_lazy e ->
6170+
submode ~loc ~env Value.legacy expected_mode;
61806171
let ty = newgenvar (Jkind.value ~why:Lazy_expression) in
61816172
let to_unify = Predef.type_lazy_t ty in
61826173
with_explanation (fun () ->
61836174
unify_exp_types loc env to_unify (generic_instance ty_expected));
61846175
let env = Env.add_escape_lock Lazy env in
61856176
let env = Env.add_share_lock Lazy env in
6186-
let arg = type_expect env (mode_lazy expected_mode) e (mk_expected ty) in
6177+
let arg = type_expect env mode_legacy e (mk_expected ty) in
61876178
re {
61886179
exp_desc = Texp_lazy arg;
61896180
exp_loc = loc; exp_extra = [];
@@ -6192,6 +6183,7 @@ and type_expect_
61926183
exp_env = env;
61936184
}
61946185
| Pexp_object s ->
6186+
submode ~loc ~env Value.legacy expected_mode;
61956187
let desc, meths = !type_object env loc s in
61966188
rue {
61976189
exp_desc = Texp_object (desc, meths);
@@ -6253,6 +6245,8 @@ and type_expect_
62536245
type_newtype_expr ~loc ~env ~expected_mode ~rue ~attributes:sexp.pexp_attributes
62546246
name None sbody
62556247
| Pexp_pack m ->
6248+
(* CR zqian: pass [expected_mode] to [type_package] *)
6249+
submode ~loc ~env Value.legacy expected_mode;
62566250
let (p, fl) =
62576251
match get_desc (Ctype.expand_head env (instance ty_expected)) with
62586252
Tpackage (p, fl) ->
@@ -6292,6 +6286,7 @@ and type_expect_
62926286
exp_env = env;
62936287
}
62946288
| Pexp_letop{ let_ = slet; ands = sands; body = sbody } ->
6289+
submode ~loc ~env Value.legacy expected_mode;
62956290
let rec loop spat_acc ty_acc ty_acc_sort sands =
62966291
match sands with
62976292
| [] -> spat_acc, ty_acc, ty_acc_sort

0 commit comments

Comments
 (0)