lib.memory: improve and regularize diagnostics.

Author: whitequark

amaranth/lib/memory.py

@@ -294,7 +294,7 @@ class Signature(wiring.Signature):
 
         def __init__(self, *, addr_width, shape):
             if not isinstance(addr_width, int) or addr_width < 0:
-                raise TypeError(f"`addr_width` must be a non-negative int, not {addr_width!r}")
+                raise TypeError(f"Address width must be a non-negative integer, not {addr_width!r}")
             self._addr_width = addr_width
             self._shape = shape
             super().__init__({
@@ -326,24 +326,27 @@ def __repr__(self):
 
     def __init__(self, signature, *, memory, domain, transparent_for=(), path=None, src_loc_at=0):
         if not isinstance(signature, ReadPort.Signature):
-            raise TypeError(f"Expected `ReadPort.Signature`, not {signature!r}")
+            raise TypeError(f"Expected signature to be ReadPort.Signature, not {signature!r}")
         if memory is not None: # may be None if created via `Signature.create()`
             if not isinstance(memory, Memory):
-                raise TypeError(f"Expected `Memory` or `None`, not {memory!r}")
-            if signature.shape != memory.shape or Shape.cast(signature.shape) != Shape.cast(memory.shape):
-                raise ValueError(f"Memory shape {memory.shape!r} doesn't match port shape {signature.shape!r}")
+                raise TypeError(f"Expected memory to be Memory or None, not {memory!r}")
+            if (signature.shape != memory.shape or
+                    Shape.cast(signature.shape) != Shape.cast(memory.shape)):
+                raise ValueError(f"Memory shape {memory.shape!r} doesn't match "
+                                 f"port shape {signature.shape!r}")
             if signature.addr_width != ceil_log2(memory.depth):
-                raise ValueError(f"Memory address width {ceil_log2(memory.depth)!r} doesn't match port address width {signature.addr_width!r}")
+                raise ValueError(f"Memory address width {ceil_log2(memory.depth)!r} doesn't match "
+                                 f"port address width {signature.addr_width!r}")
         if not isinstance(domain, str):
-            raise TypeError(f"Domain has to be a string, not {domain!r}")
+            raise TypeError(f"Domain must be a string, not {domain!r}")
         transparent_for = tuple(transparent_for)
         for port in transparent_for:
             if not isinstance(port, WritePort):
-                raise TypeError("`transparent_for` must contain only `WritePort` instances")
+                raise TypeError("Transparency set must contain only WritePort instances")
             if memory is not None and port not in memory._write_ports:
-                raise ValueError("Transparent write ports must belong to the same memory")
+                raise ValueError("Ports in transparency set must belong to the same memory")
             if port.domain != domain:
-                raise ValueError("Transparent write ports must belong to the same domain")
+                raise ValueError("Ports in transparency set must belong to the same domain")
         self._signature = signature
         self._memory = memory
         self._domain = domain
@@ -420,24 +423,26 @@ class Signature(wiring.Signature):
 
         def __init__(self, *, addr_width, shape, granularity=None):
             if not isinstance(addr_width, int) or addr_width < 0:
-                raise TypeError(f"`addr_width` must be a non-negative int, not {addr_width!r}")
+                raise TypeError(f"Address width must be a non-negative integer, not {addr_width!r}")
             self._addr_width = addr_width
             self._shape = shape
             self._granularity = granularity
             if granularity is None:
                 en_width = 1
             elif not isinstance(granularity, int) or granularity < 0:
-                raise TypeError(f"Granularity must be a non-negative int or None, not {granularity!r}")
+                raise TypeError(f"Granularity must be a non-negative integer or None, "
+                                f"not {granularity!r}")
             elif not isinstance(shape, ShapeCastable):
                 actual_shape = Shape.cast(shape)
                 if actual_shape.signed:
-                    raise ValueError("Granularity cannot be specified with signed shape")
+                    raise ValueError("Granularity cannot be specified for a memory with "
+                                     "a signed shape")
                 elif actual_shape.width == 0:
                     en_width = 0
                 elif granularity == 0:
                     raise ValueError("Granularity must be positive")
                 elif actual_shape.width % granularity != 0:
-                    raise ValueError("Granularity must divide data width")
+                    raise ValueError("Granularity must evenly divide data width")
                 else:
                     en_width = actual_shape.width // granularity
             elif isinstance(shape, data.ArrayLayout):
@@ -446,11 +451,12 @@ def __init__(self, *, addr_width, shape, granularity=None):
                 elif granularity == 0:
                     raise ValueError("Granularity must be positive")
                 elif shape.length % granularity != 0:
-                    raise ValueError("Granularity must divide data array length")
+                    raise ValueError("Granularity must evenly divide data array length")
                 else:
                     en_width = shape.length // granularity
             else:
-                raise TypeError("Granularity can only be specified for plain unsigned `Shape` or `ArrayLayout`")
+                raise TypeError("Granularity can only be specified for memories whose shape "
+                                "is unsigned or data.ArrayLayout")
             super().__init__({
                 "en": wiring.In(en_width),
                 "addr": wiring.In(addr_width),
@@ -486,18 +492,21 @@ def __repr__(self):
 
     def __init__(self, signature, *, memory, domain, path=None, src_loc_at=0):
         if not isinstance(signature, WritePort.Signature):
-            raise TypeError(f"Expected `WritePort.Signature`, not {signature!r}")
+            raise TypeError(f"Expected signature to be WritePort.Signature, not {signature!r}")
         if memory is not None: # may be None if created via `Signature.create()`
             if not isinstance(memory, Memory):
-                raise TypeError(f"Expected `Memory` or `None`, not {memory!r}")
-            if signature.shape != memory.shape or Shape.cast(signature.shape) != Shape.cast(memory.shape):
-                raise ValueError(f"Memory shape {memory.shape!r} doesn't match port shape {signature.shape!r}")
+                raise TypeError(f"Expected memory to be Memory or None, not {memory!r}")
+            if (signature.shape != memory.shape or
+                    Shape.cast(signature.shape) != Shape.cast(memory.shape)):
+                raise ValueError(f"Memory shape {memory.shape!r} doesn't match "
+                                 f"port shape {signature.shape!r}")
             if signature.addr_width != ceil_log2(memory.depth):
-                raise ValueError(f"Memory address width {ceil_log2(memory.depth)!r} doesn't match port address width {signature.addr_width!r}")
+                raise ValueError(f"Memory address width {ceil_log2(memory.depth)!r} doesn't match "
+                                 f"port address width {signature.addr_width!r}")
         if not isinstance(domain, str):
-            raise TypeError(f"Domain has to be a string, not {domain!r}")
+            raise TypeError(f"Domain must be a string, not {domain!r}")
         if domain == "comb":
-            raise ValueError("Write port domain cannot be \"comb\"")
+            raise ValueError("Write ports cannot be asynchronous")
         self._signature = signature
         self._memory = memory
         self._domain = domain

tests/test_lib_memory.py

@@ -58,28 +58,29 @@ def test_signature(self):
 
     def test_signature_wrong(self):
         with self.assertRaisesRegex(TypeError,
-                "^`addr_width` must be a non-negative int, not -2$"):
+                r"^Address width must be a non-negative integer, not -2$"):
             memory.WritePort.Signature(addr_width=-2, shape=8)
         with self.assertRaisesRegex(TypeError,
-                "^Granularity must be a non-negative int or None, not -2$"):
+                r"^Granularity must be a non-negative integer or None, not -2$"):
             memory.WritePort.Signature(addr_width=4, shape=8, granularity=-2)
         with self.assertRaisesRegex(ValueError,
-                "^Granularity cannot be specified with signed shape$"):
+                r"^Granularity cannot be specified for a memory with a signed shape$"):
             memory.WritePort.Signature(addr_width=2, shape=signed(8), granularity=2)
         with self.assertRaisesRegex(TypeError,
-                "^Granularity can only be specified for plain unsigned `Shape` or `ArrayLayout`$"):
+                r"^Granularity can only be specified for memories whose shape is unsigned or "
+                r"data.ArrayLayout$"):
             memory.WritePort.Signature(addr_width=2, shape=MyStruct, granularity=2)
         with self.assertRaisesRegex(ValueError,
-                "^Granularity must be positive$"):
+                r"^Granularity must be positive$"):
             memory.WritePort.Signature(addr_width=2, shape=8, granularity=0)
         with self.assertRaisesRegex(ValueError,
-                "^Granularity must be positive$"):
+                r"^Granularity must be positive$"):
             memory.WritePort.Signature(addr_width=2, shape=data.ArrayLayout(8, 8), granularity=0)
         with self.assertRaisesRegex(ValueError,
-                "^Granularity must divide data width$"):
+                r"^Granularity must evenly divide data width$"):
             memory.WritePort.Signature(addr_width=2, shape=8, granularity=3)
         with self.assertRaisesRegex(ValueError,
-                "^Granularity must divide data array length$"):
+                r"^Granularity must evenly divide data array length$"):
             memory.WritePort.Signature(addr_width=2, shape=data.ArrayLayout(8, 8), granularity=3)
 
     def test_signature_eq(self):
