[Typing] Constaint generics from as checks do not work on immediate values in unreacheable code

[lexidor]

Describe the bug
A clear and concise description of what the bug is.

Type assertions in unreachable code give false positives for type constraints on expressions like as KeyedContainer<_, _>. The keytype is limited to a subtype of arraykey, but the typechecker forgets that in unreachable code.

Standalone code, or other way to reproduce the problem

This should not depend on installing any libraries or frameworks. Ideally, it should be possible to copy-paste this into a single file and reproduce the problem by running hhvm and/or hh_client

function weird(): void {
  return;
  dict(mixed_function() as KeyedContainer<_, _>); // Typing[4323] Invalid argument
}

function mixed_function(): mixed {
  return dict[];
}

Typing[4323] Invalid argument
   --> public/weird.hack
  3 |   dict(mixed_function() as KeyedContainer<_, _>);
    |        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |        ^^^^^^^^^^^^^^^^ But got nonnull from this "as" assertion
   --> /tmp/hh_server/hhi_37227195/container_functions.hhi
 81 | function dict<Tk as arraykey, Tv>(<<__OnlyRxIfImpl(\HH\Rx\Traversable::class), __MaybeMutable>> KeyedTraversable<Tk, Tv> $arr): dict<Tk, Tv>;
    |                     ^^^^^^^^ Some type constraint(s) are violated here
    |               ^^ Tk is a constrained type parameter
    |                     ^^^^^^^^ Expected arraykey

1 error found.

Expected behavior
A clear and concise description of what you expected to happen.

This code should not have a typeerror, since dict(mixed_function() as KeyedContainer<_, _>) is typesafe.

Screenshots

Desktop (please complete the following information):

• OS: Ubuntu 18.04
• HHVM Version:

HipHop VM 4.44.0 (rel)
Compiler: 1581359518_476528326
Repo schema: a5702ef4bee231bc227952e0fa4d060be2fe90b0
hackc-9afa6a03e410c96282d447b6cb5e4edc92e4cc13-4.44.0

Additional context

Removing the return; fixes the typeerror.

[lexidor]

@Wilfred This might help with the case in #8647 since you think it has something to do with generics allowing their constraints to be violated in unreachable code.

This somehow feels like it is the same problem, but in reverse.
The as KeyedContainer<_, _> gets its constraints ignored, which causes dict(KeyedTraversable<arraykey, mixed>) to start complaining.

Even smaller:

function main(): void {
    $huh = null;
    if ($huh is Container<_>) {
      // this block is never executed
    }

    takes_int("foo");
}

function takes_int<T as int>(T $_): void {}

Hack knows that the if block will never execute, so it allows generics to have their constraints violated inside that block. However, the scope of this 'inconsistent type environment' is too big, and it includes the takes_int call.

[Wilfred]

Interestingly, the following code produces no type errors:

function weird(): void {
  return;
  $x = mixed_function() as KeyedContainer<_, _>;
  dict($x);
}

function mixed_function(): mixed {
  return dict[];
}

Presumably this is because we always refine $x when we introduce a variable. I agree it's a little odd.

Dead code is really tricky. Users like to have some type checking there, but it's legitimate for the type checker to ignore it entirely. Dead code checks are a best effort.

Is this just a weird behaviour you noticed, or is this blocking legitimate code patterns?

[lexidor]

Is this just a weird behaviour you noticed, or is this blocking legitimate code patterns?

During development I often insert a return some value for debugging at the top of a function, if I want to reproduce buggy behavior.

You could say this is a legitimate use case, but this is not something you'd want to do in production.

The workaround is quite easy.

if(always_true()){
  return 'something I want to test quickly';
}
//... 

function always_true(): bool {
  return true;
}

The code below is not dead code to the typechecker, so it won't have this bug.

This would also have another benefit.
If you put the definition of always true in a git ignored file, hh_client would fail in Travis if I were to accidentally leave it in.

There is no need to fix this issue, but let's keep it open anyway.

[lexidor]

Superceded by #9013. This issue is a generalization of the concept described in this issue.