Repository: amaranth-lang/amaranth
Pull request #1424 (2024-06-27 14:47:20)

lib.enum: privatize internal attributes in `EnumView`/`FlagView`
Before this commit, public names `enum` and `target` were not usable as enumeration member or method names. This was unintentional and our policy is to not reserve public names where they may clash with names defined in downstream code.

This commit makes them a private name (`__enum`, etc). This was chosen instead of a sunder name (`_amaranth_enum_`, etc) because there is a 1:1 correspondence between the attribute and a `ValueCastable` method:
* `shape()` -> `__enum`
* `as_value()` -> `__target`

codecov[bot]: ## [Codecov](https://app.codecov.io/gh/amaranth-lang/amaranth/pull/1424?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=amaranth-lang) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Project coverage is 91.13%. Comparing base [(`42d90a3`)](https://app.codecov.io/gh/amaranth-lang/amaranth/commit/42d90a3c001a6c219c1eda1d6d7bb8d1943d6cf7?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=amaranth-lang) to head [(`fe1f5fd`)](https://app.codecov.io/gh/amaranth-lang/amaranth/commit/fe1f5fd2ed8d0c64f8d918b351d969716c398148?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=amaranth-lang).
> Report is 1 commits behind head on main.

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


```diff
@@            Coverage Diff             @@
##             main    #1424      +/-   ##
==========================================
+ Coverage   90.83%   91.13%   +0.29%     
==========================================
  Files          43       43              
  Lines       10946    10963      +17     
  Branches     2305     2656     +351     
==========================================
+ Hits         9943     9991      +48     
+ Misses        830      801      -29     
+ Partials      173      171       -2     
```

</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/amaranth-lang/amaranth/pull/1424?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/lib/enum.py b/amaranth/lib/enum.py
index d6463c3..d9d415b 100644
--- a/amaranth/lib/enum.py
+++ b/amaranth/lib/enum.py
@@ -234,16 +234,16 @@ class EnumView(ValueCastable):
                             .format(target)) from e
         if cast_target.shape() != enum.as_shape():
             raise TypeError("EnumView target must have the same shape as the enum")
-        self.enum = enum
-        self.target = cast_target
+        self.__enum = enum
+        self.__target = cast_target
 
     def shape(self):
         """Returns the underlying enum type."""
-        return self.enum
+        return self.__enum
 
     def as_value(self):
         """Returns the underlying value."""
-        return self.target
+        return self.__target
 
     def eq(self, other):
         """Assign to the underlying value.
@@ -296,21 +296,25 @@ class EnumView(ValueCastable):
         :class:`Value`
             The result of the equality comparison, as a single-bit value.
         """
-        if isinstance(other, self.enum):
-            other = self.enum(Value.cast(other))
-        if not isinstance(other, EnumView) or other.enum is not self.enum:
-            raise TypeError("an EnumView can only be compared to value or other EnumView of the same enum type")
-        return self.target == other.target
+        enum_cls = self.shape()
+        if isinstance(other, enum_cls):
+            other = enum_cls(Value.cast(other))
+        if not isinstance(other, EnumView) or other.shape() is not enum_cls:
+            raise TypeError("an EnumView can only be compared to value or other EnumView of "
+                            "the same enum type")
+        return self.as_value() == other.as_value()
 
     def __ne__(self, other):
-        if isinstance(other, self.enum):
-            other = self.enum(Value.cast(other))
-        if not isinstance(other, EnumView) or other.enum is not self.enum:
-            raise TypeError("an EnumView can only be compared to value or other EnumView of the same enum type")
-        return self.target != other.target
+        enum_cls = self.shape()
+        if isinstance(other, enum_cls):
+            other = enum_cls(Value.cast(other))
+        if not isinstance(other, EnumView) or other.shape() is not enum_cls:
+            raise TypeError("an EnumView can only be compared to value or other EnumView of "
+                            "the same enum type")
+        return self.as_value() != other.as_value()
 
     def __repr__(self):
