Implement RFC 37: Make Signature immutable.

Fixes #995.

Author: wanda-phi

amaranth/lib/wiring.py

@@ -151,8 +151,12 @@ class SignatureError(Exception):
 class SignatureMembers(Mapping):
     def __init__(self, members=()):
         self._dict = dict()
-        self._frozen = False
-        self += members
+        for name, member in dict(members).items():
+            self._check_name(name)
+            if type(member) is not Member:
+                raise TypeError(f"Value {member!r} must be a member; "
+                                f"did you mean In({member!r}) or Out({member!r})?")
+            self._dict[name] = member
 
     def flip(self):
         return FlippedSignatureMembers(self)
@@ -179,16 +183,7 @@ def __getitem__(self, name):
         return self._dict[name]
 
     def __setitem__(self, name, member):
-        self._check_name(name)
-        if name in self._dict:
-            raise SignatureError(f"Member '{name}' already exists in the signature and cannot "
-                                 f"be replaced")
-        if type(member) is not Member:
-            raise TypeError(f"Assigned value {member!r} must be a member; "
-                            f"did you mean In({member!r}) or Out({member!r})?")
-        if self._frozen:
-            raise SignatureError("Cannot add members to a frozen signature")
-        self._dict[name] = member
+        raise SignatureError("Members cannot be added to a signature once constructed")
 
     def __delitem__(self, name):
         raise SignatureError("Members cannot be removed from a signature")
@@ -199,21 +194,6 @@ def __iter__(self):
     def __len__(self):
         return len(self._dict)
 
-    def __iadd__(self, members):
-        for name, member in dict(members).items():
-            self[name] = member
-        return self
-
-    @property
-    def frozen(self):
-        return self._frozen
-
-    def freeze(self):
-        self._frozen = True
-        for member in self.values():
-            if member.is_signature:
-                member.signature.freeze()
-
     def flatten(self, *, path=()):
         for name, member in self.items():
             yield ((*path, name), member)
@@ -244,8 +224,7 @@ def create_dimensions(dimensions, *, path, src_loc_at):
         return attrs
 
     def __repr__(self):
-        frozen_repr = ".freeze()" if self._frozen else ""
-        return f"SignatureMembers({self._dict}){frozen_repr}"
+        return f"SignatureMembers({self._dict})"
 
 
 @final
@@ -277,17 +256,6 @@ def __iter__(self):
     def __len__(self):
         return self.__unflipped.__len__()
 
-    def __iadd__(self, members):
-        self.__unflipped.__iadd__({name: member.flip() for name, member in members.items()})
-        return self
-
-    @property
-    def frozen(self):
-        return self.__unflipped.frozen
-
-    def freeze(self):
-        self.__unflipped.freeze()
-
     # These methods do not access instance variables and so their implementation can be shared
     # between the normal and the flipped member collections.
     flatten = SignatureMembers.flatten
@@ -358,12 +326,6 @@ def flip(self):
     def members(self):
         return self.__members
 
-    @members.setter
-    def members(self, new_members):
-        # The setter is called when `sig.members += ...` is used.
-        if new_members is not self.__members:
-            raise AttributeError("property 'members' of 'Signature' object cannot be set")
-
     def __eq__(self, other):
         other_unflipped = other.flip() if type(other) is FlippedSignature else other
         if type(self) is type(other_unflipped) is Signature:
@@ -374,14 +336,6 @@ def __eq__(self, other):
             # usually be overridden in a derived class.
             return self is other
 
-    @property
-    def frozen(self):
-        return self.members.frozen
-
-    def freeze(self):
-        self.members.freeze()
-        return self
-
     def flatten(self, obj):
         for name, member in self.members.items():
             path = (name,)
@@ -544,11 +498,6 @@ def flip(self):
     def members(self):
         return FlippedSignatureMembers(self.__unflipped.members)
 
