Skip to content
Permalink
Browse files

New inference: overloaded idx

Summary: There is a subtle issue with the type-checking of `idx`. After constructing the signature for `idx`, according to the number of parameters, and checking the call, we try to eliminate double nullables using a call to `non_null`. This shouldn't be necessary (subtyping etc should just deal with it) but in the absence of this code we trigger various weirdnesses related to growing of unresolveds etc. (See new test `idx_bug.php` for an example.) Under `--new-inference` we don't trigger these problems (it seems to deal with double nullables better) but instead there is an interaction with variance of type variables, caused by the eager solving of type variables in `non_null` (see test `idx_test.php`). So for now, we avoid calling `non_null` only when `--new-inference` is set. Once we move to the new algorithm, we should just remove the code completely.

Reviewed By: manzyuk

Differential Revision: D13720874

fbshipit-source-id: 7d99174516295f23fa5b915034f931c84f5084ab
  • Loading branch information...
andrewjkennedy authored and hhvm-bot committed Jan 19, 2019
1 parent eec35ce commit 4029873865d2da8d3f810aed4283852ca0c14199
@@ -3785,8 +3785,11 @@ and is_abstract_ft fty = match fty with
let env, fty, tyvars = Phase.localize_ft ~use_pos:p ~ety_env env fty in
let tfun = Reason.Rwitness fty.ft_pos, Tfun fty in
let env, tel, _tuel, ty = call ~expected p env tfun el [] in
(* Remove double nullables. This shouldn't be necessary, and currently
* interferes with new_inference because it "solves" before we set variance
*)
let env, ty = match ty with
| r, Toption ty ->
| r, Toption ty when not (TypecheckerOptions.new_inference (Env.get_tcopt env)) ->
let env, ty = TUtils.non_null env ty in
env, (r, Toption ty)
| _ -> env, ty in
@@ -0,0 +1,19 @@
<?hh // strict
// Copyright 2004-present Facebook. All Rights Reserved.
class C {
public static function decryptParams <Tk, Tv>(
): KeyedContainer<Tk, Tv> {
return dict[];
}
private ?string $title;
private bool $whatever = false;
public function bar(Map<string, bool> $m): void {
$m = C::decryptParams();
// This is rejected by legacy type checker if we remove the non_null hack
// from the checking of idx
$this->title = idx($m, 'title');
$this->whatever = (bool) idx($m, 'whatever', false);
}
}
@@ -0,0 +1,10 @@
<?hh // strict
// Copyright 2004-present Facebook. All Rights Reserved.
function expect_nullable_int(?int $_): void {}
function test(Vector<int> $is): void {
$m = Map {'foo' => 42};
$x = idx($m, 'foo');
expect_nullable_int($x);
}

0 comments on commit 4029873

Please sign in to comment.
You can’t perform that action at this time.