-        return f"{type(self).__qualname__}({self.enum.__qualname__}, {self.target!r})"
+        return f"{type(self).__qualname__}({self.__enum.__qualname__}, {self.__target!r})"
 
 
 class FlagView(EnumView):
@@ -330,21 +334,23 @@ class FlagView(EnumView):
         -------
         :class:`FlagView`
         """
-        if hasattr(self.enum, "_boundary_") and self.enum._boundary_ in (EJECT, KEEP):
-            return self.enum._amaranth_view_class_(self.enum, ~self.target)
+        enum_cls = self.shape()
+        if hasattr(enum_cls, "_boundary_") and enum_cls._boundary_ in (EJECT, KEEP):
+            return enum_cls._amaranth_view_class_(enum_cls, ~self.as_value())
         else:
             singles_mask = 0
-            for flag in self.enum:
+            for flag in enum_cls:
                 if (flag.value & (flag.value - 1)) == 0:
                     singles_mask |= flag.value
-            return self.enum._amaranth_view_class_(self.enum, ~self.target & singles_mask)
+            return enum_cls._amaranth_view_class_(enum_cls, ~self.as_value() & singles_mask)
 
     def __bitop(self, other, op):
-        if isinstance(other, self.enum):
-            other = self.enum(Value.cast(other))
-        if not isinstance(other, FlagView) or other.enum is not self.enum:
+        enum_cls = self.shape()
+        if isinstance(other, enum_cls):
+            other = enum_cls(Value.cast(other))
+        if not isinstance(other, FlagView) or other.shape() is not enum_cls:
             raise TypeError("a FlagView can only perform bitwise operation with a value or other FlagView of the same enum type")
-        return self.enum._amaranth_view_class_(self.enum, op(self.target, other.target))
+        return enum_cls._amaranth_view_class_(enum_cls, op(self.as_value(), other.as_value()))
 
     def __and__(self, other):
         """Performs a bitwise AND and returns another :class:`FlagView`.
diff --git a/amaranth/lib/io.py b/amaranth/lib/io.py
index e4ea8b2..fc2d101 100644
--- a/amaranth/lib/io.py
+++ b/amaranth/lib/io.py
@@ -203,10 +203,10 @@ class SingleEndedPort(PortLike):
                                direction=self._direction & other._direction)
 
     def __repr__(self):
-        if all(self._invert):
-            invert = True
-        elif not any(self._invert):
+        if not any(self._invert):
             invert = False
+        elif all(self._invert):
+            invert = True
         else:
             invert = self._invert
         return f"SingleEndedPort({self._io!r}, invert={invert!r}, direction={self._direction})"


Test patch:
diff --git a/tests/test_lib_io.py b/tests/test_lib_io.py
index 9dc208e..c8d75f4 100644
--- a/tests/test_lib_io.py
+++ b/tests/test_lib_io.py
@@ -88,6 +88,12 @@ class SingleEndedPortTestCase(FHDLTestCase):
         iport = ~port
         self.assertRepr(iport, "SingleEndedPort((io-port io), invert=(False, True, False, True), direction=Direction.Output)")
 
+    def test_empty(self):
+        io = IOPort(1)
+        port = SingleEndedPort(io, invert=False)
+        eport = port[0:0]
+        self.assertRepr(eport, "SingleEndedPort((io-slice (io-port io) 0:0), invert=False, direction=Direction.Bidir)")
+
 
 class DifferentialPortTestCase(FHDLTestCase):
     def test_construct(self):
@@ -160,6 +166,13 @@ class DifferentialPortTestCase(FHDLTestCase):
         iport = ~port
         self.assertRepr(iport, "DifferentialPort((io-port iop), (io-port ion), invert=(False, True, False, True), direction=Direction.Output)")
 
+    def test_empty(self):
+        iop = IOPort(1)
+        ion = IOPort(1)
+        port = DifferentialPort(iop, ion, invert=False)
+        eport = port[0:0]
+        self.assertRepr(eport, "DifferentialPort((io-slice (io-port iop) 0:0), (io-slice (io-port ion) 0:0), invert=False, direction=Direction.Bidir)")
+
 
 class BufferTestCase(FHDLTestCase):
     def test_signature(self):