Repository: amaranth-lang/amaranth (python)

Issue:
Unused cases in a switch do not trigger warnings
Consider those two elaboratables:

```python
class Impl1(Elaboratable):
    def __init__(self):
        self.sel = Signal()
        self.A = Signal()
        self.B = Signal()

    def elaborate(self, platform):
        m = Module()

        with m.Switch(self.sel):
            with m.Case():
                m.d.comb += [
                    self.A.eq(1),
                    self.B.eq(1),
                ]
            with m.Case(0):
                m.d.comb += [
                    self.A.eq(1),
                    self.B.eq(0),
                ]
            with m.Case(1):
                m.d.comb += [
                    self.A.eq(0),
                    self.B.eq(0)
                ]

        return m

class Impl2(Elaboratable):
    def __init__(self):
        self.sel = Signal()
        self.A = Signal()
        self.B = Signal()

    def elaborate(self, platform):
        m = Module()

        with m.Switch(self.sel):
            with m.Case(0):
                m.d.comb += [
                    self.A.eq(1),
                    self.B.eq(0),
                ]
            with m.Case(1):
                m.d.comb += [
                    self.A.eq(0),
                    self.B.eq(0)
                ]
            with m.Case():
                m.d.comb += [
                    self.A.eq(1),
                    self.B.eq(1),
                ]

        return m
```

Because the default/wildcard case was declared first in Impl1, it prevents Case(0) and Case(1) from being matched. This is a bit misleading for newbies who would expect cases to be position agnostic (and therefore Impl1 having equivalent behaviour as Impl2).

As discussed on IRC, a sensible way of dealing with that issue would be to trigger warnings whenever some cases are unreachable. We still have to figure out how we can silence those warnings when they are undesired (think code generation).

Pull request:
Warn if a case is defined after a default case
Fixes #404.
Closes #435.

Patch:
diff --git a/amaranth/hdl/dsl.py b/amaranth/hdl/dsl.py
index 2da0dac..5cbfd52 100644
--- a/amaranth/hdl/dsl.py
+++ b/amaranth/hdl/dsl.py
@@ -308,6 +308,9 @@ class Module(_ModuleBuilderRoot, Elaboratable):
         src_loc = tracer.get_src_loc(src_loc_at=1)
         switch_data = self._get_ctrl("Switch")
         new_patterns = ()
+        if () in switch_data["cases"]:
+            warnings.warn("A case defined after the default case will never be active",
+                          SyntaxWarning, stacklevel=3)
         # This code should accept exactly the same patterns as `v.matches(...)`.
         for pattern in patterns:
             if isinstance(pattern, str) and any(bit not in "01- \t" for bit in pattern):
@@ -357,6 +360,9 @@ class Module(_ModuleBuilderRoot, Elaboratable):
         self._check_context("Default", context="Switch")
         src_loc = tracer.get_src_loc(src_loc_at=1)
         switch_data = self._get_ctrl("Switch")
+        if () in switch_data["cases"]:
+            warnings.warn("A case defined after the default case will never be active",
+                          SyntaxWarning, stacklevel=3)
         try:
             _outer_case, self._statements = self._statements, []
             self._ctrl_context = None


Test patch:
diff --git a/tests/test_hdl_dsl.py b/tests/test_hdl_dsl.py
index dddae25..06611c3 100644
--- a/tests/test_hdl_dsl.py
+++ b/tests/test_hdl_dsl.py
@@ -497,6 +497,26 @@ class DSLTestCase(FHDLTestCase):
             with m.Case():
                 pass
 
+    def test_Case_after_Default_wrong(self):
+        m = Module()
+        with m.Switch(self.w1):
+            with m.Default():
+                pass
+            with self.assertWarnsRegex(SyntaxWarning,
+                    r"^A case defined after the default case will never be active$"):
+                with m.Case():
+                    pass
+
+    def test_Default_after_Default_wrong(self):
+        m = Module()
+        with m.Switch(self.w1):
+            with m.Default():
+                pass
+            with self.assertWarnsRegex(SyntaxWarning,
+                    r"^A case defined after the default case will never be active$"):
+                with m.Default():
+                    pass
+
     def test_If_inside_Switch_wrong(self):
         m = Module()
         with m.Switch(self.s1):