From a9dd4a70b482a91350439fc0b68f63b27b075830 Mon Sep 17 00:00:00 2001 From: Dwayne Reeves Date: Sat, 30 Jul 2016 10:32:53 -0700 Subject: [PATCH] Properly localize require ancestors in Typing_subtype Summary: We did not properly substitute type parameters when upcasing to a required ancestor. This diff fixes this by making sure we create the `ety_env` the same for both branches of our upcasting algorithm. In order to avoid unnecessary error reporting I hade to make the creation of `ety_env` lazy. The added test case shows how it arises. ``` abstract class Expression {} abstract class Evaluator> {} class LiteralEvaluator> extends Evaluator {} interface ILiteralExpression { require extends Expression; } ``` Here when we are checking if `ILiteralExpression <: Expression` we utilize the require extends to upcast `ILiteralExpression` to `Expression`. Before the diff we would not instantiate the type parameter correctly so we would end up checking `Expression <: Expression` which would fail because `TlitResult` and `T` are different generics. Note taht if `TlitResult` was named `T` then it would pass because now the type parameters have the same name, even though the type parameter was not properly instantiated. {D2613160} revealed this particular issue because now with scoping we can tell that `ILiteralExpression\T` is different than `LiteralEvaluator\T`. Reviewed By: andrewjkennedy Differential Revision: D3561665 fbshipit-source-id: 907ecaaabb9eff63a4e8e4d38bc01137ac023f4c --- hphp/hack/src/typing/typing_subtype.ml | 58 ++++++++++------------ .../test/typecheck/req_ancestor_with_generics.php | 12 +++++ .../typecheck/req_ancestor_with_generics.php.exp | 1 + 3 files changed, 40 insertions(+), 31 deletions(-) create mode 100644 hphp/hack/test/typecheck/req_ancestor_with_generics.php create mode 100644 hphp/hack/test/typecheck/req_ancestor_with_generics.php.exp diff --git a/hphp/hack/src/typing/typing_subtype.ml b/hphp/hack/src/typing/typing_subtype.ml index db0223b48bc..36d120e5e17 100644 --- a/hphp/hack/src/typing/typing_subtype.ml +++ b/hphp/hack/src/typing/typing_subtype.ml @@ -309,6 +309,31 @@ and sub_type_with_uenv env (uenv_sub, ty_sub) (uenv_super, ty_super) = (match class_ with | None -> env | Some class_ -> + let ety_env = + (* We handle the case where a generic A is used as A *) + let tyl_sub = + if tyl_sub = [] && not (Env.is_strict env) + then List.map class_.tc_tparams (fun _ -> (p_sub, Tany)) + else tyl_sub + in + if List.length class_.tc_tparams <> List.length tyl_sub + then + Errors.expected_tparam + (Reason.to_pos p_sub) (List.length class_.tc_tparams); + (* NOTE: We rely on the fact that we fold all ancestors of + * ty_sub in its class_type so we will never hit this case + * again. If this ever changes then we would need to store + * ty_sub as the 'this_ty' in the uenv and be careful to + * thread it through. + * + * This is covered by test/typecheck/this_tparam2.php + *) + { + type_expansions = []; + substs = Subst.make class_.tc_tparams tyl_sub; + this_ty = ExprDepTy.apply uenv_sub.TUEnv.dep_tys ty_sub; + from_class = None; + } in let subtype_req_ancestor = if class_.tc_kind = Ast.Ctrait || class_.tc_kind = Ast.Cinterface then (* a trait is never the runtime type, but it can be used @@ -319,12 +344,6 @@ and sub_type_with_uenv env (uenv_sub, ty_sub) (uenv_super, ty_super) = | _, Some _ -> acc | env, None -> Errors.try_ begin fun () -> - let ety_env = { - type_expansions = []; - substs = SMap.empty; - this_ty = ExprDepTy.apply uenv_sub.TUEnv.dep_tys ty_sub; - from_class = None; - } in let env, req_type = Phase.localize ~ety_env env req_type in let _, req_ty = req_type in @@ -339,31 +358,8 @@ and sub_type_with_uenv env (uenv_sub, ty_sub) (uenv_super, ty_super) = let up_obj = SMap.get cid_super class_.tc_ancestors in match up_obj with | Some up_obj -> - (* We handle the case where a generic A is used as A *) - let tyl_sub = - if tyl_sub = [] && not (Env.is_strict env) - then List.map class_.tc_tparams (fun _ -> (p_sub, Tany)) - else tyl_sub - in - if List.length class_.tc_tparams <> List.length tyl_sub - then - Errors.expected_tparam - (Reason.to_pos p_sub) (List.length class_.tc_tparams); - (* NOTE: We rely on the fact that we fold all ancestors of - * ty_sub in its class_type so we will never hit this case - * again. If this ever changes then we would need to store - * ty_sub as the 'this_ty' in the uenv and be careful to - * thread it through. - * - * This is covered by test/typecheck/this_tparam2.php - *) - let ety_env = { - type_expansions = []; - substs = Subst.make class_.tc_tparams tyl_sub; - this_ty = ExprDepTy.apply uenv_sub.TUEnv.dep_tys ty_sub; - from_class = None; - } in - let env, up_obj = Phase.localize ~ety_env env up_obj in + let env, up_obj = + Phase.localize ~ety_env env up_obj in sub_type env up_obj ty_super | None when class_.tc_members_fully_known -> TUtils.uerror p_super ty_super_ p_sub ty_sub_; diff --git a/hphp/hack/test/typecheck/req_ancestor_with_generics.php b/hphp/hack/test/typecheck/req_ancestor_with_generics.php new file mode 100644 index 00000000000..afae4a1a173 --- /dev/null +++ b/hphp/hack/test/typecheck/req_ancestor_with_generics.php @@ -0,0 +1,12 @@ + {} +abstract class Evaluator> {} + +class LiteralEvaluator> + extends Evaluator {} + +interface ILiteralExpression { + require extends Expression; +} diff --git a/hphp/hack/test/typecheck/req_ancestor_with_generics.php.exp b/hphp/hack/test/typecheck/req_ancestor_with_generics.php.exp new file mode 100644 index 00000000000..4269126fceb --- /dev/null +++ b/hphp/hack/test/typecheck/req_ancestor_with_generics.php.exp @@ -0,0 +1 @@ +No errors -- 2.11.4.GIT