-    @members.setter
-    def members(self, new_members):
-        if new_members.flip() is not self.__unflipped.members:
-            raise AttributeError("property 'members' of 'FlippedSignature' object cannot be set")
-
     def __eq__(self, other):
         if type(other) is FlippedSignature:
             # Trivial case.
@@ -561,10 +510,8 @@ def __eq__(self, other):
             # in infinite recursion.
             return NotImplemented
 
-    # These methods do not access instance variables and so their implementation can be shared
+    # This method does not access instance variables and so its implementation can be shared
     # between the normal and the flipped member collections.
-    frozen = Signature.frozen
-    freeze = Signature.freeze
     is_compliant = Signature.is_compliant
 
     # FIXME: document this logic
@@ -581,16 +528,10 @@ def __getattr__(self, name):
             return getattr(self.__unflipped, name)
 
     def __setattr__(self, name, value):
-        if name == "members":
-            # Although `sig.flip().members` does not call `__getattr__` but directly invokes
-            # the descriptor of the `FlippedSignature.members` property, `sig.flip().members +=`
-            # does call `__setattr__`, and this must be special-cased for the setter to work.
-            FlippedSignature.members.__set__(self, value)
-        else:
-            try: # descriptor first
-                _gettypeattr(self.__unflipped, name).__set__(self, value)
-            except AttributeError:
-                setattr(self.__unflipped, name, value)
+        try: # descriptor first
+            _gettypeattr(self.__unflipped, name).__set__(self, value)
+        except AttributeError:
+            setattr(self.__unflipped, name, value)
 
     def __delattr__(self, name):
         try: # descriptor first
@@ -703,7 +644,7 @@ def connect(m, *args, **kwargs):
             reasons_as_string = "".join("\n- " + reason for reason in reasons)
             raise ConnectionError(f"Argument {handle!r} does not match its signature:" +
                                   reasons_as_string)
-        signatures[handle] = obj.signature.freeze()
+        signatures[handle] = obj.signature
 
     # Collate signatures and build connections.
     flattens = {handle: signature.members.flatten()
@@ -875,8 +816,8 @@ def __init__(self):
     @property
     def signature(self):
         cls = type(self)
-        signature = Signature({})
-        for base in cls.mro()[:cls.mro().index(Component)]:
+        members = {}
+        for base in reversed(cls.mro()[:cls.mro().index(Component)]):
             for name, annot in base.__dict__.get("__annotations__", {}).items():
                 if name.startswith("_"):
                     continue
@@ -897,9 +838,11 @@ def signature(self):
                         category=SyntaxWarning,
                         stacklevel=2)
                 elif type(annot) is Member:
-                    signature.members[name] = annot
-        if not signature.members:
+                    if name in members:
+                        raise SignatureError(f"Member '{name}' is redefined in {base.__module__}.{base.__qualname__}")
+                    members[name] = annot
+        if not members:
             raise NotImplementedError(
                 f"Component '{cls.__module__}.{cls.__qualname__}' does not have signature member "
                 f"annotations")
-        return signature
+        return Signature(members)

docs/changes.rst

@@ -63,6 +63,7 @@ Implemented RFCs
 .. _RFC 31: https://amaranth-lang.org/rfcs/0031-enumeration-type-safety.html
 .. _RFC 34: https://amaranth-lang.org/rfcs/0034-interface-rename.html
 .. _RFC 35: https://amaranth-lang.org/rfcs/0035-shapelike-valuelike.html
+.. _RFC 37: https://amaranth-lang.org/rfcs/0037-make-signature-immutable.html
 
 
 * `RFC 1`_: Aggregate data structure library
@@ -83,6 +84,7 @@ Implemented RFCs
 * `RFC 31`_: Enumeration type safety
 * `RFC 34`_: Rename ``amaranth.lib.wiring.Interface`` to ``PureInterface``
 * `RFC 35`_: Add ``ShapeLike``, ``ValueLike``
+* `RFC 37`_: Make ``Signature`` immutable
 
 
 Language changes