Repository: amaranth-lang/amaranth (python)

Issue:
Pysim should reject async generators
While converting some testbenches from generator-based to `async`-based for [pytest-amaranth-sim](https://github.com/cr1901/pytest-amaranth-sim), I accidentally forgot to remove a `yield` on one of them:

```python
@pytest.fixture(params=[pytest.param(MulTbArgs())], ids=mul_tb_id)
def mul_tb(mod, request):
    """The multiplier testbench proper. Receives arguments via request."""
    async def testbench(s):
        m = mod

        if m.registered:
            await s.tick()
        else:
            await s.delay(0.1)

        s.set(m.a, request.param.a_in)
        s.set(m.b, request.param.b_in)

        if m.registered:
            await s.tick()
        else:
            await s.delay(0.1)

        # assert s.get(m.o) == request.param.o_out
        assert (yield m.o) == request.param.o_out

    return testbench
```

Running a simulation with this testbench currently fails with an unhelpful ` TypeError: mul_tb.<locals>.testbench() missing 1 required positional argument: 's'`.

In reality, what happened is "forgetting to remove a `yield` turns the testbench into an async generator, and `Simulator.add_process` [treats it as](https://github.com/amaranth-lang/amaranth/blob/e2b47c1a9ea319f8165d00363768654f85d23eb1/amaranth/sim/core.py#L46-L54) as a plain-old-generator". Plain-old-generator testbenches don't expect any input arguments, but `async`-based testbenches _do_ expect a `sim` argument, which is missing which that testbench is run. [Agreed-upon fix](https://libera.irclog.whitequark.org/amaranth-lang/2024-05-21#1716267918-1716268945) is to not allow async generators, which in and of itself will give a better error message.

Pull request:
Check type of testbench when adding one to a simulator
Fixes #1363.

Patch:
diff --git a/.editorconfig b/.editorconfig
index 2d0ad6a..6d3184c 100644
--- a/.editorconfig
+++ b/.editorconfig
@@ -7,3 +7,6 @@ indent_size = 4
 max_line_length = 100
 trim_trailing_whitespace = true
 insert_final_newline = true
+
+[*.yaml]
+indent_size = 2
diff --git a/amaranth/sim/core.py b/amaranth/sim/core.py
index 668870d..4ea4993 100644
--- a/amaranth/sim/core.py
+++ b/amaranth/sim/core.py
@@ -35,14 +35,15 @@ class Simulator:
         self._engine   = engine(self._design)
         self._clocked  = set()
 
-    def _check_process(self, process):
-        if not (inspect.isgeneratorfunction(process) or inspect.iscoroutinefunction(process)):
-            raise TypeError("Cannot add a process {!r} because it is not a generator function"
-                            .format(process))
-        return process
+    def _check_function(self, function, *, kind):
+        if not (inspect.isgeneratorfunction(function) or inspect.iscoroutinefunction(function)):
+            raise TypeError(
+                f"Cannot add a {kind} {function!r} because it is not an async function or "
+                f"generator function")
+        return function
 
     def add_process(self, process):
-        process = self._check_process(process)
+        process = self._check_function(process, kind="process")
         if inspect.iscoroutinefunction(process):
             self._engine.add_async_process(self, process)
         else:
@@ -54,16 +55,17 @@ class Simulator:
             wrap_process = coro_wrapper(wrapper, testbench=False)
             self._engine.add_async_process(self, wrap_process)
 
-    def add_testbench(self, process, *, background=False):
-        if inspect.iscoroutinefunction(process):
-            self._engine.add_async_testbench(self, process, background=background)
+    def add_testbench(self, testbench, *, background=False):
+        testbench = self._check_function(testbench, kind="testbench")
+        if inspect.iscoroutinefunction(testbench):
+            self._engine.add_async_testbench(self, testbench, background=background)
         else:
-            process = coro_wrapper(process, testbench=True)
-            self._engine.add_async_testbench(self, process, background=background)
+            testbench = coro_wrapper(testbench, testbench=True)
+            self._engine.add_async_testbench(self, testbench, background=background)
 
     @deprecated("The `add_sync_process` method is deprecated per RFC 27. Use `add_process` or `add_testbench` instead.")
     def add_sync_process(self, process, *, domain="sync"):
-        process = self._check_process(process)
+        process = self._check_function(process, kind="process")
         def wrapper():
             # Only start a sync process after the first clock edge (or reset edge, if the domain
             # uses an asynchronous reset). This matches the behavior of synchronous FFs.
diff --git a/pyproject.toml b/pyproject.toml
index 6ae0274..9f8b75c 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -11,7 +11,7 @@ name = "amaranth"
 description = "Amaranth hardware definition language"
 readme = "README.md"
 authors = [{name = "Amaranth HDL contributors"}]
-license = {file = "LICENSE.txt"}
+license = { text = "BSD-2-clause" }
 
 requires-python = "~=3.8"
 dependencies = [


Test patch:
diff --git a/tests/test_sim.py b/tests/test_sim.py
index 678e557..8859a19 100644
--- a/tests/test_sim.py
+++ b/tests/test_sim.py
@@ -718,17 +718,35 @@ class SimulatorIntegrationTestCase(FHDLTestCase):
     def test_add_process_wrong(self):
         with self.assertSimulation(Module()) as sim:
             with self.assertRaisesRegex(TypeError,
-                    r"^Cannot add a process 1 because it is not a generator function$"):
+                    r"^Cannot add a process 1 because it is not an async function or "
+                    r"generator function$"):
                 sim.add_process(1)
 
     def test_add_process_wrong_generator(self):
         with self.assertSimulation(Module()) as sim:
             with self.assertRaisesRegex(TypeError,
-                    r"^Cannot add a process <.+?> because it is not a generator function$"):
+                    r"^Cannot add a process <.+?> because it is not an async function or "
+                    r"generator function$"):
                 def process():
                     yield Delay()
                 sim.add_process(process())
 
+    def test_add_testbench_wrong(self):
+        with self.assertSimulation(Module()) as sim:
+            with self.assertRaisesRegex(TypeError,
+                    r"^Cannot add a testbench 1 because it is not an async function or "
+                    r"generator function$"):
+                sim.add_testbench(1)
+
+    def test_add_testbench_wrong_generator(self):
+        with self.assertSimulation(Module()) as sim:
+            with self.assertRaisesRegex(TypeError,
+                    r"^Cannot add a testbench <.+?> because it is not an async function or "
+                    r"generator function$"):
+                def testbench():
+                    yield Delay()
+                sim.add_testbench(testbench())
+
     def test_add_clock_wrong_twice(self):
         m = Module()
         s = Signal()
@@ -1935,3 +1953,12 @@ class SimulatorRegressionTestCase(FHDLTestCase):
 
         self.assertTrue(reached_tb)
         self.assertTrue(reached_proc)
+
+    def test_bug_1363(self):
+        sim = Simulator(Module())
+        with self.assertRaisesRegex(TypeError,
+                r"^Cannot add a testbench <.+?> because it is not an async function or "
+                r"generator function$"):
+            async def testbench():
+                yield Delay()
+            sim.add_testbench(testbench())