Skip to content

Commit

Permalink
hdl.ast: change warning on out-of-range reset to an error, improve it.
Browse files Browse the repository at this point in the history
Fixes #1019.
  • Loading branch information
wanda-phi authored and whitequark committed Jan 30, 2024
1 parent 8501d9d commit e9299cc
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 12 deletions.
15 changes: 8 additions & 7 deletions amaranth/hdl/ast.py
Original file line number Diff line number Diff line change
Expand Up @@ -1170,13 +1170,14 @@ def __init__(self, shape=None, *, name=None, reset=None, reset_less=False,
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)
if isinstance(orig_shape, range) and orig_reset is not None and orig_reset not in orig_shape:
if orig_reset == orig_shape.stop:
raise SyntaxError(
f"Reset value {orig_reset!r} equals the non-inclusive end of the signal "
f"shape {orig_shape!r}; this is likely an off-by-one error")
else:
raise SyntaxError(
f"Reset value {orig_reset!r} is not within the signal shape {orig_shape!r}")

self.attrs = OrderedDict(() if attrs is None else attrs)

Expand Down
2 changes: 2 additions & 0 deletions docs/changes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@ Language changes
* Added: :func:`amaranth.utils.ceil_log2`, :func:`amaranth.utils.exact_log2`. (`RFC 17`_)
* Changed: ``m.Case()`` with no patterns is never active instead of always active. (`RFC 39`_)
* Changed: ``Value.matches()`` with no patterns is ``Const(0)`` instead of ``Const(1)``. (`RFC 39`_)
* Changed: ``Signal(range(stop), reset=stop)`` warning has been changed into a hard error and made to trigger on any out-of range value.
* Changed: ``Signal(range(0))`` is now valid without a warning.
* Deprecated: :func:`amaranth.utils.log2_int`. (`RFC 17`_)
* Removed: (deprecated in 0.4) :meth:`Const.normalize`. (`RFC 5`_)
* Removed: (deprecated in 0.4) :class:`ast.Sample`, :class:`ast.Past`, :class:`ast.Stable`, :class:`ast.Rose`, :class:`ast.Fell`.
Expand Down
20 changes: 15 additions & 5 deletions tests/test_hdl_ast.py
Original file line number Diff line number Diff line change
Expand Up @@ -1106,10 +1106,8 @@ def test_shape(self):
self.assertEqual(s8.shape(), signed(5))
s9 = Signal(range(-20, 16))
self.assertEqual(s9.shape(), signed(6))
with warnings.catch_warnings():
warnings.filterwarnings(action="ignore", category=SyntaxWarning)
s10 = Signal(range(0))
self.assertEqual(s10.shape(), unsigned(0))
s10 = Signal(range(0))
self.assertEqual(s10.shape(), unsigned(0))
s11 = Signal(range(1))
self.assertEqual(s11.shape(), unsigned(1))

Expand Down Expand Up @@ -1184,10 +1182,22 @@ def test_reset_wrong_too_wide(self):
Signal(signed(1), reset=-2)

def test_reset_wrong_fencepost(self):
with self.assertWarnsRegex(SyntaxWarning,
with self.assertRaisesRegex(SyntaxError,
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)
with self.assertRaisesRegex(SyntaxError,
r"^Reset value 0 equals the non-inclusive end of the signal shape "
r"range\(0, 0\); this is likely an off-by-one error$"):
Signal(range(0), reset=0)

def test_reset_wrong_range(self):
with self.assertRaisesRegex(SyntaxError,
r"^Reset value 11 is not within the signal shape range\(0, 10\)$"):
Signal(range(0, 10), reset=11)
with self.assertRaisesRegex(SyntaxError,
r"^Reset value 0 is not within the signal shape range\(1, 10\)$"):
Signal(range(1, 10), reset=0)

def test_attrs(self):
s1 = Signal()
Expand Down

0 comments on commit e9299cc

Please # to comment.