Dependency cycle detection does not seem to work

[soerendomroes]

Hi, tried to do the esweek tutorial and I noticed that the dependency cycle analysis does not work.

target C

reactor Printer {
    input in: int
    output out: int
    reaction(in) -> out {=
        // Using a thread-safe print function provided by the runtime.
        info_print("Hello World! %d", in->value);
    =}
}

main reactor {
    p = new Printer()
    p.out -> p.in
}

No validator error is created. So it does not seem to be a problem of the diagram.

[lhstrh]

Oh, that's a significant bug. Thanks for reporting this!

[a-sr]

I had a look at the problem because I was worried that it relates to changes done for modal models but it seems not the case.
It looks like an unhandled corner case. I was able to track down the issue to this point in ReactionInstanceGraph. In this instance dstRuntime and srcRuntime are actually the same and hence no edge is introduced.
My guess is that the code cannot handle this corner case, because as soon as there are two reactions involved, it works again:

I don't fully understand the handling of runtime instances in this algorithm yet but the if was explicitly introduced here to fix a problem with federated. Maybe someone else can take it up from here.

[edwardalee]

Thanks for this pointer. I'm trying to figure out why I put in the check that lets this corner case through... There must have been a reason... :-)

[edwardalee]

I think this will be fixed by PR #1026, but let's see whether the CI tests reveal further problems. I removed the check that @a-sr identified, and this resulted in only one test breaking, src/modal_models/ModalCycleBreaker.lf.
This test seems to have had a genuine cycle in mode Two.
That cycle was being masked by the bug.
I removed the cycle and the test passes, but @a-sr should check that the test is still testing what was intended.

[lhstrh]

I think this bug report shows we're not doing enough unit testing. Especially for critical parts like our dependency analysis, we need more unit tests.