Skip to content

Commit 994fab5

Browse files
authored
Fix boolean functions tail call position bug (#1957)
1 parent 2d88a48 commit 994fab5

File tree

2 files changed

+54
-23
lines changed

2 files changed

+54
-23
lines changed

ocaml/testsuite/tests/typing-local/local.ml

+26
Original file line numberDiff line numberDiff line change
@@ -2048,6 +2048,32 @@ Error: This local value escapes its region
20482048
Hint: This argument cannot be local, because this is a tail call
20492049
|}]
20502050
2051+
(* boolean operator when at tail of function makes the function local-returning
2052+
if its RHS is local-returning *)
2053+
let foo () = exclave_ let local_ _x = "hello" in true
2054+
let testboo3 () = true && (foo ())
2055+
[%%expect{|
2056+
val foo : unit -> local_ bool = <fun>
2057+
val testboo3 : unit -> local_ bool = <fun>
2058+
|}]
2059+
2060+
(* Test from Nathanaëlle Courant.
2061+
User can define strange AND. Supposedly [strange_and] will look at its first
2062+
arguments, and returns [None] or tailcall on second argument accordingly.
2063+
The second argument should not cross modes in generall. *)
2064+
external strange_and : bool -> 'a option -> 'a option = "%sequand"
2065+
2066+
let testboo4 () =
2067+
let local_ x = Some "hello" in
2068+
strange_and true x
2069+
[%%expect{|
2070+
external strange_and : bool -> 'a option -> 'a option = "%sequand"
2071+
Line 5, characters 19-20:
2072+
5 | strange_and true x
2073+
^
2074+
Error: This value escapes its region
2075+
|}]
2076+
20512077
(* mode-crossing using unary + *)
20522078
let promote (local_ x) = +x
20532079
[%%expect{|

ocaml/typing/typecore.ml

+28-23
Original file line numberDiff line numberDiff line change
@@ -331,20 +331,20 @@ type expected_mode =
331331
}
332332

333333
type position_and_mode = {
334-
(* apply_position of the current application *)
335334
apply_position : apply_position;
336-
(* [Some m] if [position] is [Tail], where m is the mode of the surrounding
337-
function's return mode *)
335+
(** Runtime tail call behaviour of the application *)
338336
region_mode : Regionality.t option;
337+
(** INVARIANT: [Some m] iff [apply_position] is [Tail], where [m] is the mode
338+
of the surrounding region *)
339339
}
340340

341341
let position_and_mode_default = {
342342
apply_position = Default;
343343
region_mode = None;
344344
}
345345

346-
(** The function produces two values, apply_position and region_mode.
347-
Invariant: if apply_position = Tail, then region_mode = Some ... *)
346+
(** Decides the runtime tail call behaviour based on lexical structures and user
347+
annotation. *)
348348
let position_and_mode env (expected_mode : expected_mode) sexp
349349
: position_and_mode =
350350
let fail err =
@@ -374,8 +374,9 @@ let position_and_mode env (expected_mode : expected_mode) sexp
374374
let check_tail_call_local_returning loc env ap_mode {region_mode; _} =
375375
match region_mode with
376376
| Some region_mode -> begin
377-
(* This application is at the tail of a function with a region;
378-
if ap_mode is local, funct_ret_mode needs to be local as well. *)
377+
(* This application will be performed after the current region is closed; if
378+
ap_mode is local, the application allocates in the outer
379+
region, and thus [region_mode] needs to be marked local as well*)
379380
match
380381
Regionality.submode (Regionality.of_locality ap_mode) region_mode
381382
with
@@ -444,6 +445,9 @@ let mode_region mode =
444445
let mode_max =
445446
mode_default Value.max_mode
446447

448+
let mode_with_position mode position =
449+
{ (mode_default mode) with position }
450+
447451
let mode_max_with_position position =
448452
{ mode_max with position }
449453

@@ -504,17 +508,16 @@ let mode_exact mode =
504508
{ (mode_default mode) with
505509
exact = true }
506510

507-
let mode_argument ~funct ~index ~position ~partial_app alloc_mode =
511+
let mode_argument ~funct ~index ~position_and_mode ~partial_app alloc_mode =
508512
let vmode = Value.of_alloc alloc_mode in
509513
if partial_app then mode_default vmode
510-
else match funct.exp_desc, index, (position : apply_position) with
514+
else match funct.exp_desc, index, position_and_mode.apply_position with
511515
| Texp_ident (_, _, {val_kind =
512516
Val_prim {Primitive.prim_name = ("%sequor"|"%sequand")}},
513517
Id_prim _, _), 1, Tail ->
514-
(* The second argument to (&&) and (||) is in
515-
tail position if the call is *)
516-
(* vmode is wrong; fine because of mode crossing on boolean *)
517-
mode_return vmode
518+
(* RHS of (&&) and (||) is at the tail of function region if the
519+
application is. The argument mode is not constrained otherwise. *)
520+
mode_with_position vmode (RTail (Option.get position_and_mode.region_mode, FTail))
518521
| Texp_ident (_, _, _, Id_prim _, _), _, _ ->
519522
(* Other primitives cannot be tail-called *)
520523
mode_default vmode
@@ -6822,12 +6825,12 @@ and type_argument ?explanation ?recarg env (mode : expected_mode) sarg
68226825
unify_exp env texp ty_expected;
68236826
texp
68246827

6825-
and type_apply_arg env ~app_loc ~funct ~index ~position ~partial_app (lbl, arg) =
6828+
and type_apply_arg env ~app_loc ~funct ~index ~position_and_mode ~partial_app (lbl, arg) =
68266829
match arg with
68276830
| Arg (Unknown_arg { sarg; ty_arg_mono; mode_arg; sort_arg }) ->
68286831
let mode, _ = Alloc.newvar_below mode_arg in
68296832
let expected_mode =
6830-
mode_argument ~funct ~index ~position ~partial_app mode in
6833+
mode_argument ~funct ~index ~position_and_mode ~partial_app mode in
68316834
let arg =
68326835
type_expect env expected_mode sarg (mk_expected ty_arg_mono)
68336836
in
@@ -6840,7 +6843,7 @@ and type_apply_arg env ~app_loc ~funct ~index ~position ~partial_app (lbl, arg)
68406843
mode_arg; wrapped_in_some; sort_arg }) ->
68416844
let mode, _ = Alloc.newvar_below mode_arg in
68426845
let expected_mode =
6843-
mode_argument ~funct ~index ~position ~partial_app mode in
6846+
mode_argument ~funct ~index ~position_and_mode ~partial_app mode in
68446847
let ty_arg', vars = tpoly_get_poly ty_arg in
68456848
let arg =
68466849
if vars = [] then begin
@@ -6899,7 +6902,7 @@ and type_apply_arg env ~app_loc ~funct ~index ~position ~partial_app (lbl, arg)
68996902
(lbl, Arg (arg, Value.legacy, sort_arg))
69006903
| Omitted _ as arg -> (lbl, arg)
69016904

