From c9716f4032399ac5a1c7ae2fc7db9fad431b5605 Mon Sep 17 00:00:00 2001
From: zerbina <100542850+zerbina@users.noreply.github.com>
Date: Mon, 8 Apr 2024 21:39:10 +0000
Subject: [PATCH 1/3] ccgtypes: don't use `#ifdef` for seq definitions

Move the `tySequence` handling from `getTypeDescWeak` to `getTypeDesc`.
Behaviour is largely the same, but with the important difference that
the `tySequence` type is only pushed to the type stack *once*, removing
the need to use a C `#ifdef` pre-processor block around the payload
struct definition.
---
 compiler/backend/ccgtypes.nim | 61 +++++++++++++++++------------------
 1 file changed, 29 insertions(+), 32 deletions(-)

diff --git a/compiler/backend/ccgtypes.nim b/compiler/backend/ccgtypes.nim
index cbadf38337f..b7d308f38ac 100644
--- a/compiler/backend/ccgtypes.nim
+++ b/compiler/backend/ccgtypes.nim
@@ -294,29 +294,6 @@ proc getTypeDescWeak(m: BModule; t: PType; check: var IntSet): Rope =
   of tyObject, tyTuple:
     result = getTypeForward(m, t, hashType(t))
     pushType(m, t)
-  of tySequence:
-      let sig = hashType(t)
-      m.config.internalAssert(skipTypes(etB[0], typedescInst).kind != tyEmpty,
-                              "cannot map the empty seq type to a C type")
-
-      result = cacheGetType(m.forwTypeCache, sig)
-      if result == "":
-        result = getTypeName(m, t, sig)
-        if not isImportedType(t):
-          m.forwTypeCache[sig] = result
-          addForwardStructFormat(m, rope"struct", result)
-          let payload = result & "_Content"
-          addForwardStructFormat(m, rope"struct", payload)
-
-      if cacheGetType(m.typeCache, sig) == "":
-        m.typeCache[sig] = result
-        #echo "adding ", sig, " ", typeToString(t), " ", m.module.name.s
-        appcg(m, m.s[cfsTypes],
-          "struct $1 {$N" &
-          "  NI len; $1_Content* p;$N" &
-          "};$N", [result])
-
-      pushType(m, t)
   else:
     result = getTypeDescAux(m, t, check)
 
@@ -329,16 +306,12 @@ proc seqV2ContentType(m: BModule; t: PType; check: var IntSet) =
   let sig = hashType(t)
   let result = cacheGetType(m.typeCache, sig)
   if result == "":
+    # the struct definition hasn't been emitted yet
     discard getTypeDescAux(m, t, check)
   else:
-    # little hack for now to prevent multiple definitions of the same
-    # Seq_Content:
-    appcg(m, m.s[cfsTypes], """$N
-$3ifndef $2_Content_PP
-$3define $2_Content_PP
-struct $2_Content { NI cap; $1 data[SEQ_DECL_SIZE];};
-$3endif$N
-      """, [getTypeDescAux(m, t.skipTypes(abstractInst)[0], check), result, rope"#"])
+    # emit the payload type:
+    appcg(m, m.s[cfsTypes], "struct $2_Content { NI cap; $1 data[SEQ_DECL_SIZE];};$N",
+          [getTypeDescAux(m, t.skipTypes(abstractInst)[0], check), result])
 
 proc prepareParameters(m: BModule, t: PType): seq[TLoc] =
   ## Sets up and returns the locs of the parameter symbols for procedure
@@ -667,7 +640,31 @@ proc getTypeDescAux(m: BModule, origTyp: PType, check: var IntSet): Rope =
             "void* ClE_0;$n} $1;$n",
              [result, rettype, desc])
   of tySequence:
