Skip to content
Permalink

Comparing changes

Choose two branches to see what’s changed or to start a new pull request. If you need to, you can also or learn more about diff comparisons.

Open a pull request

Create a new pull request by comparing changes across two branches. If you need to, you can also . Learn more about diff comparisons here.
base repository: m-labs/nmigen
Failed to load repositories. Confirm that selected base ref is valid, then try again.
Loading
base: ee03eab52fbc
Choose a base ref
...
head repository: m-labs/nmigen
Failed to load repositories. Confirm that selected head ref is valid, then try again.
Loading
compare: ace2b5ff0a0b
Choose a head ref
  • 2 commits
  • 3 files changed
  • 1 contributor

Commits on Aug 3, 2019

  1. Improve test added in 29fee01 to not leak warnings.

    whitequark committed Aug 3, 2019
    Copy the full SHA
    ab5426c View commit details
  2. 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.
    whitequark committed Aug 3, 2019
    Copy the full SHA
    ace2b5f View commit details
Showing with 38 additions and 3 deletions.
  1. +13 −0 nmigen/hdl/dsl.py
  2. +20 −0 nmigen/test/test_hdl_dsl.py
  3. +5 −3 nmigen/test/test_hdl_ir.py
13 changes: 13 additions & 0 deletions nmigen/hdl/dsl.py
Original file line number Diff line number Diff line change
@@ -165,9 +165,21 @@ def _set_ctrl(self, name, data):
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 @@ def If(self, cond):
@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:
20 changes: 20 additions & 0 deletions nmigen/test/test_hdl_dsl.py
Original file line number Diff line number Diff line change
@@ -268,6 +268,26 @@ def test_If_wide(self):
)
""")

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):
8 changes: 5 additions & 3 deletions nmigen/test/test_hdl_ir.py
Original file line number Diff line number Diff line change
@@ -18,9 +18,11 @@ def test_get_wrong(self):
msg="Object 'None' cannot be elaborated"):
Fragment.get(None, platform=None)

with self.assertRaises(AttributeError,
msg="Object 'None' cannot be elaborated"):
Fragment.get(BadElaboratable(), platform=None)
with self.assertWarns(UserWarning,
msg=".elaborate() returned None; missing return statement?"):
with self.assertRaises(AttributeError,
msg="Object 'None' cannot be elaborated"):
Fragment.get(BadElaboratable(), platform=None)


class FragmentGeneratedTestCase(FHDLTestCase):