asserts.Past doesn't work correctly with DomainRenamer

[tpwrules]

DomainRenamers don't seem to change the domain that Past looks at. Below is a minimal example that demonstrates the problem and the correct behavior.

The Toggle module flips its internal toggle every clock cycle, then uses Past to output it delayed by one cycle. Both toggles are DomainRenamed to the "fast" domain. Toggle t1 uses Past(domain=None) and generates an incorrect output because Past samples the toggle from the sync domain; the DomainRenamer didn't change it to "fast". Toggle t2, on the other hand, uses Past(domain="fast") to ensure the sample is taken from the correct clock domain and so generates the correct output.

from nmigen import *
from nmigen.asserts import Past
from nmigen.back.pysim import Simulator, Delay

class Toggle(Elaboratable):
    def __init__(self, domain):
        self.domain = domain
        self.delayed = Signal()

    def elaborate(self, platform):
        m = Module()
        toggle = Signal()
        m.d.sync += toggle.eq(~toggle)
        m.d.sync += self.delayed.eq(Past(toggle, domain=self.domain))
        return m

m = Module()
m.submodules.t1 = t1 = DomainRenamer("fast")(Toggle(None))
m.submodules.t2 = t2 = DomainRenamer("fast")(Toggle("fast"))

sim = Simulator(m)
sim.add_clock(1e-6, domain="sync")
sim.add_clock((1/3)*1e-6, domain="fast")

with sim.write_vcd("test.vcd", "test.gtkw", traces=[t1.delayed, t2.delayed]):
    sim.run_until(10e-6, run_passive=True)

GTKWave output is shown below. The top trace is the output of t1 and the bottom is the output of t2.

[whitequark]

I didn't investigate this in detail and I'll take you at your word that there is a bug; currently, clock/reset signals and domain renames are done via term rewriting, which is unfortunately as you're discovering now error-prone (and also quite slow).

It would probably be possible to add a workaround here but I would much rather prefer to use a different approach that systematically eliminates all similar issues, but would take more time to implement. Are you in a rush?

[tpwrules]

I'm not in a rush. I just avoided using Past in this situation instead. When you change the underlying approach and close the issue, this comment is me reminding myself to go back and re-add it.

[whitequark]

Past is going to be deprecated and removed (#526). Closing this issue in favor of that one.