Skip to content

Commit db52349

Browse files
Michael Thomasmeta-codesync[bot]
authored andcommitted
Remove type parameter introduced by 'expression dependent' during exposure
Summary: Typing escape implements exposure - inside a lambda, any type parameter which is introduced should not be able to escape it's scope. Type parameters appearing only co- orcontravariantly can be replaced with their lower / upper bounds and errors are raised for those which appear invariantly. However, the code deliberately avoids dealing with 'bogus' type parameters which are introduced as part of the implementation of type constant access. This gap has an unfortunate interaction with the more performant version of the common 'fluent builder' pattern: ``` interface IRepBuilderMethods<+TChainableBuilder as IRepBuilder> { public function setField<TValue>( HH\EnumClass\Label<RepAccessors, IRepField< RepFieldSettings with { type TValue super TValue }, >> $label, TValue $value, ): TChainableBuilder; } interface IRepBuilder extends IRepBuilderMethods<this::TChainableBuilder> { abstract const type TChainableBuilder as IRepBuilder with { type TChainableBuilder = this::TChainableBuilder }; } ``` Suppose we have a series of lambda each of which take a parameter of type `IRepBuilder` and call `setField`. On exit from the lambda we still have the 'expression dependent' type parameter (`<expr#N>::...`) and the 'bogus' type parameter (`IRepBuilder::TChainableBuilder`) builder in the global `tpenv`: ``` <expr#1>::TCB upper: {\IRB::TCB} \IRB::TCB lower: {<expr#1>::TCB} upper: {IRB with { type TCB = <expr#1>::TCB } ``` In the next lambda, we get a fresh 'expression dependent' type parameter which will also appear in the bounds of the 'bogus' type paramter `IRepBuilder::TChainableBuilder': ``` \IRB::TCB lower: {<expr#1>::TCB, <expr#2>::TCB} upper: {IRB with { type TCB = <expr#1>::TCB }, IRB with { type TCB = <expr#2>::TCB }} ``` Now, for each method call on an receiver expression with type `<expr#2>::TCB`, or *any* subsequent expression dependent type which shares it's upperbound, we are actually resolving the method on the intersection of the upper bounds so we have linear growth in the number of lambda. This is worsened when we have multiple method calls since each is paying the cost of access a method through an intersection. Altogether we end up with a cubic cost. This didn't happen for the non-recursive case since each call to `setField` gives us back a fresh type (the ever expanding access path). This diff addresses the issue by removing type parameters introduced for expression dependent types inside lambdas during the exposure step and also removing them from the bounds of the 'bogus' type parameters. Reviewed By: andrewjkennedy Differential Revision: D95847716 fbshipit-source-id: 2974c2ccb795e6af162a8df0c04a3456da6440a7
1 parent 3936b8b commit db52349

5 files changed

Lines changed: 178 additions & 1 deletion

File tree

hphp/hack/src/typing/typing_defs.ml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -514,6 +514,8 @@ module DependentKind = struct
514514
Some (Str.matched_group 1 str)
515515
with
516516
| _ -> None
517+
518+
let is_expr_dep_ty str = Option.is_some (strip_generic_dep_ty str)
517519
end
518520

519521
let rec is_denotable ty =

hphp/hack/src/typing/typing_defs.mli

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -329,6 +329,8 @@ module DependentKind : sig
329329
val is_generic_dep_ty : string -> bool
330330

331331
val strip_generic_dep_ty : string -> string option
332+
333+
val is_expr_dep_ty : string -> bool
332334
end
333335

334336
module ShapeFieldMap : sig

hphp/hack/src/typing/typing_escape.ml

