google / google/xls

Adding an stdlib function suddenly uncovers a type inference error

Open
#1,576 3 comments 0 reactions 0 assignees View on GitHub
bug dslx
Dominant language
C++
Stars
1.9k
Forks
283
Avg merge
2d 10h
Merged PRs (30d)
135

Description

**Describe the bug**
After adding a new, totally unrelated stdlib function, I am getting a type inference error on an unrelated section of the std.x library.

```
xls/dslx/stdlib/std.x?l=1159:21-1159:22
1157: // of the symbol name to allow recursion.
1158: fn clzt_pow2_2(value: bits[2]) -> uN[2] {
1159: const N_HALF = (2 >> 1) as s32;
~~~~~~~~~~~~~~~~~~~~~~~~~~^ TypeInferenceError: Could not infer a type for this number, please annotate a type.
1160: combine_clzt_halfs(clzt_pow2_1(value[-N_HALF:]), clzt_pow2_1(value[:-N_HALF]))
1161: }
```

This is flagging an existing, untouched function:

https://github.com/google/xls/blob/63a36e5845593b06df6647e0d02f00ea5b431084/xls/dslx/stdlib/std.x#L1126

It appears this is indeed a type inference error. For example, if you write such a constant in a vacuous function, you will get the same error.

However, it has been happily passing CI since April when introduced in https://github.com/google/xls/commit/506c62da7308cb20f3d4bc6961d9074ad0b52532.

Somehow the new function addition is surfacing the type inference error.

**To Reproduce**
Steps to reproduce the behavior:
This is a pretty weird repro:
1. `bazel test //xls/dslx/stdlib:std_dslx_test`. Should pass.
2. Add the following code somewhere above `clzt_pow1_1` (referenced above):

```
// Splits the input bit vector into an array of N equal bit slices. Takes bits from LSB -> MSB.
fn split_bits(x: bits[IN]) -> bits[OUT][N] {
const_assert!(IN % N == u32:0);
for (i, arr): (u32, bits[OUT][N]) in range(u32:0, N) {
update(arr, i, x[i * OUT+:bits[OUT]])
}(bits[OUT][N]:[bits[OUT]:0, ...])
}

#[test]
fn split_bits_one_element_test() {
let x = u16:0x3210;
let y = split_bits(x);
assert_eq(y[0], u16:0x3210);
}
```

3. `bazel test //xls/dslx/stdlib:std_dslx_test` should fail with type inference error.

This is a somewhat fragile repro. The following will cause the type inference error to disappear again:

1. Remove the test.
2. Move this new code to the bottom of the file.
3. Change the new function to:

```
fn foo() -> u32 {
42
}

#[test]
fn foo_test() {
assert_eq(foo(), u32:42);
}
```

**Expected behavior**
The type inference error should have always been present, or it should never be present if this is legal syntax.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.