From 7166455a6a23ce3a7b6808eada147567df927bb4 Mon Sep 17 00:00:00 2001 From: Catherine Date: Fri, 3 Mar 2023 05:11:31 +0000 Subject: [PATCH] lib.data: implement extensibility as specified in RFC 8. See amaranth-lang/rfcs#8 and #772. --- amaranth/lib/data.py | 49 +++++++++++++++++++++++++++++++----------- amaranth/lib/enum.py | 14 ++++++++++++ tests/test_lib_data.py | 38 ++++++++++++++++++++++++++++++++ 3 files changed, 89 insertions(+), 12 deletions(-) diff --git a/amaranth/lib/data.py b/amaranth/lib/data.py index 5f39c5e..e8ca1ff 100644 --- a/amaranth/lib/data.py +++ b/amaranth/lib/data.py @@ -187,6 +187,20 @@ class Layout(ShapeCastable, metaclass=ABCMeta): return (isinstance(other, Layout) and self.size == other.size and dict(iter(self)) == dict(iter(other))) + def __call__(self, target): + """Create a view into a target. + + When a :class:`Layout` is used as the shape of a :class:`Field` and accessed through + a :class:`View`, this method is used to wrap the slice of the underlying value into + another view with this layout. + + Returns + ------- + View + ``View(self, target)`` + """ + return View(self, target) + def _convert_to_int(self, value): """Convert ``value``, which may be a dict or an array of field values, to an integer using the representation defined by this layout. @@ -560,10 +574,12 @@ class View(ValueCastable): ################ Slicing a view or accessing its attributes returns a part of the underlying value - corresponding to the field with that index or name, which is always an Amaranth value, but - it could also be a :class:`View` if the shape of the field is a :class:`Layout`, or - an instance of the data class if the shape of the field is a class deriving from - :class:`Struct` or :class:`Union`. + corresponding to the field with that index or name, which is itself either a value or + a value-castable object. If the shape of the field is a :class:`Layout`, it will be + a :class:`View`; if it is a class deriving from :class:`Struct` or :class:`Union`, it + will be an instance of that data class; if it is another + :ref:`shape-castable ` object implementing ``__call__``, it will be + the result of calling that method. Slicing a view whose layout is an :class:`ArrayLayout` can be done with an index that is an Amaranth value instead of a constant integer. The returned element is chosen dynamically @@ -639,9 +655,10 @@ class View(ValueCastable): """Slice the underlying value. A field corresponding to ``key`` is looked up in the layout. If the field's shape is - a :class:`Layout`, returns a :class:`View`. If it is a subclass of :class:`Struct` or - :class:`Union`, returns an instance of that class. Otherwise, returns an unspecified - Amaranth expression with the right shape. + a shape-castable object that has a ``__call__`` method, it is called and the result is + returned. Otherwise, ``as_shape`` is called repeatedly on the shape until either an object + with a ``__call__`` method is reached, or a ``Shape`` is returned. In the latter case, + returns an unspecified Amaranth expression with the right shape. Arguments --------- @@ -650,7 +667,7 @@ class View(ValueCastable): Returns ------- - :class:`Value`, inout + :class:`Value` or :class:`ValueCastable`, inout A slice of the underlying value defined by the field. Raises @@ -660,6 +677,8 @@ class View(ValueCastable): TypeError If ``key`` is a value-castable object, but the layout of the view is not a :class:`ArrayLayout`. + TypeError + If ``ShapeCastable.__call__`` does not return a value or a value-castable object. """ if isinstance(self.__layout, ArrayLayout): shape = self.__layout.elem_shape @@ -672,10 +691,16 @@ class View(ValueCastable): field = self.__layout[key] shape = field.shape value = self.__target[field.offset:field.offset + field.width] - if isinstance(shape, _AggregateMeta): - return shape(value) - if isinstance(shape, Layout): - return View(shape, value) + # Field guarantees that the shape-castable object is well-formed, so there is no need + # to handle erroneous cases here. + while isinstance(shape, ShapeCastable): + if hasattr(shape, "__call__"): + value = shape(value) + if not isinstance(value, (Value, ValueCastable)): + raise TypeError("{!r}.__call__() must return a value or " + "a value-castable object, not {!r}" + .format(shape, value)) + return value if Shape.cast(shape).signed: return value.as_signed() else: diff --git a/amaranth/lib/enum.py b/amaranth/lib/enum.py index 8784d8c..62f6036 100644 --- a/amaranth/lib/enum.py +++ b/amaranth/lib/enum.py @@ -123,6 +123,20 @@ class EnumMeta(ShapeCastable, py_enum.EnumMeta): raise TypeError("Enumeration '{}.{}' does not have a defined shape" .format(cls.__module__, cls.__qualname__)) + def __call__(cls, value): + # :class:`py_enum.Enum` uses ``__call__()`` for type casting: ``E(x)`` returns + # the enumeration member whose value equals ``x``. In this case, ``x`` must be a concrete + # value. + # Amaranth extends this to indefinite values, but conceptually the operation is the same: + # :class:`View` calls :meth:`Enum.__call__` to go from a :class:`Value` to something + # representing this enumeration with that value. + # At the moment however, for historical reasons, this is just the value itself. This works + # and is backwards-compatible but is limiting in that it does not allow us to e.g. catch + # comparisons with enum members of the wrong type. + if isinstance(value, Value): + return value + return super().__call__(value) + class Enum(py_enum.Enum, metaclass=EnumMeta): """Subclass of the standard :class:`enum.Enum` that has :class:`EnumMeta` as diff --git a/tests/test_lib_data.py b/tests/test_lib_data.py index 6b9d8f7..ff72e9c 100644 --- a/tests/test_lib_data.py +++ b/tests/test_lib_data.py @@ -364,6 +364,13 @@ class LayoutTestCase(TestCase): sc.shape = sc self.assertNotEqual(StructLayout({}), sc) + def test_call(self): + sl = StructLayout({"f": unsigned(1)}) + s = Signal(1) + v = sl(s) + self.assertIs(Layout.of(v), sl) + self.assertIs(v.as_value(), s) + class ViewTestCase(FHDLTestCase): def test_construct(self): @@ -468,6 +475,37 @@ class ViewTestCase(FHDLTestCase): self.assertRepr(v["t"][0]["u"], "(slice (slice (slice (sig v) 0:4) 0:2) 0:1)") self.assertRepr(v["t"][1]["v"], "(slice (slice (slice (sig v) 0:4) 2:4) 1:2)") + def test_getitem_custom_call(self): + class Reverser(ShapeCastable): + def as_shape(self): + return unsigned(2) + + def __call__(self, value): + return value[::-1] + + v = View(StructLayout({ + "f": Reverser() + })) + self.assertRepr(v.f, "(cat (slice (slice (sig v) 0:2) 1:2) " + " (slice (slice (sig v) 0:2) 0:1))") + + def test_getitem_custom_call_wrong(self): + class WrongCastable(ShapeCastable): + def as_shape(self): + return unsigned(2) + + def __call__(self, value): + pass + + v = View(StructLayout({ + "f": WrongCastable() + })) + with self.assertRaisesRegex(TypeError, + r"^" + r"\.WrongCastable object at 0x.+?>\.__call__\(\) must return a value or " + r"a value-castable object, not None$"): + v.f + def test_index_wrong_missing(self): with self.assertRaisesRegex(KeyError, r"^'a'$"):