Document RFC 62.

This includes a few minor code changes:
- Removing redundant `lib.memory.Memory.Init = hdl.MemoryData.Init`
  re-export.
- Renaming `FrozenError` to `FrozenMemory` and moving it to `.hdl`;
- Marking `ReadPort` and `WritePort` as `@final`.

Author: whitequark

amaranth/hdl/__init__.py

@@ -8,7 +8,7 @@
 from ._cd import DomainError, ClockDomain
 from ._ir import UnusedElaboratable, Elaboratable, DriverConflict, Fragment
 from ._ir import Instance, IOBufferInstance
-from ._mem import MemoryData, MemoryInstance, Memory, ReadPort, WritePort, DummyPort
+from ._mem import FrozenMemory, MemoryData, MemoryInstance, Memory, ReadPort, WritePort, DummyPort
 from ._rec import Record
 from ._xfrm import DomainRenamer, ResetInserter, EnableInserter
 
@@ -29,7 +29,7 @@
     "UnusedElaboratable", "Elaboratable", "DriverConflict", "Fragment",
     "Instance", "IOBufferInstance",
     # _mem
-    "MemoryData", "MemoryInstance", "Memory", "ReadPort", "WritePort", "DummyPort",
+    "FrozenMemory", "MemoryData", "MemoryInstance", "Memory", "ReadPort", "WritePort", "DummyPort",
     # _rec
     "Record",
     # _xfrm

amaranth/hdl/_mem.py

@@ -9,21 +9,50 @@
 from .._utils import deprecated, final
 
 
-__all__ = ["MemoryData", "Memory", "ReadPort", "WritePort", "DummyPort"]
+__all__ = ["FrozenMemory", "MemoryData", "Memory", "ReadPort", "WritePort", "DummyPort"]
 
 
 @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 FrozenMemory(Exception):
+    """This exception is raised when a memory array is being modified after elaboration."""
 
 
 @final
 class MemoryData:
+    """Abstract description of a memory array.
+
+    A :class:`MemoryData` object describes the geometry (shape and depth) and the initial contents
+    of a memory array, without specifying the way in which it is accessed. It is conceptually
+    similar to an array of :class:`Signal`\\ s.
+
+    The :py:`init` parameter and assignment to the :py:`init` attribute have the same effect, with
+    :class:`MemoryData.Init` converting elements of the iterable to match :py:`shape` and using
+    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`.
+
+    .. warning::
+
+        Uninitialized memories (including ASIC memories and some FPGA memories) are
+        `not yet supported <https://github.com/amaranth-lang/amaranth/issues/270>`_, and
+        the :py:`init` parameter must be always provided, if only as :py:`init=[]`.
+
+    Parameters
+    ----------
+    shape : :ref:`shape-like <lang-shapelike>` object
+        Shape of each memory row.
+    depth : :class:`int`
+        Number of memory rows.
+    init : iterable of initial values
+        Initial values for memory rows.
+    """
+
     @final
     class Init(MutableSequence):
-        """Memory initialization data.
+        """Init(...)
+
+        Memory initialization data.
 
         This is a special container used only for initial contents of memories. It is similar
         to :class:`list`, but does not support inserting or deleting elements; its length is always
@@ -72,7 +101,7 @@ def __getitem__(self, index):
 
         def __setitem__(self, index, value):
             if self._frozen:
-                raise FrozenError("Cannot set 'init' on a memory that has already been elaborated")
+                raise FrozenMemory("Cannot set 'init' on a memory that has already been elaborated")
 
             if isinstance(index, slice):
                 indices = range(*index.indices(len(self._elems)))
@@ -117,7 +146,7 @@ def shape(self):
         def _lhs_signals(self):
             # This value cannot ever appear in a design.
             raise NotImplementedError # :nocov:
-        
+
         _rhs_signals = _lhs_signals
 
         def __repr__(self):
@@ -152,13 +181,28 @@ def init(self):
     @init.setter
     def init(self, init):
         if self._frozen:
