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

data.Const.__repr__ is very unhelpful for complex structures #1432

Open
whitequark opened this issue Jun 28, 2024 · 0 comments
Open

data.Const.__repr__ is very unhelpful for complex structures #1432

whitequark opened this issue Jun 28, 2024 · 0 comments
Labels
Milestone

Comments

@whitequark
Copy link
Member

For example, look at this assertion:

AssertionError: (cycle 0) Const(StructLayout({'port': StructLayout({'io0': StructLayout({'o': 1, 'oe': 1}), 'io1': StructLayout({'o': 1, 'oe': 1}), 'io2': StructLayout({'o': 1, 'oe': 1}), 'io3': StructLayout({'o': 1, 'oe': 1}), 'cs': StructLayout({'o': 1, 'oe': 1})}), 'i_en': 1, 'meta': <enum 'QSPIMode'>}), 1539) != Const(StructLayout({'port': StructLayout({'io0': StructLayout({'o': 1, 'oe': 1}), 'io1': StructLayout({'o': 1, 'oe': 1}), 'io2': StructLayout({'o': 1, 'oe': 1}), 'io3': StructLayout({'o': 1, 'oe': 1}), 'cs': StructLayout({'o': 1, 'oe': 1})}), 'i_en': 1, 'meta': <enum 'QSPIMode'>}), 1795)

It's completely incomprehensible. There are three issues:

  1. Information overload, with excess StructLayout everywhere. This is probably not solvable.
  2. Typing this back into a shell doesn't actually reproduce the value, because data.Const and Const have different argument order, and Const only accepts structured data anyway.
  3. The actual value is just a decimal number.

I think this needs at least two fixes:

  1. In general, change all amaranth.lib.* classes to prepend whatever * is to the repr. This matches our suggested import of from amaranth.lib import data, wiring, <etc> and isn't onerous, although it would make the "information overload" part surely worse.
  2. In this particular case, return hdl.Const and not a data.Const since the former casts a by-field decomposition back into a value, fulfilling both the repr contract, and being useful to read.
  3. Maybe ameliorating the info overload by adding newlines and indents to StructLayout and friends? It's kind of difficult to implement, but probably feasible, at least as an experiment.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

1 participant