From 80343d1c4ca3e989b089bf7c93bdb8fb6a1a8749 Mon Sep 17 00:00:00 2001
From: Catherine <whitequark@whitequark.org>
Date: Mon, 13 Mar 2023 20:27:13 +0000
Subject: [PATCH] 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.
---
 amaranth/hdl/ast.py   | 15 +++++++++++++++
 docs/lang.rst         |  4 ++--
 tests/test_hdl_ast.py | 18 ++++++++++++++++--
 3 files changed, 33 insertions(+), 4 deletions(-)

diff --git a/amaranth/hdl/ast.py b/amaranth/hdl/ast.py
index 8f65f1b..eb29620 100644
--- a/amaranth/hdl/ast.py
+++ b/amaranth/hdl/ast.py
@@ -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):
diff --git a/docs/lang.rst b/docs/lang.rst
index 25d193e..721dff8 100644
--- a/docs/lang.rst
+++ b/docs/lang.rst
@@ -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:
diff --git a/tests/test_hdl_ast.py b/tests/test_hdl_ast.py
index d360ef5..3d18091 100644
--- a/tests/test_hdl_ast.py
+++ b/tests/test_hdl_ast.py
@@ -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, {})