@@ -134,17 +135,17 @@ def test_constructor(self):
     def test_constructor_wrong(self):
         signature = memory.ReadPort.Signature(shape=8, addr_width=4)
         with self.assertRaisesRegex(TypeError,
-                r"^Expected `WritePort.Signature`, not ReadPort.Signature\(.*\)$"):
+                r"^Expected signature to be WritePort.Signature, not ReadPort.Signature\(.*\)$"):
             memory.WritePort(signature, memory=None, domain="sync")
         signature = memory.WritePort.Signature(shape=8, addr_width=4, granularity=2)
         with self.assertRaisesRegex(TypeError,
-                r"^Domain has to be a string, not None$"):
+                r"^Domain must be a string, not None$"):
             memory.WritePort(signature, memory=None, domain=None)
         with self.assertRaisesRegex(TypeError,
-                r"^Expected `Memory` or `None`, not 'a'$"):
+                r"^Expected memory to be Memory or None, not 'a'$"):
             memory.WritePort(signature, memory="a", domain="sync")
         with self.assertRaisesRegex(ValueError,
-                r"^Write port domain cannot be \"comb\"$"):
+                r"^Write ports cannot be asynchronous$"):
             memory.WritePort(signature, memory=None, domain="comb")
         signature = memory.WritePort.Signature(shape=8, addr_width=4)
         m = memory.Memory(depth=8, shape=8, init=[])
@@ -186,7 +187,7 @@ def test_signature(self):
 
     def test_signature_wrong(self):
         with self.assertRaisesRegex(TypeError,
-                "^`addr_width` must be a non-negative int, not -2$"):
+                "^Address width must be a non-negative integer, not -2$"):
             memory.ReadPort.Signature(addr_width=-2, shape=8)
 
     def test_signature_eq(self):
@@ -245,14 +246,14 @@ def test_constructor(self):
     def test_constructor_wrong(self):
         signature = memory.WritePort.Signature(shape=8, addr_width=4)
         with self.assertRaisesRegex(TypeError,
-                r"^Expected `ReadPort.Signature`, not WritePort.Signature\(.*\)$"):
+                r"^Expected signature to be ReadPort.Signature, not WritePort.Signature\(.*\)$"):
             memory.ReadPort(signature, memory=None, domain="sync")
         signature = memory.ReadPort.Signature(shape=8, addr_width=4)
         with self.assertRaisesRegex(TypeError,
-                r"^Domain has to be a string, not None$"):
+                r"^Domain must be a string, not None$"):
             memory.ReadPort(signature, memory=None, domain=None)
         with self.assertRaisesRegex(TypeError,
-                r"^Expected `Memory` or `None`, not 'a'$"):
+                r"^Expected memory to be Memory or None, not 'a'$"):
             memory.ReadPort(signature, memory="a", domain="sync")
         signature = memory.ReadPort.Signature(shape=8, addr_width=4)
         m = memory.Memory(depth=8, shape=8, init=[])
@@ -266,15 +267,15 @@ def test_constructor_wrong(self):
         m = memory.Memory(depth=16, shape=8, init=[])
         port = m.read_port()
         with self.assertRaisesRegex(TypeError,
-                r"^`transparent_for` must contain only `WritePort` instances$"):
+                r"^Transparency set must contain only WritePort instances$"):
             memory.ReadPort(signature, memory=m, domain="sync", transparent_for=[port])
         write_port = m.write_port()
         m2 = memory.Memory(depth=16, shape=8, init=[])
         with self.assertRaisesRegex(ValueError,
-                r"^Transparent write ports must belong to the same memory$"):
+                r"^Ports in transparency set must belong to the same memory$"):
             memory.ReadPort(signature, memory=m2, domain="sync", transparent_for=[write_port])
         with self.assertRaisesRegex(ValueError,
-                r"^Transparent write ports must belong to the same domain$"):
+                r"^Ports in transparency set must belong to the same domain$"):
             memory.ReadPort(signature, memory=m, domain="other", transparent_for=[write_port])