Skip to content

Commit c578c62

Browse files
authored
flambda-backend: Fix function return layout bug (#2471)
* Commit failing test * Fix * Add comment * Fix my original attempt at a fix * chamelon * make fmt
1 parent 16559a2 commit c578c62

File tree

10 files changed

+58
-22
lines changed

10 files changed

+58
-22
lines changed

lambda/translcore.ml

+5-9
Original file line numberDiff line numberDiff line change
@@ -1293,14 +1293,9 @@ and transl_function_without_attributes
12931293
match body with
12941294
| Tfunction_body exp ->
12951295
layout_exp return_sort exp
1296-
| Tfunction_cases { fc_cases = { c_rhs; _ } :: _ } ->
1297-
layout_exp return_sort c_rhs
1298-
| Tfunction_cases { fc_cases = [] } ->
1299-
(* ppxes can generate empty function cases, which compiles to
1300-
a function that always raises Match_failure. We try less
1301-
hard to calculate a detailed layout that the middle-end can
1302-
use for optimizations. *)
1303-
layout_of_sort loc return_sort
1296+
| Tfunction_cases cases ->
1297+
layout cases.fc_env cases.fc_loc return_sort cases.fc_ret_type
1298+
13041299
in
13051300
match
13061301
transl_tupled_function ~scopes loc params body
@@ -2031,7 +2026,8 @@ and transl_letop ~scopes loc env let_ ands param param_sort case case_sort
20312026
{ fc_cases = [case]; fc_param = param; fc_partial = partial;
20322027
fc_loc = ghost_loc; fc_exp_extra = None; fc_attributes = [];
20332028
fc_arg_mode = Mode.Alloc.disallow_right Mode.Alloc.legacy;
2034-
fc_arg_sort = param_sort;
2029+
fc_arg_sort = param_sort; fc_env = env;
2030+
fc_ret_type = case.c_rhs.exp_type;
20352031
}))
20362032
in
20372033
let attr = function_attribute_disallowing_arity_fusion in
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
(* TEST
2+
flambda2;
3+
native;
4+
*)
5+
6+
(* This is a regression test, see PR #2471 in ocaml-flambda/flambda-backend *)
7+
8+
[@@@ocaml.flambda_o3]
9+
10+
type _ value =
11+
| Int : int value
12+
| Float : float value
13+
14+
let[@inline never] get (type a) : a value -> a = function
15+
| Int -> 3
16+
| Float -> 3.
17+
18+
let[@inline] update (type a) (v : a value) (x : a) : a =
19+
match v with
20+
| Int -> x + 1
21+
| Float -> x +. 1.
22+
23+
let run x = update x (get x)
24+
25+
let (_ : float) = run Float

testsuite/tests/typing-layouts/basics_alpha.ml

+2-2
Original file line numberDiff line numberDiff line change
@@ -376,9 +376,9 @@ let id5 : 'a void5 -> 'a void5 = function
376376

377377
[%%expect{|
378378
type ('a : void) void5 = Void5 of 'a
379-
Line 4, characters 15-22:
379+
Lines 3-4, characters 33-22:
380+
3 | .................................function
380381
4 | | Void5 x -> Void5 x
381-
^^^^^^^
382382
Error: Non-value detected in [value_kind].
383383
Please report this error to the Jane Street compilers team.
384384
The layout of 'a is void, because

testsuite/tests/typing-layouts/void_alpha.ml

+9-6
Original file line numberDiff line numberDiff line change
@@ -211,14 +211,17 @@ type void_variant =
211211
}
212212
val r : '_weak2 list ref = {contents = []}
213213
val cons_r : '_weak2 -> unit = <fun>
214-
Lines 19-25, characters 5-23:
215-
19 | .....A ((cons_r 10; a1),
214+
Lines 17-35, characters 10-27:
215+
17 | ..........function
216+
18 | | A (a1, a2, x, v, z, b1, b2) ->
217+
19 | A ((cons_r 10; a1),
216218
20 | (cons_r 8; {v = ((cons_r 9; a2).v)}),
217219
21 | (cons_r 7; x),
218-
22 | (cons_r 5; {v = ((cons_r 6; v).v)}),
219-
23 | (cons_r 4; z),
220-
24 | (cons_r 2; {v = ((cons_r 3; b1).v)}),
221-
25 | (cons_r 1; b2))
220+
...
221+
32 | v = (cons_r 5; {v = ((cons_r 6; v).v)});
222+
33 | z = (cons_r 4; z);
223+
34 | b1 = (cons_r 2; {v = ((cons_r 3; b1).v)});
224+
35 | b2 = (cons_r 1; b2)}
222225
Error: Non-value detected in [value_kind].
223226
Please report this error to the Jane Street compilers team.
224227
The layout of t_void is void, because

typing/printtyped.ml

+1-1
Original file line numberDiff line numberDiff line change
@@ -367,7 +367,7 @@ and function_body i ppf (body : function_body) =
367367
expression (i+1) ppf e
368368
| Tfunction_cases
369369
{ fc_cases; fc_loc; fc_exp_extra; fc_attributes; fc_arg_mode;
370-
fc_arg_sort; fc_param = _; fc_partial = _; }
370+
fc_arg_sort; fc_param = _; fc_partial = _; fc_env = _; fc_ret_type = _ }
371371
->
372372
line i ppf "Tfunction_cases %a\n" fmt_location fc_loc;
373373
alloc_mode i ppf fc_arg_mode;

typing/tast_iterator.ml

+3-2
Original file line numberDiff line numberDiff line change
@@ -298,11 +298,12 @@ let function_body sub body =
298298
match body with
299299
| Tfunction_body body ->
300300
sub.expr sub body
301-
| Tfunction_cases { fc_cases; fc_exp_extra; fc_loc; fc_attributes } ->
301+
| Tfunction_cases { fc_cases; fc_exp_extra; fc_loc; fc_attributes; fc_env } ->
302302
List.iter (sub.case sub) fc_cases;
303303
Option.iter (extra sub) fc_exp_extra;
304304
sub.location sub fc_loc;
305-
sub.attributes sub fc_attributes
305+
sub.attributes sub fc_attributes;
306+
sub.env sub fc_env
306307

307308
let expr sub {exp_loc; exp_extra; exp_desc; exp_env; exp_attributes; _} =
308309
let extra x = extra sub x in

typing/tast_mapper.ml

+3-2
Original file line numberDiff line numberDiff line change
@@ -374,15 +374,16 @@ let function_body sub body =
374374
Tfunction_body (sub.expr sub body)
375375
| Tfunction_cases
376376
{ fc_cases; fc_partial; fc_param; fc_loc; fc_exp_extra; fc_attributes;
377-
fc_arg_mode; fc_arg_sort; }
377+
fc_arg_mode; fc_arg_sort; fc_env; fc_ret_type; }
378378
->
379379
let fc_loc = sub.location sub fc_loc in
380380
let fc_attributes = sub.attributes sub fc_attributes in
381381
let fc_cases = List.map (sub.case sub) fc_cases in
382382
let fc_exp_extra = Option.map (extra sub) fc_exp_extra in
383+
let fc_env = sub.env sub fc_env in
383384
Tfunction_cases
384385
{ fc_cases; fc_partial; fc_param; fc_loc; fc_exp_extra; fc_attributes;
385-
fc_arg_mode; fc_arg_sort; }
386+
fc_arg_mode; fc_arg_sort; fc_env; fc_ret_type; }
386387

