google / google/xls

Confusing variable shadowing with `const if`

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

Description

**Describe the bug**
Preface: the actual algorithm isn't useful to implement, but reveals some interesting bugs.

```
pub fn array_rotl_fixed
(x: bits[ELEMENT_SZ][ARRAY_SZ]) -> bits[ELEMENT_SZ][ARRAY_SZ] {
const K = K % ARRAY_SZ;
const if K == 0 {
x
} else {
let left = array_slice(x, u32:0, zero!());
let right = array_slice(x, K, zero!());
array_rev(array_rev(left) ++ array_rev(right))
}
}

#[test]
fn array_rotl_fixed_test() {
let x = u8[4]:[0, 1, 2, 3];
assert_eq(array_rotl_fixed<1>(x), u8[4]:[1, 2, 3, 0]);
assert_eq(array_rotl_fixed<2>(x), u8[4]:[2, 3, 0, 1]);
assert_eq(array_rotl_fixed<4>(x), u8[4]:[0, 1, 2, 3]);
assert_eq(array_rotl_fixed<5>(x), u8[4]:[1, 2, 3, 0]);
}
```

will generate the error:

```
Error: INVALID_ARGUMENT: TypeInferenceError: platforms/hls/xls/fox/vpu/std_vpu.x:333:11-339:6 TypeInferenceError: type mismatch: uN[8][0] vs. uN[8][4]. The body of function `array_rotl_fixed` does not actually return the function's declared return type, which is `bits[ELEMENT_SZ][ARRAY_SZ]
```

That seems to come from evaluating `ARRAY_SZ - K` to 0, which shouldn't be possible. Indeed, if you add `const if K == 0 || K == ARRAY_SZ`, this error goes away (but another one takes its place).

**To Reproduce**
See above.

**Expected behavior**
There shouldn't be any 0-length arrays, due to the original `const if`. Indeed, if you change the `const if` to depend on a new constant name, the code works fine:

```
pub fn array_rotl_fixed
(x: bits[ELEMENT_SZ][ARRAY_SZ]) -> bits[ELEMENT_SZ][ARRAY_SZ] {
const K_MOD = K % ARRAY_SZ;
const if K_MOD == 0 {
x
} else {
let left = array_slice(x, u32:0, zero!());
let right = array_slice(x, K_MOD, zero!());
array_rev(array_rev(left) ++ array_rev(right))
}
}

#[test]
fn array_rotl_fixed_test() {
let x = u8[4]:[0, 1, 2, 3];
assert_eq(array_rotl_fixed<1>(x), u8[4]:[1, 2, 3, 0]);
assert_eq(array_rotl_fixed<2>(x), u8[4]:[2, 3, 0, 1]);
assert_eq(array_rotl_fixed<4>(x), u8[4]:[0, 1, 2, 3]);
assert_eq(array_rotl_fixed<5>(x), u8[4]:[1, 2, 3, 0]);
}
```

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.