From: whitequark <whitequark@whitequark.org>
Date: Sun, 21 Apr 2019 08:52:57 +0000 (+0000)
Subject: hdl.ir: detect elaboratables that are created but not used.
X-Git-Tag: working~6
X-Git-Url: https://git.libre-soc.org/?a=commitdiff_plain;h=44711b7d085074bc82085b60215909c900372553;p=nmigen.git

hdl.ir: detect elaboratables that are created but not used.

Requres every elaboratable to inherit from Elaboratable, but still
accepts ones that do not, with a warning.

Fixes #3.
---

diff --git a/examples/alu.py b/examples/alu.py
index 211acd2..5c0e8e6 100644
--- a/examples/alu.py
+++ b/examples/alu.py
@@ -2,7 +2,7 @@ from nmigen import *
 from nmigen.cli import main
 
 
-class ALU:
+class ALU(Elaboratable):
     def __init__(self, width):
         self.sel = Signal(2)
         self.a   = Signal(width)
diff --git a/examples/alu_hier.py b/examples/alu_hier.py
index fc6beae..a3273af 100644
--- a/examples/alu_hier.py
+++ b/examples/alu_hier.py
@@ -2,7 +2,7 @@ from nmigen import *
 from nmigen.cli import main
 
 
-class Adder:
+class Adder(Elaboratable):
     def __init__(self, width):
         self.a   = Signal(width)
         self.b   = Signal(width)
@@ -14,7 +14,7 @@ class Adder:
         return m
 
 
-class Subtractor:
+class Subtractor(Elaboratable):
     def __init__(self, width):
         self.a   = Signal(width)
         self.b   = Signal(width)
@@ -26,7 +26,7 @@ class Subtractor:
         return m
 
 
-class ALU:
+class ALU(Elaboratable):
     def __init__(self, width):
         self.op  = Signal()
         self.a   = Signal(width)
diff --git a/examples/arst.py b/examples/arst.py
index 405857b..ef3ed5a 100644
--- a/examples/arst.py
+++ b/examples/arst.py
@@ -2,7 +2,7 @@ from nmigen import *
 from nmigen.cli import main
 
 
-class ClockDivisor:
+class ClockDivisor(Elaboratable):
     def __init__(self, factor):
         self.v = Signal(factor)
         self.o = Signal()
diff --git a/examples/ctr.py b/examples/ctr.py
index 9505a61..9752299 100644
--- a/examples/ctr.py
+++ b/examples/ctr.py
@@ -2,7 +2,7 @@ from nmigen import *
 from nmigen.cli import main, pysim
 
 
-class Counter:
+class Counter(Elaboratable):
     def __init__(self, width):
         self.v = Signal(width, reset=2**width-1)
         self.o = Signal()
diff --git a/examples/ctr_ce.py b/examples/ctr_ce.py
index ebf1db0..f839d67 100644
--- a/examples/ctr_ce.py
+++ b/examples/ctr_ce.py
@@ -2,7 +2,7 @@ from nmigen import *
 from nmigen.back import rtlil, verilog, pysim
 
 
-class Counter:
+class Counter(Elaboratable):
     def __init__(self, width):
         self.v = Signal(width, reset=2**width-1)
         self.o = Signal()
diff --git a/examples/fsm.py b/examples/fsm.py
index 98f5045..73e6f7d 100644
--- a/examples/fsm.py
+++ b/examples/fsm.py
@@ -2,7 +2,7 @@ from nmigen import *
 from nmigen.cli import main
 
 
-class UARTReceiver:
+class UARTReceiver(Elaboratable):
     def __init__(self, divisor):
         self.divisor = divisor
 
diff --git a/examples/gpio.py b/examples/gpio.py
index c234ce0..3dd0da0 100644
--- a/examples/gpio.py
+++ b/examples/gpio.py
@@ -3,7 +3,7 @@ from nmigen import *
 from nmigen.cli import main
 
 
-class GPIO:
+class GPIO(Elaboratable):
     def __init__(self, pins, bus):
         self.pins = pins
         self.bus  = bus
diff --git a/examples/inst.py b/examples/inst.py
index 227a5da..2fc519b 100644
--- a/examples/inst.py
+++ b/examples/inst.py
@@ -2,7 +2,7 @@ from nmigen import *
 from nmigen.cli import main
 
 
-class System:
+class System(Elaboratable):
     def __init__(self):
         self.adr   = Signal(16)
         self.dat_r = Signal(8)
diff --git a/examples/mem.py b/examples/mem.py
index 1d97042..82105fc 100644
--- a/examples/mem.py
+++ b/examples/mem.py
@@ -2,7 +2,7 @@ from nmigen import *
 from nmigen.cli import main
 
 
