Skip to content

Commit 7e5b31d

Browse files
dlreevesmeta-codesync[bot]
authored andcommitted
Fix infinite loop when type checking recursive case type bounds
Summary: # Context When a case type has an upper bound that references another case type which recursively refers back (e.g. `case type A as int, B = string; case type B = vec<A>;`), `hh_single_type_check` would loop forever. The cycle was: during localization of a `Tintersection` (the upper bound constraint), `Typing_intersection.intersect_list` is called to simplify the intersection. This depends on `typing_subtype`, which depends back on `typing_phase` to localize types — creating an infinite loop when the types are mutually recursive. # Solution Add a `simplify_intersections` flag to `expand_env` (defaulting to `true`). When localizing the upper bound constraint of a typedef in `typing_tdef.ml`, set this flag to `false` so that `Tintersection` types are constructed directly via `MakeType.intersection` without calling `Typing_intersection.intersect_list`. This breaks the cycle while keeping intersection simplification active for all other localization paths. Reviewed By: viratyosin Differential Revision: D96009559 fbshipit-source-id: 2fec43205be666b1c50a050833fcc55d010b522d
1 parent be28efc commit 7e5b31d

9 files changed

Lines changed: 109 additions & 14 deletions

hphp/hack/src/typing/env/typing_env.ml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2109,6 +2109,7 @@ module M = struct
21092109
wildcard_action = _;
21102110
ish_weakening;
21112111
under_type_constructor = _;
2112+
simplify_intersections = _;
21122113
} =
21132114
if Typing_env_types.get_log_level env "expand_env" |> Int.( = ) 0 then
21142115
[]

hphp/hack/src/typing/typing_defs.ml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -313,6 +313,7 @@ type expand_env = {
313313
wildcard_action: wildcard_action;
314314
ish_weakening: bool;
315315
under_type_constructor: bool;
316+
simplify_intersections: bool;
316317
}
317318

318319
let empty_expand_env =
@@ -327,6 +328,7 @@ let empty_expand_env =
327328
wildcard_action = Wildcard_fresh_tyvar;
328329
ish_weakening = false;
329330
under_type_constructor = false;
331+
simplify_intersections = true;
330332
}
331333

332334
let empty_expand_env_with_on_error on_error =

hphp/hack/src/typing/typing_defs.mli

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -245,6 +245,12 @@ type expand_env = {
245245
so on cycle we preserve the Tnewtype identity instead of collapsing
246246
to mixed — this prevents unsound subtyping through the constructor's
247247
type parameters. *)
248+
simplify_intersections: bool;
249+
(** When false, skip calling Typing_intersection.intersect_list during
250+
localization of Tintersection types, and instead construct the
251+
intersection directly. This breaks the cycle
252+
localize -> intersect_list -> sub_type -> localize
253+
that can occur when localizing recursive case type upper bounds. *)
248254
}
249255

250256
val empty_expand_env : expand_env

hphp/hack/src/typing/typing_phase.ml

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -580,7 +580,12 @@ and localize_ ~(ety_env : expand_env) env (dty : decl_ty) :
580580
~f:(localize ~ety_env)
581581
~combine_ty_errs:Typing_error.multiple_opt
582582
in
583-
let (env, ty) = Typing_intersection.intersect_list env r tyl in
583+
let (env, ty) =
584+
if ety_env.simplify_intersections then
585+
Typing_intersection.intersect_list env r tyl
586+
else
587+
(env, MakeType.intersection r tyl)
588+
in
584589
((env, ty_err_opt, cycles), ty)
585590
| Taccess (root_ty, id) ->
586591
let origin_opt = find_origin dty in

