fhdl.dsl: add tests for d.comb/d.sync, If/Elif/Else.

nmigen/fhdl/ast.py

@@ -606,14 +606,19 @@ class ResetSignal(Value):
         return "(reset {})".format(self.domain)
 
 
+class _StatementList(list):
+    def __repr__(self):
+        return "({})".format(" ".join(map(repr, self)))
+
+
 class Statement:
     @staticmethod
     def wrap(obj):
         if isinstance(obj, Iterable):
-            return sum((Statement.wrap(e) for e in obj), [])
+            return _StatementList(sum((Statement.wrap(e) for e in obj), []))
         else:
             if isinstance(obj, Statement):
-                return [obj]
+                return _StatementList([obj])
             else:
                 raise TypeError("Object {!r} is not a Migen statement".format(obj))
 

nmigen/fhdl/dsl.py

@@ -1,11 +1,16 @@
 from collections import OrderedDict
+from contextlib import contextmanager
 
 from .ast import *
 from .ir import *
 from .xfrm import *
 
 
-__all__ = ["Module"]
+__all__ = ["Module", "SyntaxError"]
+
+
+class SyntaxError(Exception):
+    pass
 
 
 class _ModuleBuilderProxy:
@@ -36,9 +41,11 @@ class _ModuleBuilderDomains(_ModuleBuilderProxy):
         return self.__getattr__(name)
 
     def __setattr__(self, name, value):
-        if not isinstance(value, _ModuleBuilderDomain):
+        if name == "_depth":
-            raise AttributeError("Cannot assign d.{} attribute - use += instead"
+            object.__setattr__(self, name, value)
-                                 .format(name))
+        elif not isinstance(value, _ModuleBuilderDomain):
+            raise AttributeError("Cannot assign 'd.{}' attribute; did you mean 'd.{} +='?"
+                                 .format(name, name))
 
     def __setitem__(self, name, value):
         return self.__setattr__(name, value)
@@ -57,59 +64,6 @@ class _ModuleBuilderRoot:
                              .format(type(self).__name__, name))
 
 
-class _ModuleBuilderIf(_ModuleBuilderRoot):
-    def __init__(self, builder, depth, cond):
-        super().__init__(builder, depth)
-        self._cond = cond
-
-    def __enter__(self):
-        self._builder._flush()
-        self._builder._stmt_if_cond.append(self._cond)
-        self._outer_case = self._builder._statements
-        self._builder._statements = []
-        return self
-
-    def __exit__(self, *args):
-        self._builder._stmt_if_bodies.append(self._builder._statements)
-        self._builder._statements = self._outer_case
-
-
-class _ModuleBuilderElif(_ModuleBuilderRoot):
-    def __init__(self, builder, depth, cond):
-        super().__init__(builder, depth)
-        self._cond = cond
-
-    def __enter__(self):
-        if not self._builder._stmt_if_cond:
-            raise ValueError("Elif without preceding If")
-        self._builder._stmt_if_cond.append(self._cond)
-        self._outer_case = self._builder._statements
-        self._builder._statements = []
-        return self
-
-    def __exit__(self, *args):
-        self._builder._stmt_if_bodies.append(self._builder._statements)
-        self._builder._statements = self._outer_case
-
-
-class _ModuleBuilderElse(_ModuleBuilderRoot):
-    def __init__(self, builder, depth):
-        super().__init__(builder, depth)
-
-    def __enter__(self):
-        if not self._builder._stmt_if_cond:
-            raise ValueError("Else without preceding If/Elif")
-        self._builder._stmt_if_cond.append(1)
-        self._outer_case = self._builder._statements
-        self._builder._statements = []
-        return self
-
-    def __exit__(self, *args):
-        self._builder._stmt_if_bodies.append(self._builder._statements)
-        self._builder._statements = self._outer_case
-        self._builder._flush()
-
-
 class _ModuleBuilderCase(_ModuleBuilderRoot):
     def __init__(self, builder, depth, test, value):
         super().__init__(builder, depth)
@@ -120,8 +74,8 @@ class _ModuleBuilderCase(_ModuleBuilderRoot):
         if self._value is None:
             self._value = "-" * len(self._test)
         if isinstance(self._value, str) and len(self._test) != len(self._value):