-class RegisterFile:
+class RegisterFile(Elaboratable):
     def __init__(self):
         self.adr   = Signal(4)
         self.dat_r = Signal(8)
diff --git a/examples/pmux.py b/examples/pmux.py
index 02a6155..1e938b5 100644
--- a/examples/pmux.py
+++ b/examples/pmux.py
@@ -2,7 +2,7 @@ from nmigen import *
 from nmigen.cli import main
 
 
-class ParMux:
+class ParMux(Elaboratable):
     def __init__(self, width):
         self.s = Signal(3)
         self.a = Signal(width)
diff --git a/nmigen/__init__.py b/nmigen/__init__.py
index bc1560d..b5899d7 100644
--- a/nmigen/__init__.py
+++ b/nmigen/__init__.py
@@ -1,7 +1,7 @@
 from .hdl.ast import Value, Const, C, Mux, Cat, Repl, Array, Signal, ClockSignal, ResetSignal
 from .hdl.dsl import Module
 from .hdl.cd import ClockDomain
-from .hdl.ir import Fragment, Instance
+from .hdl.ir import Elaboratable, Fragment, Instance
 from .hdl.mem import Memory
 from .hdl.rec import Record
 from .hdl.xfrm import ResetInserter, CEInserter
diff --git a/nmigen/hdl/dsl.py b/nmigen/hdl/dsl.py
index 95f6eb0..03e7e23 100644
--- a/nmigen/hdl/dsl.py
+++ b/nmigen/hdl/dsl.py
@@ -9,7 +9,7 @@ from .ir import *
 from .xfrm import *
 
 
-__all__ = ["Module", "SyntaxError", "SyntaxWarning"]
+__all__ = ["SyntaxError", "SyntaxWarning", "Module"]
 
 
 class SyntaxError(Exception):
@@ -109,7 +109,7 @@ class FSM:
         return self.state == self.encoding[name]
 
 
-class Module(_ModuleBuilderRoot):
+class Module(_ModuleBuilderRoot, Elaboratable):
     def __init__(self):
         _ModuleBuilderRoot.__init__(self, self, depth=0)
         self.submodules    = _ModuleBuilderSubmodules(self)
diff --git a/nmigen/hdl/ir.py b/nmigen/hdl/ir.py
index 079bba2..164f558 100644
--- a/nmigen/hdl/ir.py
+++ b/nmigen/hdl/ir.py
@@ -1,12 +1,30 @@
-import warnings
+from abc import ABCMeta, abstractmethod
 from collections import defaultdict, OrderedDict
+import warnings
+import traceback
+import sys
 
 from ..tools import *
 from .ast import *
 from .cd import *
 
 
-__all__ = ["Fragment", "Instance", "DriverConflict"]
+__all__ = ["Elaboratable", "DriverConflict", "Fragment", "Instance"]
+
+
+class Elaboratable(metaclass=ABCMeta):
+    def __new__(cls, *args, **kwargs):
+        self = super().__new__(cls)
+        self._Elaboratable__traceback = traceback.extract_stack()[:-1]
+        self._Elaboratable__used      = False
+        return self
+
+    def __del__(self):
+        if hasattr(self, "_Elaboratable__used") and not self._Elaboratable__used:
+            print("Elaboratable created but never used\n",
+                  "Traceback (most recent call last):\n",
+                  *traceback.format_list(self._Elaboratable__traceback),
+                  file=sys.stderr, sep="")
 
 
 class DriverConflict(UserWarning):
@@ -16,13 +34,22 @@ class DriverConflict(UserWarning):
 class Fragment:
     @staticmethod
     def get(obj, platform):
-        if isinstance(obj, Fragment):
-            return obj
-        elif hasattr(obj, "elaborate"):
-            frag = obj.elaborate(platform)
-        else:
-            raise AttributeError("Object '{!r}' cannot be elaborated".format(obj))
-        return Fragment.get(frag, platform)
+        while True:
+            if isinstance(obj, Fragment):
+                return obj
+            elif isinstance(obj, Elaboratable):
+                obj._Elaboratable__used = True
+                obj = obj.elaborate(platform)
+            elif hasattr(obj, "elaborate"):
+                warnings.warn(
+                    message="Class {!r} is an elaboratable that does not explicitly inherit from "
+                            "Elaboratable; doing so would improve diagnostics"
+                            .format(type(obj)),
+                    category=RuntimeWarning,
+                    stacklevel=2)
+                obj = obj.elaborate(platform)
+            else:
+                raise AttributeError("Object '{!r}' cannot be elaborated".format(obj))
 
     def __init__(self):
         self.ports = SignalDict()
