Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

leonschoorl
Copy link
Member

@leonschoorl leonschoorl commented Nov 30, 2023

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:

  • Write a changelog entry (see changelog/README.md)
  • Check copyright notices are up to date in edited files
  • Fix the broken tests

@christiaanb
Copy link
Member

This is the commit that originally introduced errorX clash-lang/clash-prelude@fe5305c

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

@DigitalBrains1
Copy link
Member

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.

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 XException. 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.
After the provided list runs out, the tail of the signal is an infinite 
signal filled with error calls.
This makes sure that a simulation that happens to not use the values in 
this signal can still make progress.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants