hdl: disallow signed(0) values with unclear semantics.

Fixes #807.
This commit is contained in:
Catherine 2023-09-03 04:27:17 +00:00
parent 21b5451036
commit f135226a79
3 changed files with 36 additions and 13 deletions

View file

@ -78,11 +78,15 @@ class Shape:
If ``False``, the value is unsigned. If ``True``, the value is signed two's complement. If ``False``, the value is unsigned. If ``True``, the value is signed two's complement.
""" """
def __init__(self, width=1, signed=False): def __init__(self, width=1, signed=False):
if not isinstance(width, int) or width < 0: if not isinstance(width, int):
raise TypeError("Width must be a non-negative integer, not {!r}" raise TypeError(f"Width must be an integer, not {width!r}")
.format(width)) if not signed and width < 0:
raise TypeError(f"Width of an unsigned value must be zero or a positive integer, "
f"not {width}")
if signed and width <= 0:
raise TypeError(f"Width of a signed value must be a positive integer, not {width}")
self.width = width self.width = width
self.signed = signed self.signed = bool(signed)
# The algorithm for inferring shape for standard Python enumerations is factored out so that # The algorithm for inferring shape for standard Python enumerations is factored out so that
# `Shape.cast()` and Amaranth's `EnumMeta.as_shape()` can both use it. # `Shape.cast()` and Amaranth's `EnumMeta.as_shape()` can both use it.
@ -116,7 +120,7 @@ class Shape:
return Shape(obj) return Shape(obj)
elif isinstance(obj, range): elif isinstance(obj, range):
if len(obj) == 0: if len(obj) == 0:
return Shape(0, obj.start < 0) return Shape(0)
signed = obj[0] < 0 or obj[-1] < 0 signed = obj[0] < 0 or obj[-1] < 0
width = max(bits_for(obj[0], signed), width = max(bits_for(obj[0], signed),
bits_for(obj[-1], signed)) bits_for(obj[-1], signed))

View file

@ -153,8 +153,8 @@ Shapes from ranges
Casting a shape from a :class:`range` ``r`` produces a shape that: Casting a shape from a :class:`range` ``r`` produces a shape that:
* has a width large enough to represent both ``min(r)`` and ``max(r)``, and * has a width large enough to represent both ``min(r)`` and ``max(r)``, but not larger, and
* is signed if either ``min(r)`` or ``max(r)`` are negative, unsigned otherwise. * is signed if ``r`` contains any negative values, unsigned otherwise.
Specifying a shape with a range is convenient for counters, indexes, and all other values whose width is derived from a set of numbers they must be able to fit: Specifying a shape with a range is convenient for counters, indexes, and all other values whose width is derived from a set of numbers they must be able to fit:
@ -184,6 +184,16 @@ Specifying a shape with a range is convenient for counters, indexes, and all oth
Amaranth detects uses of :class:`Const` and :class:`Signal` that invoke such an off-by-one error, and emits a diagnostic message. Amaranth detects uses of :class:`Const` and :class:`Signal` that invoke such an off-by-one error, and emits a diagnostic message.
.. note::
An empty range always casts to an ``unsigned(0)``, even if both of its bounds are negative.
This happens because, being empty, it does not contain any negative values.
.. doctest::
>>> Shape.cast(range(-1, -1))
unsigned(0)
.. _lang-shapeenum: .. _lang-shapeenum:

View file

@ -42,11 +42,20 @@ class ShapeTestCase(FHDLTestCase):
s3 = Shape(3, True) s3 = Shape(3, True)
self.assertEqual(s3.width, 3) self.assertEqual(s3.width, 3)
self.assertEqual(s3.signed, True) self.assertEqual(s3.signed, True)
s4 = Shape(0)
self.assertEqual(s4.width, 0)
self.assertEqual(s4.signed, False)
def test_make_wrong(self): def test_make_wrong(self):
with self.assertRaisesRegex(TypeError, with self.assertRaisesRegex(TypeError,
r"^Width must be a non-negative integer, not -1$"): r"^Width must be an integer, not 'a'$"):
Shape(-1) Shape("a")
with self.assertRaisesRegex(TypeError,
r"^Width of an unsigned value must be zero or a positive integer, not -1$"):
Shape(-1, signed=False)
with self.assertRaisesRegex(TypeError,
r"^Width of a signed value must be a positive integer, not 0$"):
Shape(0, signed=True)
def test_compare_non_shape(self): def test_compare_non_shape(self):
self.assertNotEqual(Shape(1, True), "hi") self.assertNotEqual(Shape(1, True), "hi")
@ -87,7 +96,7 @@ class ShapeTestCase(FHDLTestCase):
def test_cast_int_wrong(self): def test_cast_int_wrong(self):
with self.assertRaisesRegex(TypeError, with self.assertRaisesRegex(TypeError,
r"^Width must be a non-negative integer, not -1$"): r"^Width of an unsigned value must be zero or a positive integer, not -1$"):
Shape.cast(-1) Shape.cast(-1)
def test_cast_tuple_wrong(self): def test_cast_tuple_wrong(self):
@ -116,7 +125,7 @@ class ShapeTestCase(FHDLTestCase):
self.assertEqual(s6.signed, False) self.assertEqual(s6.signed, False)
s7 = Shape.cast(range(-1, -1)) s7 = Shape.cast(range(-1, -1))
self.assertEqual(s7.width, 0) self.assertEqual(s7.width, 0)
self.assertEqual(s7.signed, True) self.assertEqual(s7.signed, False)
s8 = Shape.cast(range(0, 10, 3)) s8 = Shape.cast(range(0, 10, 3))
self.assertEqual(s8.width, 4) self.assertEqual(s8.width, 4)
self.assertEqual(s8.signed, False) self.assertEqual(s8.signed, False)
@ -386,7 +395,7 @@ class ConstTestCase(FHDLTestCase):
def test_shape_wrong(self): def test_shape_wrong(self):
with self.assertRaisesRegex(TypeError, with self.assertRaisesRegex(TypeError,
r"^Width must be a non-negative integer, not -1$"): r"^Width of an unsigned value must be zero or a positive integer, not -1$"):
Const(1, -1) Const(1, -1)
def test_wrong_fencepost(self): def test_wrong_fencepost(self):
@ -1022,7 +1031,7 @@ class SignalTestCase(FHDLTestCase):
def test_shape_wrong(self): def test_shape_wrong(self):
with self.assertRaisesRegex(TypeError, with self.assertRaisesRegex(TypeError,
r"^Width must be a non-negative integer, not -10$"): r"^Width of an unsigned value must be zero or a positive integer, not -10$"):
Signal(-10) Signal(-10)
def test_name(self): def test_name(self):