hdl._dsl: raise an error when modifying an already-elaborated Module.

This renames the `FrozenMemory` exception to `AlreadyElaborated`
and reuses it for modules.

Fixes amaranth-lang#1350.

Author: wanda-phi

amaranth/hdl/__init__.py

@@ -6,9 +6,9 @@
 from ._ast import IOValue, IOPort
 from ._dsl import Module
 from ._cd import DomainError, ClockDomain
-from ._ir import UnusedElaboratable, Elaboratable, DriverConflict, Fragment
+from ._ir import AlreadyElaborated, UnusedElaboratable, Elaboratable, DriverConflict, Fragment
 from ._ir import Instance, IOBufferInstance
-from ._mem import FrozenMemory, MemoryData, MemoryInstance, Memory, ReadPort, WritePort, DummyPort
+from ._mem import MemoryData, MemoryInstance, Memory, ReadPort, WritePort, DummyPort
 from ._nir import CombinationalCycle
 from ._rec import Record
 from ._xfrm import DomainRenamer, ResetInserter, EnableInserter
@@ -27,12 +27,12 @@
     # _cd
     "DomainError", "ClockDomain",
     # _ir
-    "UnusedElaboratable", "Elaboratable", "DriverConflict", "Fragment",
+    "AlreadyElaborated", "UnusedElaboratable", "Elaboratable", "DriverConflict", "Fragment",
     "Instance", "IOBufferInstance",
     # _nir
     "CombinationalCycle",
     # _mem
-    "FrozenMemory", "MemoryData", "MemoryInstance", "Memory", "ReadPort", "WritePort", "DummyPort",
+    "MemoryData", "MemoryInstance", "Memory", "ReadPort", "WritePort", "DummyPort",
     # _rec
     "Record",
     # _xfrm

amaranth/hdl/_dsl.py

@@ -288,8 +288,11 @@ def __init__(self):
         self._domains      = {}
         self._generated    = {}
         self._src_loc      = tracer.get_src_loc()
+        self._frozen       = False
 
     def _check_context(self, construct, context):
+        if self._frozen:
+            raise AlreadyElaborated(f"Cannot modify a module that has already been elaborated")
         if self._ctrl_context != context:
             if self._ctrl_context is None:
                 raise SyntaxError("{} is not permitted outside of {}"
@@ -616,6 +619,9 @@ def _pop_ctrl(self):
                     src_loc=src_loc))
 
     def _add_statement(self, assigns, domain, depth):
+        if self._frozen:
+            raise AlreadyElaborated(f"Cannot modify a module that has already been elaborated")
+
         while len(self._ctrl_stack) > self.domain._depth:
             self._pop_ctrl()
 
@@ -643,6 +649,8 @@ def _add_submodule(self, submodule, name=None, src_loc=None):
         if not hasattr(submodule, "elaborate"):
             raise TypeError("Trying to add {!r}, which does not implement .elaborate(), as "
                             "a submodule".format(submodule))
+        if self._frozen:
+            raise AlreadyElaborated(f"Cannot modify a module that has already been elaborated")
         if name == None:
             self._anon_submodules.append((submodule, src_loc))
         else:
@@ -660,6 +668,8 @@ def _get_submodule(self, name):
     def _add_domain(self, cd):
         if cd.name in self._domains:
             raise NameError(f"Clock domain named '{cd.name}' already exists")
+        if self._frozen:
+            raise AlreadyElaborated(f"Cannot modify a module that has already been elaborated")
         self._domains[cd.name] = cd
 
     def _flush(self):
@@ -668,6 +678,7 @@ def _flush(self):
 
     def elaborate(self, platform):
         self._flush()
+        self._frozen = True
 
         fragment = Fragment(src_loc=self._src_loc)
         for name, (submodule, src_loc) in self._named_submodules.items():

amaranth/hdl/_ir.py

@@ -3,17 +3,23 @@
 import enum
 import warnings
 
