Skip to content

Commit 0000cf2

Browse files
Scott Owensmeta-codesync[bot]
authored andcommitted
Try to predict when obj_get will fail and use constraints then
Summary: Try to detect when we are about to try to resolve a method on a receiver where the type is so under-constrained (e.g., a variable with no interesting bounds) that it will definitely fail. In that case, try constraint-based method inference to defer needing the type of the receiver. We don't do constraint-based always, because the error messages are currently degraded, but also because some features checks are not implemented yet. However, going to constraints when we are otherwise doomed to get a 4297 error, gives us some more completeness for inference. This is gated behind an experimental_tc_feature flag `try_constraint_method_inference`, so we can control the roll out. It also only works inside of expression trees, because there are a number of features that aren't implemented yet, but they aren't supported in expression trees - some attributes (__Memoized, __IgnoreReadonly, __AcceptDisposable) - noreturn - explicit type applications for generics There is also a config option `constraint_method_call` that enables constraints categorically, for testing purposes. Also make constraints work with inout and named parameters Reviewed By: andrewjkennedy Differential Revision: D84817455 fbshipit-source-id: 20a6846e9318fe6605306853cda5c15362f90161
1 parent 1128e6a commit 0000cf2

5 files changed

Lines changed: 97 additions & 26 deletions

File tree

hphp/hack/src/options/typecheckerOptions.ml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,9 @@ let experimental_consider_type_const_enforceable =
6262
(** Resolve the this type inside enum classes using the proper context *)
6363
let experimental_sound_enum_class_type_const = "sound_enum_class_this_type"
6464

65+
let experimental_try_constraint_method_inference =
66+
"experimental_try_constraint_method_inference"
67+
6568
let experimental_all =
6669
List.fold_right
6770
~f:SSet.add
@@ -74,6 +77,7 @@ let experimental_all =
7477
experimental_supportdynamic_type_hint;
7578
experimental_consider_type_const_enforceable;
7679
experimental_sound_enum_class_type_const;
80+
experimental_try_constraint_method_inference;
7781
]
7882

7983
let experimental_from_flags ~disallow_static_memoized =

hphp/hack/src/typing/typing.ml

