clash-lang / clash-compiler

Haskell to VHDL/Verilog/SystemVerilog compiler
https://clash-lang.org/
Other
1.4k stars 147 forks source link

Change Signal.fromList's tail from errorX to error #2620

Open leonschoorl opened 7 months ago

leonschoorl commented 7 months ago

By making the undefined tail of a signal created with fromList into an error instead of errorX, this makes it clear when you're trying use a signal without enough input data.

A problem with the errorX was that in certain circumstances it can be turned into a signal full of XExceptions. That can turn into a signal full of undefined BitVectors. And when that is used as the basis for the expected values of a outputVerifierBitVector you end up with a testbench that reports everything is fine for some (possibly big) part of the test.

Also add HasCallStack to Signal.fromList to help with tracing this error.

This currently fails CI because it exposes the exact problem described above in XpmCdcSingle and XpmCdcArraySingle. I'm working on fixing those tests.

Still TODO:

christiaanb commented 7 months ago

This is the commit that originally introduced errorX https://github.com/clash-lang/clash-prelude/commit/fe5305c681e53f2dc950fc2b2c5172db8650b5d2

Sadly the commit message doesn't tell us a whole lot why.

DigitalBrains1 commented 6 months ago

I think we should only use XException in signal values, to indicate undefined values. Using it for the Signal itself instead of for the a in Signal dom a sounds wrong to me, and surprising behaviour. So I'm in favour of changing it to error here. If there is a use case for some non-ErrorCall exception here, it might be better to invent a new exception for that instead of using XException.