hphp/hack/src/typing/typing_tdef.ml

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,15 @@ let expand_typedef_ ~force_expand ety_env env r (x : string) argl :
159159
if String.equal x SN.Classes.cSupportDyn then
160160
((env, None, []), List.hd_exn argl)
161161
else
162-
Phase.localize_rec ~ety_env:ety_env_inner env cstr
162+
(* Disable intersection simplification when localizing the upper
163+
bound constraint. Simplifying intersections during localization
164+
calls Typing_intersection.intersect_list, which depends on
165+
typing_subtype, which depends on typing_phase — creating a cycle
166+
that can loop forever on recursive case type bounds. *)
167+
let ety_env_cstr =
168+
{ ety_env_inner with simplify_intersections = false }
169+
in
170+
Phase.localize_rec ~ety_env:ety_env_cstr env cstr
163171
in
164172
let ((env, err, cycles), expanded_ty, bound) =
165173
match expansion with

hphp/hack/test/typecheck/case_type/case_type_basics.php.exp

Lines changed: 30 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,43 @@
11
ERROR: File "case_type_basics.php", line 16, characters 14-21:
2+
Invalid argument (Typing[4110])
3+
File "case_type_basics.php", line 4, characters 25-32:
4+
Expected `(int | not arraykey)`
5+
File "case_type_basics.php", line 15, characters 18-27:
6+
But got `<expr#2> as XHPChild`
7+
File "case_type_basics.php", line 16, characters 14-21:
8+
where `<expr#2>` is a reference to this expression
9+
ERROR: File "case_type_basics.php", line 16, characters 14-21:
10+
Invalid argument (Typing[4110])
11+
File "case_type_basics.php", line 4, characters 25-32:
12+
Expected `nothing`
13+
File "case_type_basics.php", line 4, characters 35-42:
14+
But got `XHPChild`
15+
ERROR: File "case_type_basics.php", line 16, characters 14-21:
216
Invalid argument (Typing[4110])
317
File "case_type_basics.php", line 8, characters 21-23:
418
Expected `int`
19+
File "case_type_basics.php", line 4, characters 35-42:
20+
But got `XHPChild`
21+
ERROR: File "case_type_basics.php", line 21, characters 20-27:
22+
Invalid argument (Typing[4110])
23+
File "case_type_basics.php", line 4, characters 25-32:
24+
Expected `(CT_No_Bounds | not arraykey)`
525
File "case_type_basics.php", line 15, characters 18-27:
6-
But got `arraykey`
26+
But got `<expr#2> as XHPChild`
27+
File "case_type_basics.php", line 21, characters 20-27:
28+
where `<expr#2>` is a reference to this expression
29+
ERROR: File "case_type_basics.php", line 21, characters 20-27:
30+
Invalid argument (Typing[4110])
731
File "case_type_basics.php", line 4, characters 25-32:
8-
by the definition of `CT_Bounded`
32+
Expected `nothing`
33+
File "case_type_basics.php", line 4, characters 35-42:
34+
But got `XHPChild`
935
ERROR: File "case_type_basics.php", line 21, characters 20-27:
1036
Invalid argument (Typing[4110])
1137
File "case_type_basics.php", line 13, characters 27-38:
1238
Expected `CT_No_Bounds`
13-
File "case_type_basics.php", line 15, characters 18-27:
14-
But got `arraykey`
15-
File "case_type_basics.php", line 4, characters 25-32:
16-
by the definition of `CT_Bounded`
39+
File "case_type_basics.php", line 4, characters 35-42:
40+
But got `XHPChild`
1741
ERROR: File "case_type_basics.php", line 23, characters 14-23:
1842
Invalid argument (Typing[4110])
1943
File "case_type_basics.php", line 8, characters 21-23:

hphp/hack/test/typecheck/case_type/case_type_basics.php.imp_pess_exp

