From 14d18a3e08f4479a956e45de2af7f8d322ae09eb Mon Sep 17 00:00:00 2001
From: Kagamihara Nadeshiko <nkagamihara@suika>
Date: Mon, 2 Oct 2023 14:38:36 -0700
Subject: [PATCH] Change CanConnect logic: remove blanket direct feedthrough
 ban, and always check against the global precedence graph.

---
 __tests__/InvalidMutations.ts |  5 -----
 src/core/reactor.ts           | 21 +++++++++++++++++++--
 2 files changed, 19 insertions(+), 7 deletions(-)

diff --git a/__tests__/InvalidMutations.ts b/__tests__/InvalidMutations.ts
index 2345eb393..977cad095 100644
--- a/__tests__/InvalidMutations.ts
+++ b/__tests__/InvalidMutations.ts
@@ -38,11 +38,6 @@ class R1 extends Reactor {
       [this.in1],
       [this.in1, this.in2, this.out1, this.out2],
       function (this, __in1, __in2, __out1, __out2) {
-        test("expect error on creating creating direct feed through", () => {
-          expect(() => {
-            this.connect(__in2.asConnectable(), __out2.asConnectable());
-          }).toThrowError("New connection introduces direct feed through.");
-        });
         test("expect error when creating connection outside container", () => {
           expect(() => {
             this.connect(__out2.asConnectable(), __in2.asConnectable());
diff --git a/src/core/reactor.ts b/src/core/reactor.ts
index 65d7415ef..4a9410ca0 100644
--- a/src/core/reactor.ts
+++ b/src/core/reactor.ts
@@ -1201,7 +1201,24 @@ export abstract class Reactor extends Component {
         return CanConnectResult.RT_CONNECTION_OUTSIDE_CONTAINER;
       }
 
-      // Take the local graph and merge in all the causality interfaces
+      /**
+       * TODO (axmmisaka): The following code is commented for multiple reasons:
+       * The causality interface check is not fully implemented so new checks are failing
+       * Second, direct feedthrough itself would not cause any problem *per se*.
+       * To ensure there is no cycle, the safest way is to check against the global dependency graph.
+       */
+
+      let app = this as Reactor;
+      while (app._getContainer() !== app) {
+        app = app._getContainer();
+      }
+      const graph = app._getPrecedenceGraph();
+      graph.addEdge(src, dst);
+      if (graph.hasCycle()) {
+        return CanConnectResult.RT_CYCLE;
+      }
+
+      /*       // Take the local graph and merge in all the causality interfaces
       // of contained reactors. Then:
       const graph = new PrecedenceGraph<Port<unknown> | Reaction<Variable[]>>();
       graph.addAll(this._dependencyGraph);
@@ -1227,7 +1244,7 @@ export abstract class Reactor extends Component {
         dst.getContainer() === src.getContainer()
       ) {
         return CanConnectResult.RT_DIRECT_FEED_THROUGH;
-      }
+      } */
 
       return CanConnectResult.SUCCESS;
     }