From cba126e9eeb00ebaf684efb906185119c6e02dea Mon Sep 17 00:00:00 2001
From: Matthias Koefferlein <matthias@klayout.de>
Date: Thu, 18 Apr 2024 21:27:09 +0200
Subject: [PATCH] Small enhancements for Spice reader

- Detects recursive subcircuit calls now
- Dismisses empty top level circuit which happened to be created
  when there were not top level elements and control statements
  were present (such as .param)
---
 src/db/db/dbCircuit.cc                    |  5 ++++
 src/db/db/dbCircuit.h                     |  5 ++++
 src/db/db/dbNetlistSpiceReader.cc         | 17 +++++++++++-
 src/db/unit_tests/dbNetlistReaderTests.cc | 34 +++++++++++++++++++++++
 testdata/algo/nreader24.cir               | 14 ++++++++++
 testdata/algo/nreader25.cir               | 11 ++++++++
 6 files changed, 85 insertions(+), 1 deletion(-)
 create mode 100644 testdata/algo/nreader24.cir
 create mode 100644 testdata/algo/nreader25.cir

diff --git a/src/db/db/dbCircuit.cc b/src/db/db/dbCircuit.cc
index aaca632c8..c9c4d2153 100644
--- a/src/db/db/dbCircuit.cc
+++ b/src/db/db/dbCircuit.cc
@@ -644,6 +644,11 @@ void Circuit::set_pin_ref_for_pin (size_t pin_id, Net::pin_iterator iter)
   m_pin_refs [pin_id] = iter;
 }
 
+bool Circuit::is_empty () const
+{
+  return m_nets.empty () && m_pins.empty () && m_devices.empty () && m_subcircuits.empty ();
+}
+
 void Circuit::blank ()
 {
   tl_assert (netlist () != 0);
diff --git a/src/db/db/dbCircuit.h b/src/db/db/dbCircuit.h
index 1e0f0a956..abcb74274 100644
--- a/src/db/db/dbCircuit.h
+++ b/src/db/db/dbCircuit.h
@@ -759,6 +759,11 @@ public:
    */
   void blank ();
 
+  /**
+   *  @brief Gets a value indicating whether the circuit is empty
+   */
+  bool is_empty () const;
+
   /**
    *  @brief Generate memory statistics
    */
diff --git a/src/db/db/dbNetlistSpiceReader.cc b/src/db/db/dbNetlistSpiceReader.cc
index 88632b71a..05976791d 100644
--- a/src/db/db/dbNetlistSpiceReader.cc
+++ b/src/db/db/dbNetlistSpiceReader.cc
@@ -943,6 +943,12 @@ SpiceNetlistBuilder::circuit_for (const SpiceCachedCircuit *cc, const parameters
   if (cp == c->second.end ()) {
     return 0;
   }
+
+  //  a null pointer indicates that we are currently defining this circuit
+  if (cp->second == 0) {
+    error (tl::sprintf (tl::to_string (tr ("Subcircuit '%s' called recursively")), cc->name ()));
+  }
+
   return cp->second;
 }
 
@@ -1055,7 +1061,8 @@ SpiceNetlistBuilder::build_circuit (const SpiceCachedCircuit *cc, const paramete
     c->set_name (make_circuit_name (cc->name (), pv));
   }
 
-  register_circuit_for (cc, pv, c, anonymous_top_level);
+  //  pre-register the circuit - allows detecting recursive calls
+  register_circuit_for (cc, pv, 0, false);
 
   std::unique_ptr<std::map<std::string, db::Net *> > n2n (mp_nets_by_name.release ());
   mp_nets_by_name.reset (0);
@@ -1095,6 +1102,14 @@ SpiceNetlistBuilder::build_circuit (const SpiceCachedCircuit *cc, const paramete
   std::swap (c, mp_netlist_circuit);
   std::swap (vars, m_variables);
 
+  //  final registration if required
+  if (! anonymous_top_level || ! c->is_empty ()) {
+    register_circuit_for (cc, pv, c, anonymous_top_level);
+  } else {
+    mp_netlist->remove_circuit (c);
+    c = 0;
+  }
+
   return c;
 }
 
diff --git a/src/db/unit_tests/dbNetlistReaderTests.cc b/src/db/unit_tests/dbNetlistReaderTests.cc
index 19858f26d..c3b67b343 100644
--- a/src/db/unit_tests/dbNetlistReaderTests.cc
+++ b/src/db/unit_tests/dbNetlistReaderTests.cc
@@ -880,6 +880,40 @@ TEST(23_endl)
   );
 }
 
+TEST(24_recursive_calls)
+{
+  db::Netlist nl;
+
+  std::string path = tl::combine_path (tl::combine_path (tl::testdata (), "algo"), "nreader24.cir");
+
+  db::NetlistSpiceReader reader;
+  tl::InputStream is (path);
+
+  try {
+    reader.read (is, nl);
+    EXPECT_EQ (false, true);
+  } catch (tl::Exception &ex) {
+    EXPECT_EQ (ex.msg (), "Subcircuit 'C1' called recursively in /home/matthias/klayout/master/testdata/algo/nreader24.cir, line 8");
+  }
+}
+
+TEST(25_dismiss_top_level)
+{
+  db::Netlist nl;
+
+  std::string path = tl::combine_path (tl::combine_path (tl::testdata (), "algo"), "nreader25.cir");
+
+  db::NetlistSpiceReader reader;
+  tl::InputStream is (path);
+  reader.read (is, nl);
+
+  EXPECT_EQ (nl.to_string (),
+    "circuit TOP (A=A,B=B);\n"
+    "  device NMOS '1' (S=A,G=B,D=A,B=B) (L=100,W=100,AS=0,AD=0,PS=0,PD=0);\n"
+    "end;\n"
+  );
+}
+
 TEST(100_ExpressionParser)
 {
   std::map<std::string, tl::Variant> vars;
diff --git a/testdata/algo/nreader24.cir b/testdata/algo/nreader24.cir
new file mode 100644
index 000000000..c42a4ff51
--- /dev/null
+++ b/testdata/algo/nreader24.cir
@@ -0,0 +1,14 @@
+* Testing recursive call detection
+
+.subckt c1 a b
+x1 a b c2
+.end
+
+.subckt c2 a b
+x1 a b c1
+.ends
+
+xtop vdd vss c2
+
+.end
+
diff --git a/testdata/algo/nreader25.cir b/testdata/algo/nreader25.cir
new file mode 100644
index 000000000..9026cfc70
--- /dev/null
+++ b/testdata/algo/nreader25.cir
@@ -0,0 +1,11 @@
+* Test: dismiss empty top level circuit
+
+.subckt top a b 
+m1 a b a b nmos
+.ends
+
+* this triggered generation of a top level circuit
+.param p1 17
+
+.end
+