turn nimIncrSeqV3 into deadcode (#20388)

changelog.md

@@ -26,7 +26,7 @@
 - `shallowCopy` is removed for ARC/ORC. Use `move` when possible or combine assignment and
 `sink` for optimization purposes.
 
-- `nimPreviewDotLikeOps` is going to be removed or deprecated.
+- The `nimPreviewDotLikeOps` define is going to be removed or deprecated.
 
 - The `{.this.}` pragma, deprecated since 0.19, has been removed.
 - `nil` literals can no longer be directly assigned to variables or fields of `distinct` pointer types. They must be converted instead.
@@ -44,6 +44,8 @@
 - [Overloadable enums](https://nim-lang.github.io/Nim/manual_experimental.html#overloadable-enum-value-names)
   are no longer experimental.
 
+- Removed the `nimIncrSeqV3` define.
+
 - Static linking against OpenSSL versions below 1.1, previously done by
   setting `-d:openssl10`, is no longer supported.
 

compiler/condsyms.nim

@@ -70,7 +70,7 @@ proc initDefines*(symbols: StringTableRef) =
   defineSymbol("nimVmExportFixed") # deadcode
   defineSymbol("nimHasSymOwnerInMacro") # deadcode
   defineSymbol("nimNewRuntime") # deadcode
-  defineSymbol("nimIncrSeqV3") # xxx: turn this into deadcode
+  defineSymbol("nimIncrSeqV3") # deadcode
   defineSymbol("nimAshr") # deadcode
   defineSymbol("nimNoNilSeqs") # deadcode
   defineSymbol("nimNoNilSeqs2") # deadcode

lib/system/gc.nim

@@ -561,35 +561,8 @@ proc growObj(old: pointer, newsize: int, gch: var GcHeap): pointer =
   gcTrace(res, csAllocated)
   track("growObj old", ol, 0)
   track("growObj new", res, newsize)
-  when defined(nimIncrSeqV3):
-    # since we steal the old seq's contents, we set the old length to 0.
-    cast[PGenericSeq](old).len = 0
-  elif reallyDealloc:
-    sysAssert(allocInv(gch.region), "growObj before dealloc")
-    if ol.refcount shr rcShift <=% 1:
-      # free immediately to save space:
-      if (ol.refcount and ZctFlag) != 0:
-        var j = gch.zct.len-1
-        var d = gch.zct.d
-        while j >= 0:
-          if d[j] == ol:
-            d[j] = res
-            break
-          dec(j)
-      beforeDealloc(gch, ol, "growObj stack trash")
-      decTypeSize(ol, ol.typ)
-      rawDealloc(gch.region, ol)
-    else:
-      # we split the old refcount in 2 parts. XXX This is still not entirely
-      # correct if the pointer that receives growObj's result is on the stack.
-      # A better fix would be to emit the location specific write barrier for
-      # 'growObj', but this is lots of more work and who knows what new problems
-      # this would create.
-      res.refcount = rcIncrement
-      decRef(ol)
-  else:
-    sysAssert(ol.typ != nil, "growObj: 5")
-    zeroMem(ol, sizeof(Cell))
+  # since we steal the old seq's contents, we set the old length to 0.
+  cast[PGenericSeq](old).len = 0
   when useCellIds:
     inc gch.idGenerator
     res.id = gch.idGenerator * 1000_000 + gch.gcThreadId

lib/system/sysstr.nim

@@ -160,13 +160,9 @@ proc addChar(s: NimString, c: char): NimString =
     result = s
     if result.len >= result.space:
       let r = resize(result.space)
-      when defined(nimIncrSeqV3):
-        result = rawNewStringNoInit(r)
-        result.len = s.len
-        copyMem(addr result.data[0], unsafeAddr(s.data[0]), s.len+1)
-      else:
-        result = cast[NimString](growObj(result,
-          sizeof(TGenericSeq) + r + 1))
+      result = rawNewStringNoInit(r)
+      result.len = s.len
+      copyMem(addr result.data[0], unsafeAddr(s.data[0]), s.len+1)
       result.reserved = r
   result.data[result.len] = c
   result.data[result.len+1] = '\0'
@@ -210,12 +206,9 @@ proc resizeString(dest: NimString, addlen: int): NimString {.compilerRtl.} =
     result = dest
   else: # slow path:
     let sp = max(resize(dest.space), dest.len + addlen)
-    when defined(nimIncrSeqV3):
-      result = rawNewStringNoInit(sp)
-      result.len = dest.len
-      copyMem(addr result.data[0], unsafeAddr(dest.data[0]), dest.len+1)
-    else:
-      result = cast[NimString](growObj(dest, sizeof(TGenericSeq) + sp + 1))
+    result = rawNewStringNoInit(sp)
+    result.len = dest.len
+    copyMem(addr result.data[0], unsafeAddr(dest.data[0]), dest.len+1)
     result.reserved = sp
     #result = rawNewString(sp)
     #copyMem(result, dest, dest.len + sizeof(TGenericSeq))
@@ -239,14 +232,11 @@ proc setLengthStr(s: NimString, newLen: int): NimString {.compilerRtl.} =
     result = s
   else:
     let sp = max(resize(s.space), newLen)
-    when defined(nimIncrSeqV3):
-      result = rawNewStringNoInit(sp)
-      result.len = s.len
-      copyMem(addr result.data[0], unsafeAddr(s.data[0]), s.len+1)
-      zeroMem(addr result.data[s.len], newLen - s.len)
-      result.reserved = sp
-    else:
-      result = resizeString(s, n)
+    result = rawNewStringNoInit(sp)
+    result.len = s.len
+    copyMem(addr result.data[0], unsafeAddr(s.data[0]), s.len+1)
+    zeroMem(addr result.data[s.len], newLen - s.len)
+    result.reserved = sp
   result.len = n
   result.data[n] = '\0'
 
@@ -282,15 +272,11 @@ proc incrSeqV3(s: PGenericSeq, typ: PNimType): PGenericSeq {.compilerproc.} =
     result = s
     if result.len >= result.space:
       let r = resize(result.space)
-      when defined(nimIncrSeqV3):
-        result = cast[PGenericSeq](newSeq(typ, r))
-        result.len = s.len
-        copyMem(dataPointer(result, typ.base.align), dataPointer(s, typ.base.align), s.len * typ.base.size)
-        # since we steal the content from 's', it's crucial to set s's len to 0.
-        s.len = 0
-      else:
-        result = cast[PGenericSeq](growObj(result, align(GenericSeqSize, typ.base.align) + typ.base.size * r))
-        result.reserved = r
+      result = cast[PGenericSeq](newSeq(typ, r))
+      result.len = s.len
+      copyMem(dataPointer(result, typ.base.align), dataPointer(s, typ.base.align), s.len * typ.base.size)
+      # since we steal the content from 's', it's crucial to set s's len to 0.
+      s.len = 0
 
 proc setLengthSeq(seq: PGenericSeq, elemSize, elemAlign, newLen: int): PGenericSeq {.
     compilerRtl, inl.} =
@@ -324,36 +310,33 @@ proc setLengthSeqV2(s: PGenericSeq, typ: PNimType, newLen: int): PGenericSeq {.
   if s == nil:
     result = cast[PGenericSeq](newSeq(typ, newLen))
   else:
-    when defined(nimIncrSeqV3):
-      let elemSize = typ.base.size
-      let elemAlign = typ.base.align
-      if s.space < newLen:
-        let r = max(resize(s.space), newLen)
-        result = cast[PGenericSeq](newSeq(typ, r))
-        copyMem(dataPointer(result, elemAlign), dataPointer(s, elemAlign), s.len * elemSize)
-        # since we steal the content from 's', it's crucial to set s's len to 0.
-        s.len = 0
-      elif newLen < s.len:
-        result = s
-        # we need to decref here, otherwise the GC leaks!
-        when not defined(boehmGC) and not defined(nogc) and
-            not defined(gcMarkAndSweep) and not defined(gogc) and
-            not defined(gcRegions):
-          if ntfNoRefs notin typ.base.flags:
-            for i in newLen..result.len-1:
-              forAllChildrenAux(dataPointer(result, elemAlign, elemSize, i),
-                                extGetCellType(result).base, waZctDecRef)
+    let elemSize = typ.base.size
+    let elemAlign = typ.base.align
+    if s.space < newLen:
+      let r = max(resize(s.space), newLen)
+      result = cast[PGenericSeq](newSeq(typ, r))
+      copyMem(dataPointer(result, elemAlign), dataPointer(s, elemAlign), s.len * elemSize)
+      # since we steal the content from 's', it's crucial to set s's len to 0.
+      s.len = 0
+    elif newLen < s.len:
+      result = s
+      # we need to decref here, otherwise the GC leaks!
+      when not defined(boehmGC) and not defined(nogc) and
+          not defined(gcMarkAndSweep) and not defined(gogc) and
+          not defined(gcRegions):
+        if ntfNoRefs notin typ.base.flags:
+          for i in newLen..result.len-1:
+            forAllChildrenAux(dataPointer(result, elemAlign, elemSize, i),
+                              extGetCellType(result).base, waZctDecRef)
 
-        # XXX: zeroing out the memory can still result in crashes if a wiped-out
-        # cell is aliased by another pointer (ie proc parameter or a let variable).
-        # This is a tough problem, because even if we don't zeroMem here, in the
-        # presence of user defined destructors, the user will expect the cell to be
-        # "destroyed" thus creating the same problem. We can destroy the cell in the
-        # finalizer of the sequence, but this makes destruction non-deterministic.
-        zeroMem(dataPointer(result, elemAlign, elemSize, newLen), (result.len-%newLen) *% elemSize)
-      else:
-        result = s
-        zeroMem(dataPointer(result, elemAlign, elemSize, result.len), (newLen-%result.len) *% elemSize)
-      result.len = newLen
+      # XXX: zeroing out the memory can still result in crashes if a wiped-out
+      # cell is aliased by another pointer (ie proc parameter or a let variable).
+      # This is a tough problem, because even if we don't zeroMem here, in the
+      # presence of user defined destructors, the user will expect the cell to be
+      # "destroyed" thus creating the same problem. We can destroy the cell in the
+      # finalizer of the sequence, but this makes destruction non-deterministic.
+      zeroMem(dataPointer(result, elemAlign, elemSize, newLen), (result.len-%newLen) *% elemSize)
     else:
-      result = setLengthSeq(s, typ.base.size, newLen)
+      result = s
+      zeroMem(dataPointer(result, elemAlign, elemSize, result.len), (newLen-%result.len) *% elemSize)
+    result.len = newLen