diff --git a/nmigen/hdl/mem.py b/nmigen/hdl/mem.py
index bef8035..076d284 100644
--- a/nmigen/hdl/mem.py
+++ b/nmigen/hdl/mem.py
@@ -1,6 +1,6 @@
 from .. import tracer
 from .ast import *
-from .ir import Instance
+from .ir import Elaboratable, Instance
 
 
 __all__ = ["Memory", "ReadPort", "WritePort", "DummyPort"]
@@ -70,7 +70,7 @@ class Memory:
         return self._array[index]
 
 
-class ReadPort:
+class ReadPort(Elaboratable):
     def __init__(self, memory, domain, synchronous, transparent):
         self.memory      = memory
         self.domain      = domain
@@ -135,7 +135,7 @@ class ReadPort:
         return f
 
 
-class WritePort:
+class WritePort(Elaboratable):
     def __init__(self, memory, domain, priority, granularity):
         self.memory       = memory
         self.domain       = domain
diff --git a/nmigen/hdl/xfrm.py b/nmigen/hdl/xfrm.py
index 1385ce8..c554bc6 100644
--- a/nmigen/hdl/xfrm.py
+++ b/nmigen/hdl/xfrm.py
@@ -292,7 +292,7 @@ class FragmentTransformer:
             raise AttributeError("Object '{!r}' cannot be elaborated".format(value))
 
 
-class TransformedElaboratable:
+class TransformedElaboratable(Elaboratable):
     def __init__(self, elaboratable):
         assert hasattr(elaboratable, "elaborate")
 
diff --git a/nmigen/lib/cdc.py b/nmigen/lib/cdc.py
index 3018b37..ec83317 100644
--- a/nmigen/lib/cdc.py
+++ b/nmigen/lib/cdc.py
@@ -4,7 +4,7 @@ from .. import *
 __all__ = ["MultiReg", "ResetSynchronizer"]
 
 
-class MultiReg:
+class MultiReg(Elaboratable):
     """Resynchronise a signal to a different clock domain.
 
     Consists of a chain of flip-flops. Eliminates metastabilities at the output, but provides
@@ -69,7 +69,7 @@ class MultiReg:
         return m
 
 
-class ResetSynchronizer:
+class ResetSynchronizer(Elaboratable):
     def __init__(self, arst, domain="sync", n=2):
         self.arst = arst
         self.domain = domain
diff --git a/nmigen/lib/coding.py b/nmigen/lib/coding.py
index e06d5a7..eed02c9 100644
--- a/nmigen/lib/coding.py
+++ b/nmigen/lib/coding.py
@@ -10,7 +10,7 @@ __all__ = [
 ]
 
 
-class Encoder:
+class Encoder(Elaboratable):
     """Encode one-hot to binary.
 
     If one bit in ``i`` is asserted, ``n`` is low and ``o`` indicates the asserted bit.
@@ -48,7 +48,7 @@ class Encoder:
         return m
 
 
-class PriorityEncoder:
+class PriorityEncoder(Elaboratable):
     """Priority encode requests to binary.
 
     If any bit in ``i`` is asserted, ``n`` is low and ``o`` indicates the least significant
@@ -85,7 +85,7 @@ class PriorityEncoder:
         return m
 
 
-class Decoder:
+class Decoder(Elaboratable):
     """Decode binary to one-hot.
 
     If ``n`` is low, only the ``i``th bit in ``o`` is asserted.
@@ -130,7 +130,7 @@ class PriorityDecoder(Decoder):
     """
 
 
-class GrayEncoder:
+class GrayEncoder(Elaboratable):
     """Encode binary to Gray code.
 
     Parameters
@@ -157,7 +157,7 @@ class GrayEncoder:
         return m
 
 
-class GrayDecoder:
+class GrayDecoder(Elaboratable):
     """Decode Gray code to binary.
 
     Parameters
diff --git a/nmigen/lib/fifo.py b/nmigen/lib/fifo.py
index e484ec8..2844a05 100644
--- a/nmigen/lib/fifo.py
+++ b/nmigen/lib/fifo.py
@@ -102,7 +102,7 @@ def _decr(signal, modulo):
         return Mux(signal == 0, modulo - 1, signal - 1)
 
 
-class SyncFIFO(FIFOInterface):
+class SyncFIFO(Elaboratable, FIFOInterface):
     __doc__ = FIFOInterface._doc_template.format(
     description="""
     Synchronous first in, first out queue.
