amaranth-lang / amaranth

A modern hardware definition language and toolchain based on Python
https://amaranth-lang.org/docs/amaranth/
BSD 2-Clause "Simplified" License
1.54k stars 170 forks source link

RFC 15 breaks expectations of uses of `Signal.like` #794

Closed whitequark closed 1 year ago

whitequark commented 1 year ago

Consider:

from amaranth import *
from amaranth.hdl.ast import ShapeCastable
from amaranth.lib import data

x = data.StructLayout({"v": 1})

class Y(data.Struct):
    v: 1

sx = Signal(x)
print(sx, type(sx), isinstance(type(sx), ShapeCastable))
# <amaranth.lib.data.View object at 0x7fa1157e76d0> <class 'amaranth.lib.data.View'> False
sy = Signal(Y)
print(sy, type(sy), isinstance(type(sy), ShapeCastable))
# <__main__.Y object at 0x7fa1157e7ca0> <class '__main__.Y'> True

sxl = Signal.like(sx)
print(sxl, sxl.shape())
# (sig sxl) unsigned(1)
syl = Signal.like(sy)
print(syl, sxl.shape())
# (sig syl) unsigned(1)
whitequark commented 1 year ago

We can fix this easily for values whose Python type is shape-castable (this is entirely within defined protocol), but there's no way to go from a View instance to its Layout besides Layout.of. Just like RFC 8 and RFC 15 themselves replaced special-casing in lib.data with a general protocol, we'll likely need to add a protocol to replace Layout.of in a general way, such that Signal.like can call new_function_to_extract_shape_castable_from_value(other)(Signal(other.as_value()).

whitequark commented 1 year ago

It turns out that this is a problem for more than just Signal.like; it is also a problem for the use of Layout.of with non-View classes. Example:

from amaranth import *
from amaranth.lib import data

class A(data.Struct):
   a : unsigned(1)

layout = data.Layout.of(Signal(A))
print(layout.members) # AttributeError: type object 'c' has no attribute 'members'

I'm bumping this to 0.4, we clearly need to address this before the release.

whitequark commented 1 year ago

This should be fixed by RFC 22.

whitequark commented 1 year ago

See https://github.com/amaranth-lang/amaranth/issues/876.