Lines changed: 92 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -6254,6 +6254,58 @@ end = struct
62546254
| _ -> ((env, expr, ty), s))
62556255
(* Call instance method *)
62566256
| Obj_get (e1, (_, pos_id, Id m), nullflavor, Is_method) ->
6257+
(* We should use the constraint only if we're sure that immediately doing
6258+
obj_get will fail to get to concrete types. *)
6259+
let rec should_use_constraint seen env ty =
6260+
let (env, ty) = Env.expand_type env ty in
6261+
match get_node ty with
6262+
| Tdynamic
6263+
| Tprim _
6264+
| Ttuple _
6265+
| Tfun _
6266+
| Tshape _
6267+
| Toption _
6268+
| Tclass _
6269+
| Tvec_or_dict _
6270+
| Tclass_ptr _
6271+
| Tlabel _
6272+
| Tany _
6273+
| Tnonnull
6274+
| Tneg _
6275+
| Taccess _ ->
6276+
(* All of these can be handled by obj_get *)
6277+
(env, false)
6278+
| Tvar var ->
6279+
let bounds = Env.get_tyvar_lower_bounds env var in
6280+
should_use_constraint_for_inter
6281+
seen
6282+
env
6283+
(List.filter_map (Internal_type_set.to_list bounds) ~f:(fun ity ->
6284+
match ity with
6285+
| LoclType ty -> Some ty
6286+
| _ -> None))
6287+
| Tunion ts ->
6288+
(* If one of the branches of the union won't resolve,
6289+
then obj_get will end up with a 4297 from trying to force solve *)
6290+
List.exists_env env ~f:(should_use_constraint seen) ts
6291+
| Tintersection ts -> should_use_constraint_for_inter seen env ts
6292+
| Tdependent (_, ty) -> should_use_constraint seen env ty
6293+
| Tgeneric name
6294+
| Tnewtype (name, _, _) ->
6295+
if not (SSet.mem name seen) then
6296+
let (env, ts) =
6297+
TUtils.get_concrete_supertypes ~abstract_enum:true env ty
6298+
in
6299+
should_use_constraint_for_inter (SSet.add name seen) env ts
6300+
else
6301+
(env, false)
6302+
and should_use_constraint_for_inter seen env ts =
6303+
match ts with
6304+
| [] -> (env, true)
6305+
| [ty] -> should_use_constraint seen env ty
6306+
| _ -> (env, false)
6307+
(* The constraint doesn't work well with intersections, so don't bother *)
6308+
in
62576309
let (env, te1, ty1) =
62586310
expr
62596311
~expected:None
@@ -6266,11 +6318,31 @@ end = struct
62666318
| Regular -> None
62676319
| Nullsafe -> Some p
62686320
in
6269-
let use_constraint_inference =
6321+
(* These are the only kinds of calls that we support with constraint inference *)
6322+
let maybe_use_constraint_inference =
62706323
List.is_empty explicit_targs
62716324
&& Option.is_none unpacked_element
62726325
&& Option.is_none nullsafe
6273-
&& TCO.constraint_method_call env.genv.tcopt
6326+
in
6327+
let (env, use_constraint_inference) =
6328+
if
6329+
maybe_use_constraint_inference
6330+
&& TCO.constraint_method_call env.genv.tcopt
6331+
then
6332+
(* Config tells us to use it for all supported functions *)
6333+
(env, true)
6334+
else if
6335+
maybe_use_constraint_inference
6336+
&& Option.is_some env.in_expr_tree
6337+
&& TypecheckerOptions.experimental_feature_enabled
6338+
(Env.get_tcopt env)
6339+
TypecheckerOptions.experimental_try_constraint_method_inference
6340+
then
6341+
(* Inside of an expression tree, we can use constraint inference if the appropriate experimental_tc_feature is enabled.
6342+
Many of the features that constraint inference doesn't support are not allowed in expression trees: inout, disposable, etc. *)
6343+
should_use_constraint SSet.empty env ty1
6344+
else
6345+
(env, false)
62746346
in
62756347
if use_constraint_inference then (
62766348
(* Construct a function type from the types of the arguments, and check
@@ -6289,21 +6361,26 @@ end = struct
62896361
in
62906362
let arg_posl =
62916363
List.map el ~f:(fun arg ->
6292-
match arg with
6293-
| Aast_defs.Ainout (_, (_, pos, e))
6294-
| Aast_defs.Anamed (_, (_, pos, e))
6295-
| Aast_defs.Anormal (_, pos, e) ->
6296-
(match e with
6297-
| ReadonlyExpr _ -> (pos, true)
6298-
| _ -> (pos, false)))
6299-
in
6300-
let make_param (pos, is_readonly) ty =
6364+
let (e, pos, name, is_inout) =
6365+
match arg with
6366+
| Aast_defs.Ainout (_, (_, pos, e)) -> (e, pos, None, true)
6367+
| Aast_defs.Anamed (n, (_, pos, e)) -> (e, pos, Some n, false)
6368+
| Aast_defs.Anormal (_, pos, e) -> (e, pos, None, false)
6369+
in
6370+
match e with
6371+
| ReadonlyExpr _ -> (pos, name, true, is_inout)
6372+
| _ -> (pos, name, false, is_inout))
6373+
in
6374+
let make_param (pos, name, is_readonly, is_inout) ty =
63016375
{
63026376
fp_pos = Pos_or_decl.of_raw_pos pos;
6303-
fp_name = None;
6377+
fp_name = Option.map ~f:snd name;
63046378
fp_type = ty;
63056379
fp_flags =
6306-
Typing_defs_flags.FunParam.(set_readonly is_readonly default);
6380+
Typing_defs_flags.FunParam.(
6381+
set_named
6382+
(Option.is_some name)
6383+
(set_inout is_inout (set_readonly is_readonly default)));
63076384
fp_def_value = None;
63086385
}
63096386
in
@@ -6346,7 +6423,8 @@ end = struct
63466423
(Some
63476424
{
63486425
hmm_explicit_targs = [];
6349-
hmm_args_pos = List.map ~f:fst arg_posl;
6426+
hmm_args_pos =
6427+
List.map ~f:(fun (pos, _, _, _) -> pos) arg_posl;
63506428
})
63516429
in
63526430
let (env, ty_err_opt) =
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,3 @@
11
--extra-builtin ../../../../expr_tree.php
22
--config enable_experimental_stx_features='{"await_in_splice":"OngoingRelease","expression_trees": "Unstable", "expression_tree_nested_bindings": "Unstable", "case_types": "Preview", "case_type_where_clauses": "Preview", "like_type_hints": "Unstable"}'
3+
--config enable_experimental_tc_features=experimental_try_constraint_method_inference

hphp/hack/test/typecheck/expression_trees/nesting/vars/simple_error.php.exp

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,3 @@ Typing error (Typing[4110])
44
Expected `int`
55
File "expr_tree.php", line 153, characters 40-45:
66
But got `string`
7-
ERROR: File "simple_error.php", line 7, characters 43-48:
8-
Was expecting an object but type is unknown (Typing[4297])
9-
File "simple_error.php", line 7, characters 43-48:
10-
It is unknown because type parameter `T` of `operationType` could not be determined. Please add explicit type parameters to the invocation of `operationType`
11-
File "expr_tree.php", line 176, characters 8-8:
12-
via this generic `T`

hphp/hack/test/typecheck/expression_trees/nesting/vars/simple_error.php.imp_pess_exp

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,3 @@ Typing error (Typing[4110])
44
Expected `int`
55
File "expr_tree.php", line 153, characters 40-45:
66
But got `string`
7-
ERROR: File "simple_error.php", line 7, characters 30-50:
8-
Typing error (Typing[4110])
9-
File "expr_tree.php", line 175, characters 5-21:
10-
Expected `ExprTreeOpType<nothing>`
11-
File "expr_tree.php", line 144, characters 39-41:
12-
But got `int`

0 commit comments

Comments
 (0)