-from .._utils import flatten, to_binary
+from .._utils import flatten, to_binary, final
 from .. import tracer, _unused
 from . import _ast, _cd, _ir, _nir
 
 
 __all__ = [
-    "UnusedElaboratable", "Elaboratable", "DuplicateElaboratable", "DriverConflict", "Fragment",
-    "Instance", "IOBufferInstance", "PortDirection", "Design", "build_netlist",
+    "AlreadyElaborated", "UnusedElaboratable", "Elaboratable", "DuplicateElaboratable",
+    "DriverConflict", "Fragment", "Instance", "IOBufferInstance", "PortDirection", "Design",
+    "build_netlist",
 ]
 
 
+@final
+class AlreadyElaborated(Exception):
+    """Exception raised when an elaboratable is being modified after elaboration."""
+
+
 class UnusedElaboratable(_unused.UnusedMustUse):
     # The warning is initially silenced. If everything that has been constructed remains unused,
     # it means the application likely crashed (with an exception, or in another way that does not

amaranth/hdl/_mem.py

@@ -4,17 +4,12 @@
 
 from .. import tracer
 from ._ast import *
-from ._ir import Elaboratable, Fragment
+from ._ir import Elaboratable, Fragment, AlreadyElaborated
 from ..utils import ceil_log2
 from .._utils import deprecated, final
 
 
-__all__ = ["FrozenMemory", "MemoryData", "Memory", "ReadPort", "WritePort", "DummyPort"]
-
-
-@final
-class FrozenMemory(Exception):
-    """Exception raised when a memory array is being modified after elaboration."""
+__all__ = ["MemoryData", "Memory", "ReadPort", "WritePort", "DummyPort"]
 
 
 @final
@@ -30,7 +25,7 @@ class MemoryData:
     a default value for rows that are not explicitly initialized.
 
     Changing the initial contents of a :class:`MemoryData` is only possible until it is used to
-    elaborate a memory; afterwards, attempting to do so will raise :exc:`FrozenMemory`.
+    elaborate a memory; afterwards, attempting to do so will raise :exc:`AlreadyElaborated`.
 
     .. warning::
 
@@ -101,7 +96,7 @@ def __getitem__(self, index):
 
         def __setitem__(self, index, value):
             if self._frozen:
-                raise FrozenMemory("Cannot set 'init' on a memory that has already been elaborated")
+                raise AlreadyElaborated("Cannot set 'init' on a memory that has already been elaborated")
 
             if isinstance(index, slice):
                 indices = range(*index.indices(len(self._elems)))
@@ -181,7 +176,7 @@ def init(self):
     @init.setter
     def init(self, init):
         if self._frozen:
-            raise FrozenMemory("Cannot set 'init' on a memory that has already been elaborated")
+            raise AlreadyElaborated("Cannot set 'init' on a memory that has already been elaborated")
         self._init = MemoryData.Init(init, shape=self._shape, depth=self._depth)
 
     def __repr__(self):

amaranth/lib/memory.py

@@ -2,15 +2,14 @@
 from collections import OrderedDict
 from collections.abc import MutableSequence
 
-from ..hdl import MemoryData, MemoryInstance, Shape, ShapeCastable, Const
-from ..hdl._mem import FrozenMemory
+from ..hdl import MemoryData, MemoryInstance, Shape, ShapeCastable, Const, AlreadyElaborated
 from ..utils import ceil_log2
 from .._utils import final
 from .. import tracer
 from . import wiring, data
 
 
-__all__ = ["FrozenMemory", "Memory", "ReadPort", "WritePort"]
+__all__ = ["Memory", "ReadPort", "WritePort"]
 
 
 class Memory(wiring.Component):
@@ -24,7 +23,7 @@ class Memory(wiring.Component):
     the :ref:`elaborate <lang-elaboration>` method.
 
     Adding ports or changing initial contents of a :class:`Memory` is only possible until it is
-    elaborated; afterwards, attempting to do so will raise :class:`~amaranth.hdl.FrozenMemory`.
+    elaborated; afterwards, attempting to do so will raise :class:`~amaranth.hdl.AlreadyElaborated`.
 
     Platform overrides
     ------------------