@@ -209,7 +209,7 @@ class SyncFIFO(FIFOInterface):
         return m
 
 
-class SyncFIFOBuffered(FIFOInterface):
+class SyncFIFOBuffered(Elaboratable, FIFOInterface):
     __doc__ = FIFOInterface._doc_template.format(
     description="""
     Buffered synchronous first in, first out queue.
@@ -265,7 +265,7 @@ class SyncFIFOBuffered(FIFOInterface):
         return m
 
 
-class AsyncFIFO(FIFOInterface):
+class AsyncFIFO(Elaboratable, FIFOInterface):
     __doc__ = FIFOInterface._doc_template.format(
     description="""
     Asynchronous first in, first out queue.
@@ -361,7 +361,7 @@ class AsyncFIFO(FIFOInterface):
         return m
 
 
-class AsyncFIFOBuffered(FIFOInterface):
+class AsyncFIFOBuffered(Elaboratable, FIFOInterface):
     __doc__ = FIFOInterface._doc_template.format(
     description="""
     Buffered asynchronous first in, first out queue.
diff --git a/nmigen/test/__init__.py b/nmigen/test/__init__.py
index e69de29..724ad8a 100644
--- a/nmigen/test/__init__.py
+++ b/nmigen/test/__init__.py
@@ -0,0 +1,6 @@
+from ..hdl.ir import Elaboratable
+
+
+# The nMigen testsuite creates a lot of elaboratables that are intentionally unused.
+# Disable the unused elaboratable check, as in our case it provides nothing but noise.
+del Elaboratable.__del__
diff --git a/nmigen/test/test_hdl_xfrm.py b/nmigen/test/test_hdl_xfrm.py
index 94af1f8..3973b8d 100644
--- a/nmigen/test/test_hdl_xfrm.py
+++ b/nmigen/test/test_hdl_xfrm.py
@@ -488,7 +488,7 @@ class CEInserterTestCase(FHDLTestCase):
         """)
 
 
-class _MockElaboratable:
+class _MockElaboratable(Elaboratable):
     def __init__(self):
         self.s1 = Signal()
 
diff --git a/nmigen/test/test_lib_coding.py b/nmigen/test/test_lib_coding.py
index 83b7831..8c6b624 100644
--- a/nmigen/test/test_lib_coding.py
+++ b/nmigen/test/test_lib_coding.py
@@ -1,6 +1,7 @@
 from .tools import *
 from ..hdl.ast import *
 from ..hdl.dsl import *
+from ..hdl.ir import *
 from ..back.pysim import *
 from ..lib.coding import *
 
@@ -82,7 +83,7 @@ class DecoderTestCase(FHDLTestCase):
             sim.run()
 
 
-class ReversibleSpec:
+class ReversibleSpec(Elaboratable):
     def __init__(self, encoder_cls, decoder_cls, args):
         self.encoder_cls = encoder_cls
         self.decoder_cls = decoder_cls
@@ -99,7 +100,7 @@ class ReversibleSpec:
         return m
 
 
-class HammingDistanceSpec:
+class HammingDistanceSpec(Elaboratable):
     def __init__(self, distance, encoder_cls, args):
         self.distance    = distance
         self.encoder_cls = encoder_cls
diff --git a/nmigen/test/test_lib_fifo.py b/nmigen/test/test_lib_fifo.py
index c7f4c28..3497186 100644
--- a/nmigen/test/test_lib_fifo.py
+++ b/nmigen/test/test_lib_fifo.py
@@ -45,7 +45,7 @@ class FIFOSmokeTestCase(FHDLTestCase):
         self.assertAsyncFIFOWorks(AsyncFIFOBuffered(width=8, depth=3))
 
 
-class FIFOModel(FIFOInterface):
+class FIFOModel(Elaboratable, FIFOInterface):
     """
     Non-synthesizable first-in first-out queue, implemented naively as a chain of registers.
     """
@@ -104,7 +104,7 @@ class FIFOModel(FIFOInterface):
         return m
 
 
-class FIFOModelEquivalenceSpec:
+class FIFOModelEquivalenceSpec(Elaboratable):
     """
     The first-in first-out queue model equivalence specification: for any inputs and control
     signals, the behavior of the implementation under test exactly matches the ideal model,
@@ -148,7 +148,7 @@ class FIFOModelEquivalenceSpec:
         return m
 
 
-class FIFOContractSpec:
+class FIFOContractSpec(Elaboratable):
     """
     The first-in first-out queue contract specification: if two elements are written to the queue
     consecutively, they must be read out consecutively at some later point, no matter all other