Repository: amaranth-lang/amaranth
Pull request #1237 (2024-03-25 13:42:34)

hdl._ast: fix `_value_repr` computation.
Fixes fallout from #1165.

codecov[bot]: ## [Codecov](https://app.codecov.io/gh/amaranth-lang/amaranth/pull/1237?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=amaranth-lang) Report
Attention: Patch coverage is `92.30769%` with `1 lines` in your changes are missing coverage. Please review.
> Project coverage is 89.82%. Comparing base [(`11f7b88`)](https://app.codecov.io/gh/amaranth-lang/amaranth/commit/11f7b887ad382e71fa1ec98466fe9e6650aa7117?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=amaranth-lang) to head [(`756f349`)](https://app.codecov.io/gh/amaranth-lang/amaranth/pull/1237?dropdown=coverage&src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=amaranth-lang).

| [Files](https://app.codecov.io/gh/amaranth-lang/amaranth/pull/1237?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=amaranth-lang) | Patch % | Lines |
|---|---|---|
| [amaranth/hdl/\_repr.py](https://app.codecov.io/gh/amaranth-lang/amaranth/pull/1237?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=amaranth-lang#diff-YW1hcmFudGgvaGRsL19yZXByLnB5) | 87.50% | [1 Missing :warning: ](https://app.codecov.io/gh/amaranth-lang/amaranth/pull/1237?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=amaranth-lang) |

<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##             main    #1237      +/-   ##
==========================================
- Coverage   89.85%   89.82%   -0.04%     
==========================================
  Files          43       43              
  Lines        9957     9962       +5     
  Branches     2404     2404              
==========================================
+ Hits         8947     8948       +1     
- Misses        817      820       +3     
- Partials      193      194       +1     
```



</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/amaranth-lang/amaranth/pull/1237?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=amaranth-lang).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=amaranth-lang).


Patch:
diff --git a/amaranth/hdl/_ast.py b/amaranth/hdl/_ast.py
index 9fbf2b7..5541dd7 100644
--- a/amaranth/hdl/_ast.py
+++ b/amaranth/hdl/_ast.py
@@ -1972,9 +1972,11 @@ class Signal(Value, DUID, metaclass=_SignalMeta):
 
         if decoder is not None:
             # The value representation is specified explicitly. Since we do not expose `hdl._repr`,
-            # this is the only way to add a custom filter to the signal right now. The setter sets
-            # `self._value_repr` as well as the compatibility `self.decoder`.
-            pass
+            # this is the only way to add a custom filter to the signal right now. 
+            if isinstance(decoder, type) and issubclass(decoder, Enum):
+                self._value_repr = (_repr.Repr(_repr.FormatEnum(decoder), self),)
+            else:
+                self._value_repr = (_repr.Repr(_repr.FormatCustom(decoder), self),)
         else:
             # If it's an enum, expose it via `self.decoder` for compatibility, whether it's a Python
             # enum or an Amaranth enum. This also sets the value representation, even for custom
@@ -1995,20 +1997,16 @@ class Signal(Value, DUID, metaclass=_SignalMeta):
                 self._value_repr = (_repr.Repr(_repr.FormatInt(), self),)
 
         # Compute the value representation that will be used by Amaranth.
-        if decoder is None:
-            self._value_repr = (_repr.Repr(_repr.FormatInt(), self),)
-            self._decoder = None
-        elif not (isinstance(decoder, type) and issubclass(decoder, Enum)):
-            self._value_repr = (_repr.Repr(_repr.FormatCustom(decoder), self),)
-            self._decoder = decoder
-        else: # Violence. In the name of backwards compatibility!
-            self._value_repr = (_repr.Repr(_repr.FormatEnum(decoder), self),)
+        if isinstance(decoder, type) and issubclass(decoder, Enum):
+            # Violence. In the name of backwards compatibility!
             def enum_decoder(value):
                 try:
                     return "{0.name:}/{0.value:}".format(decoder(value))
                 except ValueError:
                     return str(value)
             self._decoder = enum_decoder
+        else:
+            self._decoder = decoder
 
     @property
     def width(self):
diff --git a/amaranth/hdl/_repr.py b/amaranth/hdl/_repr.py
index 398d10b..6c70db1 100644
--- a/amaranth/hdl/_repr.py
+++ b/amaranth/hdl/_repr.py
@@ -14,6 +14,9 @@ class FormatInt(Format):
     def format(self, value):
         return f"{value:d}"
 
+    def __repr__(self):
+        return f"FormatInt()"
+
 
 class FormatEnum(Format):
     def __init__(self, enum):
@@ -25,6 +28,9 @@ class FormatEnum(Format):
         except ValueError:
             return f"?/{value:d}"
 
+    def __repr__(self):
+        return f"FormatEnum({self.enum.__name__})"
+
 
 class FormatCustom(Format):
     def __init__(self, formatter):
@@ -33,6 +39,9 @@ class FormatCustom(Format):
     def format(self, value):
         return self.formatter(value)
 
+    def __repr__(self):
+        return f"FormatCustom({self.formatter})"
+
 
 class Repr:
     def __init__(self, format, value, *, path=()):
@@ -44,3 +53,6 @@ class Repr:
         self.format = format
         self.value  = value
         self.path   = path
+
+    def __repr__(self):
+        return f"Repr({self.format!r}, {self.value!r}, {self.path!r})"
\ No newline at end of file


Test patch:
diff --git a/tests/test_hdl_ast.py b/tests/test_hdl_ast.py
index d477855..d233b23 100644
--- a/tests/test_hdl_ast.py
+++ b/tests/test_hdl_ast.py
@@ -1321,6 +1321,7 @@ class SignalTestCase(FHDLTestCase):
         s = Signal(decoder=Color)
         self.assertEqual(s.decoder(1), "RED/1")
         self.assertEqual(s.decoder(3), "3")
+        self.assertRepr(s._value_repr, "(Repr(FormatEnum(Color), (sig s), ()),)")
 
     def test_enum(self):
         s1 = Signal(UnsignedEnum)
@@ -1328,6 +1329,7 @@ class SignalTestCase(FHDLTestCase):
         s2 = Signal(SignedEnum)
         self.assertEqual(s2.shape(), signed(2))
         self.assertEqual(s2.decoder(SignedEnum.FOO), "FOO/-1")
+        self.assertRepr(s2._value_repr, "(Repr(FormatEnum(SignedEnum), (sig s2), ()),)")
 
     def test_const_wrong(self):
         s1 = Signal()
@@ -1335,6 +1337,10 @@ class SignalTestCase(FHDLTestCase):
                 r"^Value \(sig s1\) cannot be converted to an Amaranth constant$"):
             Const.cast(s1)
 
+    def test_value_repr(self):
+        s = Signal()
+        self.assertRepr(s._value_repr, "(Repr(FormatInt(), (sig s), ()),)")
+
 
 class ClockSignalTestCase(FHDLTestCase):
     def test_domain(self):
diff --git a/tests/test_lib_data.py b/tests/test_lib_data.py
index 19c46db..0426053 100644
--- a/tests/test_lib_data.py
+++ b/tests/test_lib_data.py
@@ -458,6 +458,11 @@ class ViewTestCase(FHDLTestCase):
         self.assertIsInstance(cv, Signal)
         self.assertEqual(cv.shape(), unsigned(3))
         self.assertEqual(cv.name, "v")
+        self.assertRepr(cv._value_repr, """
+        (Repr(FormatInt(), (sig v), ()),
+            Repr(FormatInt(), (slice (sig v) 0:1), ('a',)),
+            Repr(FormatInt(), (slice (sig v) 1:3), ('b',)))
+        """)
 
     def test_construct_signal_init(self):
         v1 = Signal(data.StructLayout({"a": unsigned(1), "b": unsigned(2)}),
diff --git a/tests/test_lib_enum.py b/tests/test_lib_enum.py
index 6d7799b..9d8d0cf 100644
--- a/tests/test_lib_enum.py
+++ b/tests/test_lib_enum.py
@@ -137,6 +137,7 @@ class EnumTestCase(FHDLTestCase):
             B = -3
         a = Signal(EnumA)
         self.assertRepr(a, "(sig a)")
+        self.assertRepr(a._value_repr, "(Repr(FormatEnum(EnumA), (sig a), ()),)")
 
     def test_enum_view(self):
         class EnumA(Enum, shape=signed(4)):