-            raise ValueError("Case value {} must have the same width as test {}"
+            raise SyntaxError("Case value {} must have the same width as test {}"
-                             .format(self._value, self._test))
+                              .format(self._value, self._test))
         if self._builder._stmt_switch_test != ValueKey(self._test):
             self._builder._flush()
             self._builder._stmt_switch_test = ValueKey(self._test)
@@ -154,21 +108,56 @@ class Module(_ModuleBuilderRoot):
 
         self._submodules        = []
         self._driving           = ValueDict()
-        self._statements        = []
+        self._statements        = Statement.wrap([])
         self._stmt_depth        = 0
         self._stmt_if_cond      = []
         self._stmt_if_bodies    = []
         self._stmt_switch_test  = None
         self._stmt_switch_cases = OrderedDict()
 
+    @contextmanager
     def If(self, cond):
-        return _ModuleBuilderIf(self, self._stmt_depth + 1, cond)
+        self._flush()
+        try:
+            _outer_case = self._statements
+            self._statements = []
+            self.domain._depth += 1
+            yield
+            self._stmt_if_cond.append(cond)
+            self._stmt_if_bodies.append(self._statements)
+        finally:
+            self.domain._depth -= 1
+            self._statements = _outer_case
 
+    @contextmanager
     def Elif(self, cond):
-        return _ModuleBuilderElif(self, self._stmt_depth + 1, cond)
+        if not self._stmt_if_cond:
+            raise SyntaxError("Elif without preceding If")
+        try:
+            _outer_case = self._statements
+            self._statements = []
+            self.domain._depth += 1
+            yield
+            self._stmt_if_cond.append(cond)
+            self._stmt_if_bodies.append(self._statements)
+        finally:
+            self.domain._depth -= 1
+            self._statements = _outer_case
 
+    @contextmanager
     def Else(self):
-        return _ModuleBuilderElse(self, self._stmt_depth + 1)
+        if not self._stmt_if_cond:
+            raise SyntaxError("Else without preceding If/Elif")
+        try:
+            _outer_case = self._statements
+            self._statements = []
+            self.domain._depth += 1
+            yield
+            self._stmt_if_bodies.append(self._statements)
+        finally:
+            self.domain._depth -= 1
+            self._statements = _outer_case
+        self._flush()
 
     def Case(self, test, value=None):
         return _ModuleBuilderCase(self, self._stmt_depth + 1, test, value)
@@ -176,13 +165,17 @@ class Module(_ModuleBuilderRoot):
     def _flush(self):
         if self._stmt_if_cond:
             tests, cases = [], OrderedDict()
-            for if_cond, if_case in zip(self._stmt_if_cond, self._stmt_if_bodies):
+            for if_cond, if_case in zip(self._stmt_if_cond + [None], self._stmt_if_bodies):
-                if_cond = Value.wrap(if_cond)
+                if if_cond is not None:
-                if len(if_cond) != 1:
+                    if_cond = Value.wrap(if_cond)
-                    if_cond = if_cond.bool()
+                    if len(if_cond) != 1:
-                tests.append(if_cond)
+                        if_cond = if_cond.bool()
+                    tests.append(if_cond)
 
-                match = ("1" + "-" * (len(tests) - 1)).rjust(len(self._stmt_if_cond), "-")
+                if if_cond is not None:
+                    match = ("1" + "-" * (len(tests) - 1)).rjust(len(self._stmt_if_cond), "-")
+                else:
+                    match = "-" * len(tests)
                 cases[match] = if_case
             self._statements.append(Switch(Cat(tests), cases))
 
@@ -207,24 +200,25 @@ class Module(_ModuleBuilderRoot):
 
         for assign in Statement.wrap(assigns):
             if not compat_mode and not isinstance(assign, Assign):
-                raise TypeError("Only assignments can be appended to {}"
+                raise SyntaxError(
-                                .format(cd_human_name(cd_name)))
+                    "Only assignments may be appended to d.{}"
+                    .format(cd_human_name(cd_name)))
 
             for signal in assign._lhs_signals():
                 if signal not in self._driving:
                     self._driving[signal] = cd_name
                 elif self._driving[signal] != cd_name:
                     cd_curr = self._driving[signal]
