hdl.dsl: warn on suspicious statements like `m.If(~True):`.

This pattern usually produces an extremely hard to notice bug that
will usually break a design when it is triggered, but will also be
hidden unless the pathological value of a boolean switch is used.

Fixes #159.

diff --git a/nmigen/hdl/dsl.py b/nmigen/hdl/dsl.py
index 361fbebfa7f4d1e20ce228e1353a02deab18122e..e090b0967fc16e9ba299c2a8fcac55ae5b3bad2b 100644 (file)
--- a/nmigen/hdl/dsl.py
+++ b/nmigen/hdl/dsl.py
@@ -165,9 +165,21 @@ class Module(_ModuleBuilderRoot, Elaboratable):
         self._ctrl_stack.append((name, data))
         return data
 
+    def _check_signed_cond(self, cond):
+        cond = Value.wrap(cond)
+        bits, sign = cond.shape()
+        if sign:
+            warnings.warn("Signed values in If/Elif conditions usually result from inverting "
+                          "Python booleans with ~, which leads to unexpected results: ~True is "
+                          "-2, which is truthful. "
+                          "(Silence this warning with `m.If(x)` → `m.If(x.bool())`.)",
+                          SyntaxWarning, stacklevel=4)
+        return cond
+
     @contextmanager
     def If(self, cond):
         self._check_context("If", context=None)
+        cond = self._check_signed_cond(cond)
         src_loc = tracer.get_src_loc(src_loc_at=1)
         if_data = self._set_ctrl("If", {
             "tests":    [],
@@ -190,6 +202,7 @@ class Module(_ModuleBuilderRoot, Elaboratable):
     @contextmanager
     def Elif(self, cond):
         self._check_context("Elif", context=None)
+        cond = self._check_signed_cond(cond)
         src_loc = tracer.get_src_loc(src_loc_at=1)
         if_data = self._get_ctrl("If")
         if if_data is None:

diff --git a/nmigen/test/test_hdl_dsl.py b/nmigen/test/test_hdl_dsl.py
index 502e9a88d83796223e7d229dfcd2b5395eb36e38..0ff7fefe932484649e4346558d8004d6686b9f48 100644 (file)
--- a/nmigen/test/test_hdl_dsl.py
+++ b/nmigen/test/test_hdl_dsl.py
@@ -268,6 +268,26 @@ class DSLTestCase(FHDLTestCase):
         )
         """)
 
+    def test_If_signed_suspicious(self):
+        m = Module()
+        with self.assertWarns(SyntaxWarning,
+                msg="Signed values in If/Elif conditions usually result from inverting Python "
+                    "booleans with ~, which leads to unexpected results: ~True is -2, which is "
+                    "truthful. (Silence this warning with `m.If(x)` → `m.If(x.bool())`.)"):
+            with m.If(~True):
+                pass
+
+    def test_Elif_signed_suspicious(self):
+        m = Module()
+        with m.If(0):
+            pass
+        with self.assertWarns(SyntaxWarning,
+                msg="Signed values in If/Elif conditions usually result from inverting Python "
+                    "booleans with ~, which leads to unexpected results: ~True is -2, which is "
+                    "truthful. (Silence this warning with `m.If(x)` → `m.If(x.bool())`.)"):
+            with m.Elif(~True):
+                pass
+
     def test_Switch(self):
         m = Module()
         with m.Switch(self.w1):