From: whitequark <whitequark@whitequark.org>
Date: Thu, 6 Feb 2020 17:47:46 +0000 (+0000)
Subject: hdl.dsl: make referencing undefined FSM states an error.
X-Git-Url: https://git.libre-soc.org/?a=commitdiff_plain;h=fafe21d9ac339321cf90642270f8cfdf126a1708;p=nmigen.git

hdl.dsl: make referencing undefined FSM states an error.

Before this commit, doing something like:

    with m.FSM():
        with m.State("FOO"):
            m.next = "bAR"
        with m.State("BAR"):
            m.next = "FOO"

would silently create an empty state `bAR` and get stuck in it until
the module is reset. This was done intentionally (in Migen, this code
would in fact miscompile), but in retrospect was clearly a bad idea;
it turns typos into bugs, while in the rare case that branching to
a completely empty state is desired, it is trivial to define one.

Fixes #315.
---

diff --git a/nmigen/hdl/dsl.py b/nmigen/hdl/dsl.py
index 96f1923..85ae472 100644
--- a/nmigen/hdl/dsl.py
+++ b/nmigen/hdl/dsl.py
@@ -375,6 +375,10 @@ class Module(_ModuleBuilderRoot, Elaboratable):
             self._ctrl_context = "FSM"
             self.domain._depth += 1
             yield fsm
+            for state_name in fsm_data["encoding"]:
+                if state_name not in fsm_data["states"]:
+                    raise NameError("FSM state '{}' is referenced but not defined"
+                                    .format(state_name))
         finally:
             self.domain._depth -= 1
             self._ctrl_context = None
@@ -386,7 +390,7 @@ class Module(_ModuleBuilderRoot, Elaboratable):
         src_loc = tracer.get_src_loc(src_loc_at=1)
         fsm_data = self._get_ctrl("FSM")
         if name in fsm_data["states"]:
-            raise SyntaxError("FSM state '{}' is already defined".format(name))
+            raise NameError("FSM state '{}' is already defined".format(name))
         if name not in fsm_data["encoding"]:
             fsm_data["encoding"][name] = len(fsm_data["encoding"])
         try:
diff --git a/nmigen/test/test_hdl_dsl.py b/nmigen/test/test_hdl_dsl.py
index 9b3a91d..1c5686b 100644
--- a/nmigen/test/test_hdl_dsl.py
+++ b/nmigen/test/test_hdl_dsl.py
@@ -582,12 +582,19 @@ class DSLTestCase(FHDLTestCase):
             with m.FSM(domain="comb"):
                 pass
 
+    def test_FSM_wrong_undefined(self):
+        m = Module()
+        with self.assertRaises(NameError,
+                msg="FSM state 'FOO' is referenced but not defined"):
+            with m.FSM() as fsm:
+                fsm.ongoing("FOO")
+
     def test_FSM_wrong_redefined(self):
         m = Module()
         with m.FSM():
             with m.State("FOO"):
                 pass
-            with self.assertRaises(SyntaxError,
+            with self.assertRaises(NameError,
                     msg="FSM state 'FOO' is already defined"):
                 with m.State("FOO"):
                     pass