6902-
and type_application env app_loc expected_mode pm
6905+
and type_application env app_loc expected_mode position_and_mode
69036906
funct funct_mode sargs ret_tvar =
69046907
let is_ignore funct =
69056908
is_prim ~name:"%ignore" funct &&
@@ -6924,12 +6927,12 @@ and type_application env app_loc expected_mode pm
69246927
submode ~loc:app_loc ~env ~reason:Other
69256928
mode_res expected_mode;
69266929
let arg_mode =
6927-
mode_argument ~funct ~index:0 ~position:(pm.apply_position)
6930+
mode_argument ~funct ~index:0 ~position_and_mode
69286931
~partial_app:false arg_mode
69296932
in
69306933
let exp = type_expect env arg_mode sarg (mk_expected ty_arg) in
69316934
check_partial_application ~statement:false exp;
6932-
([Nolabel, Arg (exp, arg_sort)], ty_ret, ap_mode, pm)
6935+
([Nolabel, Arg (exp, arg_sort)], ty_ret, ap_mode, position_and_mode)
69336936
| _ ->
69346937
let ty = funct.exp_type in
69356938
let ignore_labels =
@@ -6955,11 +6958,13 @@ and type_application env app_loc expected_mode pm
69556958
(Value.regional_to_local_alloc funct_mode) sargs ret_tvar
69566959
in
69576960
let partial_app = is_partial_apply untyped_args in
6958-
let pm = if partial_app then position_and_mode_default else pm in
6961+
let position_and_mode =
6962+
if partial_app then position_and_mode_default else position_and_mode
6963+
in
69596964
let args =
69606965
List.mapi (fun index arg ->
69616966
type_apply_arg env ~app_loc ~funct ~index
6962-
~position:(pm.apply_position) ~partial_app arg)
6967+
~position_and_mode ~partial_app arg)
69636968
untyped_args
69646969
in
69656970
let ty_ret, mode_ret, args =
@@ -6977,8 +6982,8 @@ and type_application env app_loc expected_mode pm
69776982
submode ~loc:app_loc ~env ~reason:(Application ty_ret)
69786983
mode_ret expected_mode;
69796984

6980-
check_tail_call_local_returning app_loc env ap_mode pm;
6981-
args, ty_ret, ap_mode, pm
6985+
check_tail_call_local_returning app_loc env ap_mode position_and_mode;
6986+
args, ty_ret, ap_mode, position_and_mode
69826987

69836988
and type_construct env (expected_mode : expected_mode) loc lid sarg
69846989
ty_expected_explained attrs =

0 commit comments

Comments
 (0)