Repository: amaranth-lang/amaranth
Pull request #776 (2023-05-12 15:49:59)

lib.data: implement extensibility as specified in RFC 8

codecov[bot]: ## [Codecov](https://app.codecov.io/gh/amaranth-lang/amaranth/pull/776?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=amaranth-lang) Report
> Merging [#776](https://app.codecov.io/gh/amaranth-lang/amaranth/pull/776?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=amaranth-lang) (2b1e586) into [main](https://app.codecov.io/gh/amaranth-lang/amaranth/commit/4398575322bd07a03732f787b798daaa83107303?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=amaranth-lang) (4398575) will **decrease** coverage by `0.05%`.
> The diff coverage is `80.00%`.

```diff
@@            Coverage Diff             @@
##             main     #776      +/-   ##
==========================================
- Coverage   82.16%   82.12%   -0.05%     
==========================================
  Files          51       51              
  Lines        6881     6882       +1     
  Branches     1673     1673              
==========================================
- Hits         5654     5652       -2     
- Misses       1033     1034       +1     
- Partials      194      196       +2     
```


| [Impacted Files](https://app.codecov.io/gh/amaranth-lang/amaranth/pull/776?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=amaranth-lang) | Coverage Δ | |
|---|---|---|
| [amaranth/lib/data.py](https://app.codecov.io/gh/amaranth-lang/amaranth/pull/776?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=amaranth-lang#diff-YW1hcmFudGgvbGliL2RhdGEucHk=) | `99.34% <80.00%> (-0.33%)` | :arrow_down: |

... and [1 file with indirect coverage changes](https://app.codecov.io/gh/amaranth-lang/amaranth/pull/776/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=amaranth-lang)

:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=amaranth-lang)


Patch:
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 <lang-shapecasting>` 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


Test patch:
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"^<tests\.test_lib_data\.ViewTestCase\.test_getitem_custom_call_wrong\.<locals>"
+                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'$"):