From 5d89db0bce8ea1dba629615ae1ac203fb01ee2a0 Mon Sep 17 00:00:00 2001
From: Neta London <67196883+netalondon@users.noreply.github.com>
Date: Wed, 14 Aug 2024 21:34:31 +0300
Subject: [PATCH] Report error when chip parts have a loop (#423)

* Report error when chip parts have a loop
---
 simulator/src/chip/builder.test.ts | 20 +++++++++++++++++++
 simulator/src/chip/builder.ts      |  2 --
 simulator/src/chip/chip.ts         | 31 +++++++++++++++++++++++++-----
 3 files changed, 46 insertions(+), 7 deletions(-)

diff --git a/simulator/src/chip/builder.test.ts b/simulator/src/chip/builder.test.ts
index f926dfc35..7cf403af0 100644
--- a/simulator/src/chip/builder.test.ts
+++ b/simulator/src/chip/builder.test.ts
@@ -191,6 +191,26 @@ describe("Chip Builder", () => {
       throw new Error(display(e.message ?? e.shortMessage ?? e));
     }
   });
+
+  it("returns error for part loop", async () => {
+    try {
+      const chip = unwrap(
+        HDL.parse(`CHIP Not {
+        IN in;
+        OUT out;
+        PARTS:
+        Nand(a=in, b=b, out=c);
+        Nand(a=in, b=c, out=b);
+      }`),
+      );
+      const foo = await build(chip);
+      expect(foo).toBeErr();
+
+      // eslint-disable-next-line @typescript-eslint/no-explicit-any
+    } catch (e: any) {
+      throw new Error(display(e.message ?? e.shortMessage ?? e));
+    }
+  });
 });
 
 const USE_COPY_HDL = `CHIP UseCopy {
diff --git a/simulator/src/chip/builder.ts b/simulator/src/chip/builder.ts
index 5a7878bb6..086575b36 100644
--- a/simulator/src/chip/builder.ts
+++ b/simulator/src/chip/builder.ts
@@ -218,8 +218,6 @@ class ChipBuilder {
         .filter((pin) => this.chip.isClockedPin(pin)),
     );
 
-    this.chip.sortParts();
-
     // Reset clock order after wiring sub-pins
     for (const part of this.chip.parts) {
       part.subscribeToClock();
diff --git a/simulator/src/chip/chip.ts b/simulator/src/chip/chip.ts
index 682235d42..e84787fc1 100644
--- a/simulator/src/chip/chip.ts
+++ b/simulator/src/chip/chip.ts
@@ -479,7 +479,8 @@ export class Chip {
     return Ok();
   }
 
-  sortParts() {
+  // Returns whether the part connection graph has a loop
+  sortParts(): boolean {
     const sorted: Chip[] = [];
     const visited = new Set<Chip>();
     const visiting = new Set<Chip>();
@@ -500,7 +501,7 @@ export class Chip {
         sorted.push(node.part);
       } else if (!visited.has(node.part)) {
         if (visiting.has(node.part)) {
-          continue;
+          return true;
         }
         visiting.add(node.part);
 
@@ -522,6 +523,7 @@ export class Chip {
     }
 
     this.parts = sorted.reverse();
+    return false;
   }
 
   private findPin(from: string, minWidth?: number): Pin {
@@ -592,14 +594,23 @@ export class Chip {
       chipPin = new OutSubBus(chipPin, to.start, to.width);
     }
 
-    partPin.connect(chipPin);
-
     if (!part.clockedPins.has(partPin.name)) {
       const partToOuts = this.partToOuts.get(part) ?? new Set();
       partToOuts.add(chipPin.name);
       this.partToOuts.set(part, partToOuts);
     }
 
+    const loop = this.sortParts();
+
+    if (loop) {
+      const partToOuts = this.partToOuts.get(part) ?? new Set();
+      partToOuts.delete(chipPin.name);
+      this.partToOuts.set(part, partToOuts);
+      return Err({ message: "Circular pin dependency", lhs: false });
+    } else {
+      partPin.connect(chipPin);
+    }
+
     return Ok();
   }
 
@@ -643,7 +654,6 @@ export class Chip {
         partPin = new OutSubBus(partPin, from.start, from.width);
       }
     }
-    chipPin.connect(partPin);
 
     if (!part.clockedPins.has(partPin.name)) {
       const pinsToPart = this.insToPart.get(chipPin.name) ?? new Set();
@@ -651,6 +661,17 @@ export class Chip {
       this.insToPart.set(chipPin.name, pinsToPart);
     }
 
+    const loop = this.sortParts();
+
+    if (loop) {
+      const pinsToPart = this.insToPart.get(chipPin.name) ?? new Set();
+      pinsToPart.delete(part);
+      this.insToPart.set(chipPin.name, pinsToPart);
+      return Err({ message: "Circular pin dependency", lhs: true });
+    } else {
+      chipPin.connect(partPin);
+    }
+
     return Ok();
   }