-            raise FrozenError("Cannot set 'init' on a memory that has already been elaborated")
+            raise FrozenMemory("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):
         return f"(memory-data {self.name})"
 
     def __getitem__(self, index):
+        """Retrieve a memory row for simulation.
+
+        A :class:`MemoryData` object can be indexed with an :class:`int` to construct a special
+        value that can be used to read and write the selected memory row in a simulation testbench,
+        without having to create a memory port.
+
+        .. important::
+
+            Even in a simulation, the value returned by this function cannot be used in a module;
+            it can only be used with :py:`sim.get()` and :py:`sim.set()`.
+
+        Returns
+        -------
+        :class:`~amaranth.hdl.Value`, :ref:`assignable <lang-assignable>`
+        """
         index = operator.index(index)
         if index not in range(self.depth):
             raise IndexError(f"Index {index} is out of bounds (memory has {self.depth} rows)")

amaranth/lib/memory.py

@@ -3,53 +3,46 @@
 from collections.abc import MutableSequence
 
 from ..hdl import MemoryData, MemoryInstance, Shape, ShapeCastable, Const
-from ..hdl._mem import FrozenError
+from ..hdl._mem import FrozenMemory
 from ..utils import ceil_log2
 from .._utils import final
 from .. import tracer
 from . import wiring, data
 
 
-__all__ = ["Memory", "ReadPort", "WritePort"]
+__all__ = ["FrozenMemory", "Memory", "ReadPort", "WritePort"]
 
 
 class Memory(wiring.Component):
     """Addressable array of rows.
 
     This :ref:`component <wiring>` is used to construct a memory array by first specifying its
-    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. It is an error to add ports to or change
-    initial contents of a memory after it has been elaborated.
+    dimensions and initial contents using the :class:`~amaranth.hdl.MemoryData` object and
+    the :py:`data` parameter (or by providing :py:`shape`, :py:`depth`, and :py:`init` parameters
+    directly instead) 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 :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
-    a default value for rows that are not explicitly initialized.
+    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`.
 
-    .. warning::
-
-        Uninitialized memories (including ASIC memories and some FPGA memories) are
-        `not yet supported <https://github.com/amaranth-lang/amaranth/issues/270>`_, and
-        the :py:`init` parameter must be always provided, if only as :py:`init=[]`.
+    Platform overrides
+    ------------------
+    Define the :py:`get_memory()` platform method to override the implementation of
+    :class:`Memory`, e.g. to instantiate library cells directly.
 
     Parameters
     ----------
+    data : :class:`~amaranth.hdl.MemoryData`
+        Representation of memory geometry and contents.
     shape : :ref:`shape-like <lang-shapelike>` object
         Shape of each memory row.
     depth : :class:`int`
         Number of memory rows.
     init : iterable of initial values
         Initial values for memory rows.
-
-    Platform overrides
-    ------------------
-    Define the :py:`get_memory()` platform method to override the implementation of
-    :class:`Memory`, e.g. to instantiate library cells directly.
     """
 
-    Init = MemoryData.Init
-
     def __init__(self, data=None, *, shape=None, depth=None, init=None, attrs=None, src_loc_at=0):
         if data is None:
             if shape is None:
@@ -128,7 +121,7 @@ def read_port(self, *, domain="sync", transparent_for=(), src_loc_at=0):
         :class:`ReadPort`
         """
         if self._frozen:
-            raise FrozenError("Cannot add a memory port to a memory that has already been elaborated")
+            raise FrozenMemory("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)
@@ -153,7 +146,7 @@ def write_port(self, *, domain="sync", granularity=None, src_loc_at=0):
         :class:`WritePort`
         """
         if self._frozen:
-            raise FrozenError("Cannot add a memory port to a memory that has already been elaborated")
+            raise FrozenMemory("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,
@@ -195,6 +188,7 @@ def elaborate(self, platform):
         return instance
 
 
+@final
 class ReadPort:
     """A read memory port.
 
@@ -318,6 +312,7 @@ def transparent_for(self):
         return self._transparent_for
 
 
+@final
 class WritePort:
     """A write memory port.
 

docs/stdlib/memory.rst

@@ -170,17 +170,39 @@ However, the memory read port is also configured to be *transparent* relative to
     }
 
 
-Memories
-========
+Simulation
+==========
+
+.. todo::
+
+    This section will be written once the simulator itself is documented.
+
+
+Memory description
+==================
+
+.. autoexception:: amaranth.hdl.FrozenMemory
+
+.. autoclass:: amaranth.hdl.MemoryData
+
+
+Memory component
+================
 
 ..
     attributes are not documented because they can be easily used to break soundness and we don't
     document them for signals either; they are rarely necessary for interoperability
 
-.. autoclass:: Memory(*, depth, shape, init, src_loc_at=0)
-    :no-members:
+..
+    the following two directives document a pair of overloads of :class:`Memory`; this is a little
+    weird and not really how rst/sphinx are supposed to work but it results in a comprehensible
+    generated document. be careful to not break this!
+
+.. class:: Memory(data, *, src_loc_at=0)
 
-    .. autoclass:: amaranth.lib.memory::Memory.Init(...)
+.. autoclass:: Memory(*, shape, depth, init, src_loc_at=0)
+    :noindex:
+    :no-members:
 
     .. automethod:: read_port
 

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.FrozenError,
+        with self.assertRaisesRegex(memory.FrozenMemory,
                 r"^Cannot add a memory port to a memory that has already been elaborated$"):
             m.write_port()
-        with self.assertRaisesRegex(memory.FrozenError,
+        with self.assertRaisesRegex(memory.FrozenMemory,
                 r"^Cannot add a memory port to a memory that has already been elaborated$"):
             m.read_port()
-        with self.assertRaisesRegex(memory.FrozenError,
+        with self.assertRaisesRegex(memory.FrozenMemory,
                 r"^Cannot set 'init' on a memory that has already been elaborated$"):
             m.init = [1, 2, 3, 4]
-        with self.assertRaisesRegex(memory.FrozenError,
+        with self.assertRaisesRegex(memory.FrozenMemory,
                 r"^Cannot set 'init' on a memory that has already been elaborated$"):
             m.init[0] = 1