Lines changed: 102 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -601,7 +601,108 @@ let refresh_env_and_type ~remove:(types, remove) ~pos env ty =
601601
in
602602
let renv = { renv with on_error } in
603603
let (renv, ty, _) = refresh_type renv Ast_defs.Covariant ty in
604-
(refresh_tvars Tvid.Set.empty renv, ty)
604+
let env = refresh_tvars Tvid.Set.empty renv in
605+
606+
(* Remove escaping expression-dependent types from the global tpenv.
607+
Without this cleanup, expression-dependent types introduced inside
608+
one lambda leak into subsequent lambdas via the global tpenv.
609+
typing_taccess expands type constants on a Tgeneric by iterating
610+
over all its upper bounds and intersecting the results. With the
611+
recursive `with` refinement pattern (e.g.,
612+
abstract const type TCB as IRepBuilder
613+
with { type TCB = this::TCB };
614+
), each lambda adds a distinct `with` upper bound to the mangled
615+
generic, so typing_taccess produces a growing intersection,
616+
causing exponential blowup.
617+
618+
After the refresh steps above have replaced escaping types in
619+
locals, the return type, and unsolved tyvar bounds, we:
620+
1. Build a substitution mapping each deleted expr-dep generic
621+
to its upper bound (read before deletion)
622+
2. Delete the expr-dep entries from the tpenv
623+
3. Apply the substitution to remaining tpenv entries' bounds
624+
(cleaning stale `with` refinements that reference deleted types)
625+
4. Apply the substitution to solved tyvar solutions (which the
626+
refresh steps expanded inline but did not write back) *)
627+
let pre_deletion_tpenv = Env.get_global_tpenv env in
628+
629+
(* Build substitution map: deleted name -> upper bound *)
630+
let subst =
631+
List.fold_left types ~init:SMap.empty ~f:(fun acc name ->
632+
if Typing_defs.DependentKind.is_expr_dep_ty name then
633+
match
634+
TySet.elements
635+
(Type_parameter_env.get_upper_bounds pre_deletion_tpenv name)
636+
with
637+
| [ub] -> SMap.add name ub acc
638+
| _ -> acc
639+
else
640+
acc)
641+
in
642+
let subst_ty ty =
643+
Typing_defs_core.Locl_subst.apply
644+
ty
645+
~subst
646+
~combine_reasons:(fun ~src ~dest:_ -> src)
647+
in
648+
649+
(* Delete expr-dep entries from the tpenv *)
650+
let global_tpenv =
651+
List.fold_left types ~init:pre_deletion_tpenv ~f:(fun tpenv name ->
652+
if
653+
Typing_defs.DependentKind.is_expr_dep_ty name
654+
&& Type_parameter_env.mem name tpenv
655+
then
656+
Type_parameter_env.remove tpenv name
657+
else
658+
tpenv)
659+
in
660+
661+
(* Substitute in remaining entries bounds *)
662+
let global_tpenv =
663+
if SMap.is_empty subst then
664+
global_tpenv
665+
else
666+
List.fold_left
667+
(Type_parameter_env.get_tparam_names global_tpenv)
668+
~init:global_tpenv
669+
~f:(fun tpenv name ->
670+
match Type_parameter_env.get_with_pos name tpenv with
671+
| None -> tpenv
672+
| Some (def_pos, tparam_info) ->
673+
let ubs = TySet.map subst_ty tparam_info.upper_bounds in
674+
let lbs = TySet.map subst_ty tparam_info.lower_bounds in
675+
if
676+
TySet.equal ubs tparam_info.upper_bounds
677+
&& TySet.equal lbs tparam_info.lower_bounds
678+
then
679+
tpenv
680+
else
681+
Type_parameter_env.add
682+
~def_pos
683+
name
684+
{ tparam_info with upper_bounds = ubs; lower_bounds = lbs }
685+
tpenv)
686+
in
687+
let env = Env.env_with_global_tpenv env global_tpenv in
688+
689+
(* Substitute in solved tyvar solutions *)
690+
let env =
691+
if SMap.is_empty subst then
692+
env
693+
else
694+
List.fold_left (Env.get_all_tyvars env) ~init:env ~f:(fun env tv ->
695+
if Env.tyvar_is_solved env tv then
696+
let (env, solution) = Env.get_type env Typing_reason.none tv in
697+
let solution' = subst_ty solution in
698+
if phys_equal solution solution' then
699+
env
700+
else
701+
Env.add env tv solution'
702+
else
703+
env)
704+
in
705+
(env, ty)
605706
)
606707