387388
let expr sub x =
388389
let extra x = extra sub x in

typing/typecore.ml

+3
Original file line numberDiff line numberDiff line change
@@ -7446,6 +7446,7 @@ and type_argument ?explanation ?recarg env (mode : expected_mode) sarg
74467446
body =
74477447
Tfunction_cases
74487448
{ fc_cases = cases; fc_partial = Total; fc_param = param;
7449+
fc_env = env; fc_ret_type = ty_res;
74497450
fc_loc = cases_loc; fc_exp_extra = None;
74507451
fc_attributes = []; fc_arg_mode = Alloc.disallow_right marg;
74517452
fc_arg_sort = arg_sort;
@@ -8186,6 +8187,8 @@ and type_function_cases_expect
81868187
fc_param = param;
81878188
fc_loc = loc;
81888189
fc_exp_extra = None;
8190+
fc_env = env;
8191+
fc_ret_type = ty_ret;
81898192
fc_attributes = [];
81908193
fc_arg_mode = Alloc.disallow_right arg_mode;
81918194
fc_arg_sort = arg_sort;

typing/typedtree.ml

+2
Original file line numberDiff line numberDiff line change
@@ -243,8 +243,10 @@ and function_body =
243243

244244
and function_cases =
245245
{ fc_cases: value case list;
246+
fc_env : Env.t;
246247
fc_arg_mode: Mode.Alloc.l;
247248
fc_arg_sort: Jkind.sort;
249+
fc_ret_type : Types.type_expr;
248250
fc_partial: partial;
249251
fc_param: Ident.t;
250252
fc_loc: Location.t;

typing/typedtree.mli

+5
Original file line numberDiff line numberDiff line change
@@ -465,8 +465,13 @@ and function_body =
465465

466466
and function_cases =
467467
{ fc_cases: value case list;
468+
fc_env : Env.t;
469+
(** [fc_env] contains entries from all parameters except
470+
for the last one being matched by the cases.
471+
*)
468472
fc_arg_mode: Mode.Alloc.l;
469473
fc_arg_sort: Jkind.sort;
474+
fc_ret_type : Types.type_expr;
470475
fc_partial: partial;
471476
fc_param: Ident.t;
472477
fc_loc: Location.t;

0 commit comments

Comments
 (0)