Repository: amaranth-lang/amaranth
Pull request #1488 (2024-08-24 20:55:03)

sim: fix infinite recursion on non-`Signal` values. [0.5 backport]


Patch:
diff --git a/amaranth/lib/data.py b/amaranth/lib/data.py
index b653882..46f0a1b 100644
--- a/amaranth/lib/data.py
+++ b/amaranth/lib/data.py
@@ -1277,7 +1277,7 @@ class Struct(View, metaclass=_AggregateMeta):
 
         >>> IEEE754Single.as_shape()
         StructLayout({'fraction': 23, 'exponent': 8, 'sign': 1})
-        >>> Signal(IEEE754Single).as_value().width
+        >>> Signal(IEEE754Single).as_value().shape().width
         32
 
     Instances of this class can be used where :ref:`values <lang-values>` are expected:
diff --git a/amaranth/sim/pysim.py b/amaranth/sim/pysim.py
index d89ddca..2d9562d 100644
--- a/amaranth/sim/pysim.py
+++ b/amaranth/sim/pysim.py
@@ -269,24 +269,22 @@ class _VCDWriter:
             self.gtkw_save.treeopen("top")
 
             def traverse_traces(traces):
-                if isinstance(traces, Signal):
-                    for name in self.gtkw_signal_names[traces]:
-                        self.gtkw_save.trace(name)
-                elif isinstance(traces, data.View):
+                if isinstance(traces, data.View):
                     with self.gtkw_save.group("view"):
-                        trace = Value.cast(traces)
+                        traverse_traces(Value.cast(traces))
+                elif isinstance(traces, ValueLike):
+                    trace = Value.cast(traces)
+                    if isinstance(traces, MemoryData._Row):
+                        for name in self.gtkw_memory_names[traces._memory][traces._index]:
+                            self.gtkw_save.trace(name)
+                    else:
                         for trace_signal in trace._rhs_signals():
                             for name in self.gtkw_signal_names[trace_signal]:
                                 self.gtkw_save.trace(name)
-                elif isinstance(traces, ValueLike):
-                    traverse_traces(Value.cast(traces))
                 elif isinstance(traces, MemoryData):
                     for row_names in self.gtkw_memory_names[traces]:
                         for name in row_names:
                             self.gtkw_save.trace(name)
-                elif isinstance(traces, MemoryData._Row):
-                    for name in self.gtkw_memory_names[traces._memory][traces._index]:
-                        self.gtkw_save.trace(name)
                 elif hasattr(traces, "signature") and isinstance(traces.signature, wiring.Signature):
                     with self.gtkw_save.group("interface"):
                         for _, _, member in traces.signature.flatten(traces):
diff --git a/docs/guide.rst b/docs/guide.rst
index 71838a9..6268186 100644
--- a/docs/guide.rst
+++ b/docs/guide.rst
@@ -170,6 +170,14 @@ Specifying a shape with a range is convenient for counters, indexes, and all oth
 
    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:
 
+   .. doctest::
+      :hide:
+
+      >>> import warnings
+      >>> _warning_filters_backup = warnings.catch_warnings()
+      >>> _warning_filters_backup.__enter__() # have to do this horrific hack to make it work with `PYTHONWARNINGS=error` :(
+      >>> warnings.simplefilter("default", amaranth.hdl._ast.SyntaxWarning)
+
    .. doctest::
 
       >>> fencepost = C(256, range(256))
@@ -180,6 +188,11 @@ Specifying a shape with a range is convenient for counters, indexes, and all oth
       >>> fencepost.value
       0
 
+   .. doctest::
+      :hide:
+
+      >>> _warning_filters_backup.__exit__()
+
    Amaranth detects uses of :class:`Const` and :class:`Signal` that invoke such an off-by-one error, and emits a diagnostic message.
 
 .. note::
@@ -670,6 +683,14 @@ Python expression Amaranth expression (boolean operands)
 
    When applied to Amaranth boolean values, the ``~`` operator computes negation, and when applied to Python boolean values, the ``not`` operator also computes negation. However, the ``~`` operator applied to Python boolean values produces an unexpected result:
 
+   .. doctest::
+      :hide:
+
+      >>> import warnings
+      >>> _warning_filters_backup = warnings.catch_warnings()
+      >>> _warning_filters_backup.__enter__() # have to do this horrific hack to make it work with `PYTHONWARNINGS=error` :(
+      >>> warnings.simplefilter("ignore", DeprecationWarning)
+
    .. doctest::
 
       >>> ~False
@@ -688,6 +709,11 @@ Python expression Amaranth expression (boolean operands)
       >>> ~use_stb | stb # WRONG! MSB of 2-bit wide OR expression is always 1
       (| (const 2'sd-2) (sig stb))
 
+   .. doctest::
+      :hide:
+
+      >>> _warning_filters_backup.__exit__()
+
    Amaranth automatically detects some cases of misuse of ``~`` and emits a detailed diagnostic message.
 
    .. TODO: this isn't quite reliable, #380
diff --git a/pyproject.toml b/pyproject.toml
index 1d74683..2f0c4db 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -75,8 +75,8 @@ test = [
 ]
 docs = [
   "sphinx~=7.1",
-  "sphinxcontrib-platformpicker~=1.3",
-  "sphinxcontrib-yowasp-wavedrom==1.7", # exact version to avoid changes in rendering
+  "sphinxcontrib-platformpicker~=1.4",
+  "sphinxcontrib-yowasp-wavedrom==1.8", # exact version to avoid changes in rendering
   "sphinx-rtd-theme~=2.0",
   "sphinx-autobuild",
 ]


Test patch:
diff --git a/tests/test_sim.py b/tests/test_sim.py
index a75cd50..da4f0a4 100644
--- a/tests/test_sim.py
+++ b/tests/test_sim.py
@@ -1479,6 +1479,10 @@ class SimulatorTracesTestCase(FHDLTestCase):
         a = Signal()
         self.assertDef(a, [a])
 
+    def test_expr(self):
+        a = Signal()
+        self.assertDef(a + 3, [a])
+
     def test_list(self):
         a = Signal()
         self.assertDef([a], [a])