@@ -121,7 +120,7 @@ def read_port(self, *, domain="sync", transparent_for=(), src_loc_at=0):
         :class:`ReadPort`
         """
         if self._frozen:
-            raise FrozenMemory("Cannot add a memory port to a memory that has already been elaborated")
+            raise AlreadyElaborated("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)
@@ -146,7 +145,7 @@ def write_port(self, *, domain="sync", granularity=None, src_loc_at=0):
         :class:`WritePort`
         """
         if self._frozen:
-            raise FrozenMemory("Cannot add a memory port to a memory that has already been elaborated")
+            raise AlreadyElaborated("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,

docs/stdlib/memory.rst

@@ -181,7 +181,8 @@ Simulation
 Memory description
 ==================
 
-.. autoexception:: amaranth.hdl.FrozenMemory
+.. autoexception:: amaranth.hdl.AlreadyElaborated
+   :noindex:
 
 .. autoclass:: amaranth.hdl.MemoryData
 

tests/test_hdl_dsl.py

@@ -6,6 +6,7 @@
 from amaranth.hdl._ast import *
 from amaranth.hdl._cd import *
 from amaranth.hdl._dsl import *
+from amaranth.hdl._ir import *
 from amaranth.lib.enum import Enum
 
 from .utils import *
@@ -975,3 +976,35 @@ def test_bug_1331(self):
                 r"^Domain name should not be prefixed with 'cd_' in `m.domains`, "
                 r"use `m.domains.rx = ...` instead$"):
             m.domains.cd_rx = ClockDomain()
+
+    def test_freeze(self):
+        a = Signal()
+        m = Module()
+        f = Fragment.get(m, None)
+
+        with self.assertRaisesRegex(AlreadyElaborated,
+                r"^Cannot modify a module that has already been elaborated$"):
+            m.d.comb += a.eq(1)
+
+        with self.assertRaisesRegex(AlreadyElaborated,
+                r"^Cannot modify a module that has already been elaborated$"):
+            with m.If(a):
+                pass
+
+        with self.assertRaisesRegex(AlreadyElaborated,
+                r"^Cannot modify a module that has already been elaborated$"):
+            with m.Switch(a):
+                pass
+
+        with self.assertRaisesRegex(AlreadyElaborated,
+                r"^Cannot modify a module that has already been elaborated$"):
+            with m.FSM():
+                pass
+
+        with self.assertRaisesRegex(AlreadyElaborated,
+                r"^Cannot modify a module that has already been elaborated$"):
+            m.submodules.a = Module()
+
+        with self.assertRaisesRegex(AlreadyElaborated,
+                r"^Cannot modify a module that has already been elaborated$"):
+            m.domains.sync = ClockDomain()

tests/test_lib_memory.py

@@ -439,15 +439,15 @@ def test_freeze(self):
         m = memory.Memory(shape=unsigned(8), depth=4, init=[])
         m.write_port()
         m.elaborate(None)
-        with self.assertRaisesRegex(memory.FrozenMemory,
+        with self.assertRaisesRegex(AlreadyElaborated,
                 r"^Cannot add a memory port to a memory that has already been elaborated$"):
             m.write_port()
-        with self.assertRaisesRegex(memory.FrozenMemory,
+        with self.assertRaisesRegex(AlreadyElaborated,
                 r"^Cannot add a memory port to a memory that has already been elaborated$"):
             m.read_port()
-        with self.assertRaisesRegex(memory.FrozenMemory,
+        with self.assertRaisesRegex(AlreadyElaborated,
                 r"^Cannot set 'init' on a memory that has already been elaborated$"):
             m.init = [1, 2, 3, 4]
-        with self.assertRaisesRegex(memory.FrozenMemory,
+        with self.assertRaisesRegex(AlreadyElaborated,
                 r"^Cannot set 'init' on a memory that has already been elaborated$"):
             m.init[0] = 1