607708
(********************************************************************)
Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
<?hh
2+
3+
abstract class RepFieldSettings {
4+
abstract const type TValue;
5+
}
6+
7+
interface IRepField<+TSettings as RepFieldSettings> {}
8+
9+
class RepIntSettings extends RepFieldSettings {
10+
const type TValue = int;
11+
}
12+
13+
class RepIntField implements IRepField<RepIntSettings> {}
14+
15+
enum class RepAccessors: IRepField<RepFieldSettings> {
16+
RepIntField A = new RepIntField();
17+
RepIntField B = new RepIntField();
18+
RepIntField C = new RepIntField();
19+
RepIntField D = new RepIntField();
20+
RepIntField E = new RepIntField();
21+
}
22+
23+
interface IRepBuilderMethods<+TChainableBuilder as IRepBuilder> {
24+
public function setField<TValue>(
25+
HH\EnumClass\Label<RepAccessors, IRepField<
26+
RepFieldSettings with { type TValue super TValue },
27+
>> $label,
28+
TValue $value,
29+
): TChainableBuilder;
30+
}
31+
32+
interface IRepBuilder extends IRepBuilderMethods<this::TChainableBuilder> {
33+
abstract const type TChainableBuilder as
34+
IRepBuilder with { type TChainableBuilder = this::TChainableBuilder };
35+
}
36+
37+
function tpenv_leak_repro(): dict<string, (function(IRepBuilder): void)> {
38+
$x = dict[
39+
'a' => (IRepBuilder $b) ==> {
40+
$b->setField(#A, 1)->setField(#B, 2)->setField(#C, 3)->setField(#D, 4)->setField(#E, 5);
41+
},
42+
'b' => (IRepBuilder $b) ==> {
43+
$b->setField(#A, 1)->setField(#B, 2)->setField(#C, 3)->setField(#D, 4)->setField(#E, 5);
44+
},
45+
'c' => (IRepBuilder $b) ==> {
46+
$b->setField(#A, 1)->setField(#B, 2)->setField(#C, 3)->setField(#D, 4)->setField(#E, 5);
47+
},
48+
'd' => (IRepBuilder $b) ==> {
49+
$b->setField(#A, 1)->setField(#B, 2)->setField(#C, 3)->setField(#D, 4)->setField(#E, 5);
50+
},
51+
'e' => (IRepBuilder $b) ==> {
52+
$b->setField(#A, 1)->setField(#B, 2)->setField(#C, 3)->setField(#D, 4)->setField(#E, 5);
53+
},
54+
'f' => (IRepBuilder $b) ==> {
55+
$b->setField(#A, 1)->setField(#B, 2)->setField(#C, 3)->setField(#D, 4)->setField(#E, 5);
56+
},
57+
'g' => (IRepBuilder $b) ==> {
58+
$b->setField(#A, 1)->setField(#B, 2)->setField(#C, 3)->setField(#D, 4)->setField(#E, 5);
59+
},
60+
'h' => (IRepBuilder $b) ==> {
61+
$b->setField(#A, 1)->setField(#B, 2)->setField(#C, 3)->setField(#D, 4)->setField(#E, 5);
62+
},
63+
'i' => (IRepBuilder $b) ==> {
64+
$b->setField(#A, 1)->setField(#B, 2)->setField(#C, 3)->setField(#D, 4)->setField(#E, 5);
65+
},
66+
'j' => (IRepBuilder $b) ==> {
67+
$b->setField(#A, 1)->setField(#B, 2)->setField(#C, 3)->setField(#D, 4)->setField(#E, 5);
68+
},
69+
];
70+
return $x;
71+
}
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
No errors

0 commit comments

Comments
 (0)