-                    raise ValueError("Driver-driver conflict: trying to drive {!r} from d.{}, but "
+                    raise SyntaxError(
-                                     "it is already driven from d.{}"
+                        "Driver-driver conflict: trying to drive {!r} from d.{}, but it is "
-                                     .format(signal, cd_human_name(cd_name),
+                        "already driven from d.{}"
-                                             cd_human_name(cd_curr)))
+                        .format(signal, cd_human_name(cd_name), cd_human_name(cd_curr)))
 
             self._statements.append(assign)
 
     def _add_submodule(self, submodule, name=None):
         if not hasattr(submodule, "get_fragment"):
-            raise TypeError("Trying to add {!r}, which does not have .get_fragment(), as "
+            raise TypeError("Trying to add {!r}, which does not implement .get_fragment(), as "
                             "a submodule".format(submodule))
         self._submodules.append((submodule, name))
 
@@ -236,8 +230,7 @@ class Module(_ModuleBuilderRoot):
             fragment.add_subfragment(submodule.get_fragment(platform), name)
         fragment.add_statements(self._statements)
         for signal, cd_name in self._driving.items():
-            for lhs_signal in signal._lhs_signals():
+            fragment.drive(signal, cd_name)
-                fragment.drive(lhs_signal, cd_name)
         return fragment
 
     get_fragment = lower

nmigen/test/test_fhdl_dsl.py

@@ -0,0 +1,200 @@
+import re
+import unittest
+from contextlib import contextmanager
+
+from nmigen.fhdl.ast import *
+from nmigen.fhdl.dsl import *
+
+
+class DSLTestCase(unittest.TestCase):
+    def setUp(self):
+        self.s1 = Signal()
+        self.s2 = Signal()
+        self.s3 = Signal()
+        self.s4 = Signal()
+        self.c1 = Signal()
+        self.c2 = Signal()
+        self.c3 = Signal()
+        self.w1 = Signal(4)
+
+    @contextmanager
+    def assertRaises(self, exception, msg=None):
+        with super().assertRaises(exception) as cm:
+            yield
+        if msg:
+            # WTF? unittest.assertRaises is completely broken.
+            self.assertEqual(str(cm.exception), msg)
+
+    def assertRepr(self, obj, repr_str):
+        repr_str = re.sub(r"\s+",   " ",  repr_str)
+        repr_str = re.sub(r"\( (?=\()", "(", repr_str)
+        repr_str = re.sub(r"\) (?=\))", ")", repr_str)
+        self.assertEqual(repr(obj), repr_str.strip())
+
+    def test_d_comb(self):
+        m = Module()
+        m.d.comb += self.c1.eq(1)
+        m._flush()
+        self.assertEqual(m._driving[self.c1], None)
+        self.assertRepr(m._statements, """(
+            (eq (sig c1) (const 1'd1))
+        )""")
+
+    def test_d_sync(self):
+        m = Module()
+        m.d.sync += self.c1.eq(1)
+        m._flush()
+        self.assertEqual(m._driving[self.c1], "sync")
+        self.assertRepr(m._statements, """(
+            (eq (sig c1) (const 1'd1))
+        )""")
+
+    def test_d_pix(self):
+        m = Module()
+        m.d.pix += self.c1.eq(1)
+        m._flush()
+        self.assertEqual(m._driving[self.c1], "pix")
+        self.assertRepr(m._statements, """(
+            (eq (sig c1) (const 1'd1))
+        )""")
+
+    def test_d_index(self):
+        m = Module()
+        m.d["pix"] += self.c1.eq(1)
+        m._flush()
+        self.assertEqual(m._driving[self.c1], "pix")
+        self.assertRepr(m._statements, """(
+            (eq (sig c1) (const 1'd1))
+        )""")
+
+    def test_d_no_conflict(self):
+        m = Module()
+        m.d.comb += self.w1[0].eq(1)
+        m.d.comb += self.w1[1].eq(1)
+
+    def test_d_conflict(self):
+        m = Module()
+        with self.assertRaises(SyntaxError,
+                msg="Driver-driver conflict: trying to drive (sig c1) from d.sync, but it "
+                    "is already driven from d.comb"):
+            m.d.comb += self.c1.eq(1)
+            m.d.sync += self.c1.eq(1)
+
+    def test_d_wrong(self):
+        m = Module()
+        with self.assertRaises(AttributeError,
+                msg="Cannot assign 'd.pix' attribute; did you mean 'd.pix +='?"):
+            m.d.pix = None
+
+    def test_d_asgn_wrong(self):
+        m = Module()
+        with self.assertRaises(SyntaxError,
+                msg="Only assignments may be appended to d.sync"):
+            m.d.sync += Switch(self.s1, {})
+
+    def test_comb_wrong(self):
+        m = Module()
+        with self.assertRaises(AttributeError,
+                msg="'Module' object has no attribute 'comb'; did you mean 'd.comb'?"):
+            m.comb += self.c1.eq(1)
+
+    def test_sync_wrong(self):
+        m = Module()
+        with self.assertRaises(AttributeError,
+                msg="'Module' object has no attribute 'sync'; did you mean 'd.sync'?"):
+            m.sync += self.c1.eq(1)
+
+    def test_attr_wrong(self):
+        m = Module()
+        with self.assertRaises(AttributeError,
+                msg="'Module' object has no attribute 'nonexistentattr'"):
+            m.nonexistentattr
+
+    def test_If(self):
+        m = Module()
+        with m.If(self.s1):
+            m.d.comb += self.c1.eq(1)
+        m._flush()
+        self.assertRepr(m._statements, """
+        (
+            (switch (cat (sig s1))
+                (case 1 (eq (sig c1) (const 1'd1)))
+            )
+        )
+        """)
+
+    def test_If_Elif(self):
+        m = Module()
+        with m.If(self.s1):
+            m.d.comb += self.c1.eq(1)
+        with m.Elif(self.s2):
+            m.d.sync += self.c2.eq(0)
+        m._flush()
+        self.assertRepr(m._statements, """
+        (
+            (switch (cat (sig s1) (sig s2))
+                (case -1 (eq (sig c1) (const 1'd1)))
+                (case 1- (eq (sig c2) (const 0'd0)))
+            )
+        )
+        """)
+
+    def test_If_Elif_Else(self):
+        m = Module()
+        with m.If(self.s1):
+            m.d.comb += self.c1.eq(1)
+        with m.Elif(self.s2):
+            m.d.sync += self.c2.eq(0)
+        with m.Else():
+            m.d.comb += self.c3.eq(1)
+        m._flush()
+        self.assertRepr(m._statements, """
+        (
+            (switch (cat (sig s1) (sig s2))
+                (case -1 (eq (sig c1) (const 1'd1)))
+                (case 1- (eq (sig c2) (const 0'd0)))
+                (case -- (eq (sig c3) (const 1'd1)))
+            )
+        )
+        """)
+
+    def test_Elif_wrong(self):
+        m = Module()
+        with self.assertRaises(SyntaxError,
+                msg="Elif without preceding If"):
+            with m.Elif(self.s2):
+                pass
+
+    def test_Else_wrong(self):
+        m = Module()
+        with self.assertRaises(SyntaxError,
+                msg="Else without preceding If/Elif"):
+            with m.Else():
+                pass
+
+    def test_If_wide(self):
+        m = Module()
+        with m.If(self.w1):
+            m.d.comb += self.c1.eq(1)
+        m._flush()
+        self.assertRepr(m._statements, """
+        (
+            (switch (cat (b (sig w1)))
+                (case 1 (eq (sig c1) (const 1'd1)))
+            )
+        )
+        """)
+
+    def test_auto_flush(self):
+        m = Module()
+        with m.If(self.w1):
+            m.d.comb += self.c1.eq(1)
+        m.d.comb += self.c2.eq(1)
+        self.assertRepr(m._statements, """
+        (
+            (switch (cat (b (sig w1)))
+                (case 1 (eq (sig c1) (const 1'd1)))
+            )
+            (eq (sig c2) (const 1'd1))
+        )
+        """)