hdl.ast: warn on fencepost error in Signal(range(x), reset=x).

Also, relax the language reference inset from "warning" to "note"
since this is no longer something developers have to keep in mind
explicitly.
This commit is contained in:
Catherine 2023-03-13 20:27:13 +00:00
parent 32eabd9372
commit 80343d1c4c
3 changed files with 33 additions and 4 deletions

View file

@ -632,6 +632,13 @@ class Const(Value):
elif isinstance(shape, int):
shape = Shape(shape, signed=self.value < 0)
else:
if isinstance(shape, range) and self.value == shape.stop:
warnings.warn(
message="Value {!r} equals the non-inclusive end of the constant "
"shape {!r}; this is likely an off-by-one error"
.format(self.value, shape),
category=SyntaxWarning,
stacklevel=2)
shape = Shape.cast(shape, src_loc_at=1 + src_loc_at)
self.width = shape.width
self.signed = shape.signed
@ -1023,6 +1030,14 @@ class Signal(Value, DUID):
self.reset = reset.value
self.reset_less = bool(reset_less)
if isinstance(orig_shape, range) and self.reset == orig_shape.stop:
warnings.warn(
message="Reset value {!r} equals the non-inclusive end of the signal "
"shape {!r}; this is likely an off-by-one error"
.format(self.reset, orig_shape),
category=SyntaxWarning,
stacklevel=2)
self.attrs = OrderedDict(() if attrs is None else attrs)
if decoder is None and isinstance(orig_shape, type) and issubclass(orig_shape, Enum):

View file

@ -163,7 +163,7 @@ Specifying a shape with a range is convenient for counters, indexes, and all oth
.. _lang-exclrange:
.. warning::
.. note::
Python ranges are *exclusive* or *half-open*, meaning they do not contain their ``.stop`` element. Because of this, values with shapes cast from a ``range(stop)`` where ``stop`` is a power of 2 are not wide enough to represent ``stop`` itself:
@ -175,7 +175,7 @@ Specifying a shape with a range is convenient for counters, indexes, and all oth
>>> fencepost.value
0
Be mindful of this edge case!
Amaranth detects uses of :class:`Const` and :class:`Signal` that invoke such an off-by-one error, and emits a diagnostic message.
.. _lang-shapeenum:

View file

@ -357,6 +357,12 @@ class ConstTestCase(FHDLTestCase):
r"^Width must be a non-negative integer, not -1$"):
Const(1, -1)
def test_wrong_fencepost(self):
with self.assertWarnsRegex(SyntaxWarning,
r"^Value 10 equals the non-inclusive end of the constant shape "
r"range\(0, 10\); this is likely an off-by-one error$"):
Const(10, range(10))
def test_normalization(self):
self.assertEqual(Const(0b10110, signed(5)).value, -10)
@ -961,8 +967,10 @@ class SignalTestCase(FHDLTestCase):
self.assertEqual(s8.shape(), signed(5))
s9 = Signal(range(-20, 16))
self.assertEqual(s9.shape(), signed(6))
s10 = Signal(range(0))
self.assertEqual(s10.shape(), unsigned(0))
with warnings.catch_warnings():
warnings.filterwarnings(action="ignore", category=SyntaxWarning)
s10 = Signal(range(0))
self.assertEqual(s10.shape(), unsigned(0))
s11 = Signal(range(1))
self.assertEqual(s11.shape(), unsigned(1))
@ -1006,6 +1014,12 @@ class SignalTestCase(FHDLTestCase):
r"^Reset value -2 will be truncated to the signal shape signed\(1\)$"):
Signal(signed(1), reset=-2)
def test_reset_wrong_fencepost(self):
with self.assertWarnsRegex(SyntaxWarning,
r"^Reset value 10 equals the non-inclusive end of the signal shape "
r"range\(0, 10\); this is likely an off-by-one error$"):
Signal(range(0, 10), reset=10)
def test_attrs(self):
s1 = Signal()
self.assertEqual(s1.attrs, {})