Lines changed: 30 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,43 @@
11
ERROR: File "case_type_basics.php", line 16, characters 14-21:
2+
Invalid argument (Typing[4110])
3+
File "case_type_basics.php", line 4, characters 25-32:
4+
Expected `(int | not arraykey)`
5+
File "case_type_basics.php", line 15, characters 18-27:
6+
But got `<expr#2> as XHPChild`
7+
File "case_type_basics.php", line 16, characters 14-21:
8+
where `<expr#2>` is a reference to this expression
9+
ERROR: File "case_type_basics.php", line 16, characters 14-21:
10+
Invalid argument (Typing[4110])
11+
File "case_type_basics.php", line 4, characters 25-32:
12+
Expected `nothing`
13+
File "case_type_basics.php", line 4, characters 35-42:
14+
But got `XHPChild`
15+
ERROR: File "case_type_basics.php", line 16, characters 14-21:
216
Invalid argument (Typing[4110])
317
File "case_type_basics.php", line 8, characters 21-23:
418
Expected `int`
19+
File "case_type_basics.php", line 4, characters 35-42:
20+
But got `XHPChild`
21+
ERROR: File "case_type_basics.php", line 21, characters 20-27:
22+
Invalid argument (Typing[4110])
23+
File "case_type_basics.php", line 4, characters 25-32:
24+
Expected `(CT_No_Bounds | not arraykey)`
525
File "case_type_basics.php", line 15, characters 18-27:
6-
But got `arraykey`
26+
But got `<expr#2> as XHPChild`
27+
File "case_type_basics.php", line 21, characters 20-27:
28+
where `<expr#2>` is a reference to this expression
29+
ERROR: File "case_type_basics.php", line 21, characters 20-27:
30+
Invalid argument (Typing[4110])
731
File "case_type_basics.php", line 4, characters 25-32:
8-
by the definition of `CT_Bounded`
32+
Expected `nothing`
33+
File "case_type_basics.php", line 4, characters 35-42:
34+
But got `XHPChild`
935
ERROR: File "case_type_basics.php", line 21, characters 20-27:
1036
Invalid argument (Typing[4110])
1137
File "case_type_basics.php", line 13, characters 27-38:
1238
Expected `CT_No_Bounds`
13-
File "case_type_basics.php", line 15, characters 18-27:
14-
But got `arraykey`
15-
File "case_type_basics.php", line 4, characters 25-32:
16-
by the definition of `CT_Bounded`
39+
File "case_type_basics.php", line 4, characters 35-42:
40+
But got `XHPChild`
1741
ERROR: File "case_type_basics.php", line 23, characters 14-23:
1842
Invalid argument (Typing[4110])
1943
File "case_type_basics.php", line 8, characters 21-23:
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
<?hh
2+
<<file:__EnableUnstableFeatures('case_types')>>
3+
4+
// Recursive case type bounds that previously caused an infinite loop during
5+
// type checking. The cycle was: localize -> intersect_list -> sub_type -> localize
6+
// when simplifying the intersection in the upper bound constraint.
7+
case type MyEtDemoAutoLiftable as int, MyEtDemoOpType = string;
8+
9+
case type MyEtDemoOpType = vec<MyEtDemoAutoLiftable>;
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
ERROR: File "case_type_recursive_bound_loop.php", line 7, characters 11-30:
2+
Invalid constraint on `newtype` (Typing[4332])
3+
File "case_type_recursive_bound_loop.php", line 7, characters 35-37:
4+
Expected `int`
5+
File "case_type_recursive_bound_loop.php", line 7, characters 57-62:
6+
But got `string`
7+
ERROR: File "case_type_recursive_bound_loop.php", line 7, characters 11-30:
8+
Invalid constraint on `newtype` (Typing[4332])
9+
File "case_type_recursive_bound_loop.php", line 7, characters 40-53:
10+
Expected `MyEtDemoOpType`
11+
File "case_type_recursive_bound_loop.php", line 7, characters 57-62:
12+
But got `string`
13+
ERROR: File "case_type_recursive_bound_loop.php", line 9, characters 11-24:
14+
Cyclic type definition (Typing[4498])
15+
File "case_type_recursive_bound_loop.php", line 7, characters 40-53:
16+
Cyclic use is here

0 commit comments

Comments
 (0)