facebook / facebook/hhvm

Falsy checks for empty vecs break type checker soundness

Open
#8,838 1 comment 0 reactions 0 assignees View on GitHub
hack
Dominant language
C++
Stars
18.7k
Forks
3.1k
Avg merge
1h 47m
Merged PRs (30d)
2

Description

**Describe the bug**
An empty `vec[]` does not pass a truthy check but it still a vec. The typechecker incorrectly infers that values in the other branch of this check are not vecs.

**Standalone code, or other way to reproduce the problem**

The following code typechecks but results in a fatal error at runtime under hhvm 4.108

**`empty_falsey.hack`**

```Hack
use namespace HH\Lib\C;

<<__EntryPoint>>
function example(): void {
takes_vec(vec[""]); // works for non-empty vec
takes_vec(vec[]); // runtime exception for empty vec
}

function takes_vec(?vec $v): void {
// This truthy check does not pass for an empty vec, but the typechecker doesn't
// track the fact that empty vec miss this branch and are not converted to vecs
//
// If you replace the if ($v) with the next line, the typechecker can see the problem
// if ($v is nonnull && !C\is_empty($v)) {
if ($v) {
$v = 5;
}

// $v is inferred to be ?int but is `?(int | vec)`
takes_int($v);
}

function takes_int(?int $i): void {}
```

```
$ hh_client
No errors!

$ hhvm empty_falsey.hack

Fatal error: Uncaught TypeError: Argument 1 to takes_int() must be of type ?int, HH\vec given in /Users/greenberg/code/hhvm_repro/empty_falsey.hack:23
Stack trace:
#0 /Users/greenberg/code/hhvm_repro/empty_falsey.hack(20): takes_int()
#1 /Users/greenberg/code/hhvm_repro/empty_falsey.hack(6): takes_vec()
#2 (): example()
#3 {main}
```

**Expected behavior**

Typechecker error on line 20 (`takes_int($v);`) saying "Expected ?int, got vec

**Actual behavior**

(no error)

**Environment**
- Operating system: MacOS Big Sur 11.2.3
- Installation method: `brew install hhvm/hhvm/hhvm-4.108`

```
$ hhvm --version
HipHop VM 4.108.0 (rel)
Compiler: 1620152370_N
Repo schema: 09d2af8dd22201dd8d48e5dcfcaed281ff9422c7

$ hh_client --version
hackc-893a8c56ba3cbd052c6cff6e404215635966a29b-4.108.0
```

Contributor guide

Open the contributing guide

Research direction

Start by running hh_client on empty_falsey.hack and comparing its result with hhvm, focusing on the if ($v) branch in takes_vec and the takes_int($v) call on line 20. Done means the typechecker reports the expected ?int versus vec error for this reproduction.

Written by the indexing model from the issue text.

Assessment

Domain
compilers
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Clearly specified
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.