-      result = getTypeDescWeak(m, t, check)
+    # a sequence type is two structs underneath: one for the seq itself, and
+    # one for its payload
+    m.config.internalAssert(skipTypes(t[0], typedescInst).kind != tyEmpty,
+                            "cannot map the empty seq type to a C type")
+
+    result = cacheGetType(m.forwTypeCache, sig)
+    if result == "":
+      result = getTypeName(m, origTyp, sig)
+      if not isImportedType(t):
+        m.forwTypeCache[sig] = result
+        addForwardStructFormat(m, structOrUnion(t), result)
+
+    # it's possible that the element type cannot be emitted yet because it
+    # depends on the sequence type (a cyclic type). For this reason, the
+    # payload type is only forward-declared here, and the actual definition
+    # is emitted later
+    addForwardStructFormat(m, structOrUnion(t), result & "_Content")
+    # note: force push the type (by not using ``pushType``)
+    m.typeStack.add origTyp
+
+    m.typeCache[sig] = result
+    appcg(m, m.s[cfsTypes],
+      "struct $1 {$N" &
+      "  NI len; $1_Content* p;$N" &
+      "};$N", [result])
   of tyUncheckedArray:
     result = getTypeName(m, origTyp, sig)
     m.typeCache[sig] = result

From f58c7621532087588375a798efb127a690535203 Mon Sep 17 00:00:00 2001
From: zerbina <100542850+zerbina@users.noreply.github.com>
Date: Mon, 8 Apr 2024 21:39:11 +0000
Subject: [PATCH 2/3] ccgtypes: don't use `getTypeDescWeak` for seq types

* for pointer indirections, only a forward declaration is requested
* in field contexts, `getTypeDescAux` can be called directly
---
 compiler/backend/ccgtypes.nim | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/compiler/backend/ccgtypes.nim b/compiler/backend/ccgtypes.nim
index b7d308f38ac..999d266f41f 100644
--- a/compiler/backend/ccgtypes.nim
+++ b/compiler/backend/ccgtypes.nim
@@ -470,9 +470,6 @@ proc genRecordFieldsAux(m: BModule, n: PNode,
     if fieldType.kind == tyUncheckedArray:
       result.addf("$1 $2[SEQ_DECL_SIZE];$n",
           [getTypeDescAux(m, fieldType.elemType, check), sname])
-    elif fieldType.kind == tySequence:
-      # we need to use a weak dependency here for trecursive_table.
-      result.addf("$1$3 $2;$n", [getTypeDescWeak(m, field.typ, check), sname, noAlias])
     elif field.bitsize != 0:
       result.addf("$1$4 $2:$3;$n", [getTypeDescAux(m, field.typ, check), sname, rope($field.bitsize), noAlias])
     else:
@@ -591,12 +588,10 @@ proc getTypeDescAux(m: BModule, origTyp: PType, check: var IntSet): Rope =
         et = elemType(etB)
       etB = et.skipTypes(abstractInst)
     case etB.kind
-    of tyObject, tyTuple:
+    of tyObject, tyTuple, tySequence:
       # no restriction! We have a forward declaration for structs
       let name = getTypeForward(m, et, hashType et)
       result = name & star
-    of tySequence:
-        result = getTypeDescWeak(m, et, check) & star
     of tyOpenArray:
       result = getTypeDescAux(m, etB, check)
     else:

From ed1877bb3551f8f69bc6b67c62b7378d536b876d Mon Sep 17 00:00:00 2001
From: zerbina <100542850+zerbina@users.noreply.github.com>
Date: Mon, 8 Apr 2024 21:39:11 +0000
Subject: [PATCH 3/3] tests: make `trecursive_table.nim` actually work

The `T` object type wasn't actually used anywhere in alive code,
meaning no code was generated for it, and thus that the regression it
intended to catch would have never been caught.

The `p` procedure is now exported, forcing code generation to run for
it (and subsequently the `T` type).
---
 tests/ccgbugs/trecursive_table.nim | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/ccgbugs/trecursive_table.nim b/tests/ccgbugs/trecursive_table.nim
index 3406a1c31f5..fd9480941c3 100644
--- a/tests/ccgbugs/trecursive_table.nim
+++ b/tests/ccgbugs/trecursive_table.nim
@@ -13,5 +13,5 @@ type
     of eY:
       nil
 
-proc p*(x: Table[string, T]) =
+proc p*(x: Table[string, T]) {.exportc.} =
   discard