Repository: amaranth-lang/amaranth
Pull request #1236 (2024-03-25 13:22:37)

lib.memory: raise an error on mutating already-elaborated memory.

codecov[bot]: ## [Codecov](https://app.codecov.io/gh/amaranth-lang/amaranth/pull/1236?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 89.84%. 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 [(`7b29d77`)](https://app.codecov.io/gh/amaranth-lang/amaranth/pull/1236?dropdown=coverage&src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=amaranth-lang).

> :exclamation: Current head 7b29d77 differs from pull request most recent head 5381175. Consider uploading reports for the commit 5381175 to get more accurate results


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


```diff
@@            Coverage Diff             @@
##             main    #1236      +/-   ##
==========================================
- Coverage   89.85%   89.84%   -0.02%     
==========================================
  Files          43       43              
  Lines        9957     9972      +15     
  Branches     2404     2409       +5     
==========================================
+ Hits         8947     8959      +12     
- Misses        817      818       +1     
- Partials      193      195       +2     
```



</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/amaranth-lang/amaranth/pull/1236?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/memory.py b/amaranth/lib/memory.py
index 298cb44..2361ea9 100644
--- a/amaranth/lib/memory.py
+++ b/amaranth/lib/memory.py
@@ -5,6 +5,7 @@ from collections.abc import MutableSequence
 from ..hdl import MemoryIdentity, MemoryInstance, Shape, ShapeCastable, Const
 from ..hdl._mem import MemorySimRead
 from ..utils import ceil_log2
+from .._utils import final
 from .. import tracer
 from . import wiring, data
 
@@ -12,6 +13,13 @@ from . import wiring, data
 __all__ = ["Memory", "ReadPort", "WritePort"]
 
 
+@final
+class FrozenError(Exception):
+    """This exception is raised when ports are added to a :class:`Memory` or its
+    :attr:`~Memory.init` attribute is changed after it has been elaborated once.
+    """
+
+
 class Memory(wiring.Component):
     """Addressable array of rows.
 
@@ -19,7 +27,8 @@ class Memory(wiring.Component):
     dimensions and initial contents using the :py:`shape`, :py:`depth`, and :py:`init` parameters,
     and then adding memory ports using the :meth:`read_port` and :meth:`write_port` methods.
     Because it is mutable, it should be created and used locally within
-    the :ref:`elaborate <lang-elaboration>` method.
+    the :ref:`elaborate <lang-elaboration>` method. It is an error to add ports to or change
+    initial contents of a memory after it has been elaborated.
 
     The :py:`init` parameter and assignment to the :py:`init` attribute have the same effect, with
     :class:`Memory.Init` converting elements of the iterable to match :py:`shape` and using
@@ -70,6 +79,7 @@ class Memory(wiring.Component):
                                 .format(depth))
             self._shape = shape
             self._depth = depth
+            self._frozen = False
 
             if isinstance(shape, ShapeCastable):
                 self._elems = [None] * depth
@@ -92,6 +102,9 @@ class Memory(wiring.Component):
             return self._elems[index]
 
         def __setitem__(self, index, value):
+            if self._frozen:
+                raise FrozenError("Cannot set 'init' on a memory that has already been elaborated")
+
             if isinstance(index, slice):
                 indices = range(*index.indices(len(self._elems)))
                 if len(value) != len(indices):
@@ -131,6 +144,7 @@ class Memory(wiring.Component):
         self._identity = MemoryIdentity()
         self._read_ports: "list[ReadPort]" = []
         self._write_ports: "list[WritePort]" = []
+        self._frozen = False
 
         super().__init__(wiring.Signature({}))
 
@@ -148,6 +162,8 @@ class Memory(wiring.Component):
 
     @init.setter
     def init(self, init):
+        if self._frozen:
+            raise FrozenError("Cannot set 'init' on a memory that has already been elaborated")
         self._init = Memory.Init(init, shape=self._shape, depth=self._depth)
 
     @property
@@ -179,6 +195,8 @@ class Memory(wiring.Component):
         -------
         :class:`ReadPort`
         """
+        if self._frozen:
+            raise FrozenError("Cannot add a memory port to a memory that has already been elaborated")
         signature = ReadPort.Signature(shape=self.shape, addr_width=ceil_log2(self.depth))
         return ReadPort(signature, memory=self, domain=domain, transparent_for=transparent_for,
                         src_loc_at=1 + src_loc_at)
@@ -202,6 +220,8 @@ class Memory(wiring.Component):
         -------
         :class:`WritePort`
         """
+        if self._frozen:
+            raise FrozenError("Cannot add a memory port to a memory that has already been elaborated")
         signature = WritePort.Signature(
             shape=self.shape, addr_width=ceil_log2(self.depth), granularity=granularity)
         return WritePort(signature, memory=self, domain=domain,
@@ -224,6 +244,8 @@ class Memory(wiring.Component):
         return tuple(self._write_ports)
 
     def elaborate(self, platform):
+        self._frozen = True
+        self._init._frozen = True
         if hasattr(platform, "get_memory"):
             return platform.get_memory(self)
 


Test patch:
diff --git a/tests/test_lib_memory.py b/tests/test_lib_memory.py
index 91eef93..adae6db 100644
--- a/tests/test_lib_memory.py
+++ b/tests/test_lib_memory.py
@@ -404,3 +404,20 @@ class MemoryTestCase(FHDLTestCase):
         self.assertIs(f._read_ports[1]._addr, rp1.addr)
         self.assertIs(f._read_ports[1]._data, rp1.data.as_value())
         self.assertIs(f._read_ports[1]._en, rp1.en)
+
+    def test_freeze(self):
+        m = memory.Memory(shape=unsigned(8), depth=4, init=[])
+        m.write_port()
+        m.elaborate(None)
+        with self.assertRaisesRegex(memory.FrozenError,
+                r"^Cannot add a memory port to a memory that has already been elaborated$"):
+            m.write_port()
+        with self.assertRaisesRegex(memory.FrozenError,
+                r"^Cannot add a memory port to a memory that has already been elaborated$"):
+            m.read_port()
+        with self.assertRaisesRegex(memory.FrozenError,
+                r"^Cannot set 'init' on a memory that has already been elaborated$"):
+            m.init = [1, 2, 3, 4]
+        with self.assertRaisesRegex(memory.FrozenError,
+                r"^Cannot set 'init' on a memory that has already been elaborated$"):
+            m.init[0] = 1