remove shallowCopy for ARC/ORC (#20070)

* remove shallowCopy for ARC/ORC

* use move

* fix

* more fixes

* typo

* Update lib/system.nim

* follow

* add nodestroy

* move

* copy string

* add a changelog entry

Co-authored-by: xflywind <43030857+xflywind@users.noreply.github.com>
Co-authored-by: Andreas Rumpf <rumpf_a@web.de>

changelog.md

@@ -25,6 +25,9 @@
   or define your own `Math.trunc` polyfill using the [`emit` pragma](https://nim-lang.org/docs/manual.html#implementation-specific-pragmas-emit-pragma). Nim uses
   `Math.trunc` for the division and modulo operators for integers.
 
+- `shallowCopy` is removed for ARC/ORC. Use `move` when possible or combine assignment and
+`sink` for optimization purposes.
+
 ## Standard library additions and changes
 
 [//]: # "Changes:"

compiler/btrees.nim

@@ -68,7 +68,10 @@ proc copyHalf[Key, Val](h, result: Node[Key, Val]) =
       result.links[j] = h.links[Mhalf + j]
   else:
     for j in 0..<Mhalf:
-      shallowCopy(result.vals[j], h.vals[Mhalf + j])
+      when defined(gcArc) or defined(gcOrc):
+        result.vals[j] = move h.vals[Mhalf + j]
+      else:
+        shallowCopy(result.vals[j], h.vals[Mhalf + j])
 
 proc split[Key, Val](h: Node[Key, Val]): Node[Key, Val] =
   ## split node in half
@@ -88,7 +91,10 @@ proc insert[Key, Val](h: Node[Key, Val], key: Key, val: Val): Node[Key, Val] =
       if less(key, h.keys[j]): break
       inc j
     for i in countdown(h.entries, j+1):
-      shallowCopy(h.vals[i], h.vals[i-1])
+      when defined(gcArc) or defined(gcOrc):
+        h.vals[i] = move h.vals[i-1]
+      else:
+        shallowCopy(h.vals[i], h.vals[i-1])
     h.vals[j] = val
   else:
     var newLink: Node[Key, Val] = nil

compiler/msgs.nim

@@ -218,11 +218,18 @@ proc setDirtyFile*(conf: ConfigRef; fileIdx: FileIndex; filename: AbsoluteFile)
 
 proc setHash*(conf: ConfigRef; fileIdx: FileIndex; hash: string) =
   assert fileIdx.int32 >= 0
-  shallowCopy(conf.m.fileInfos[fileIdx.int32].hash, hash)
+  when defined(gcArc) or defined(gcOrc):
+    conf.m.fileInfos[fileIdx.int32].hash = hash
+  else:
+    shallowCopy(conf.m.fileInfos[fileIdx.int32].hash, hash)
+
 
 proc getHash*(conf: ConfigRef; fileIdx: FileIndex): string =
   assert fileIdx.int32 >= 0
-  shallowCopy(result, conf.m.fileInfos[fileIdx.int32].hash)
+  when defined(gcArc) or defined(gcOrc):
+    result = conf.m.fileInfos[fileIdx.int32].hash
+  else:
+    shallowCopy(result, conf.m.fileInfos[fileIdx.int32].hash)
 
 proc toFullPathConsiderDirty*(conf: ConfigRef; fileIdx: FileIndex): AbsoluteFile =
   if fileIdx.int32 < 0:

compiler/nimfix/prettybase.nim

@@ -22,7 +22,10 @@ proc replaceDeprecated*(conf: ConfigRef; info: TLineInfo; oldSym, newSym: PIdent
   let last = first+identLen(line, first)-1
   if cmpIgnoreStyle(line[first..last], oldSym.s) == 0:
     var x = line.substr(0, first-1) & newSym.s & line.substr(last+1)
-    system.shallowCopy(conf.m.fileInfos[info.fileIndex.int32].lines[info.line.int-1], x)
+    when defined(gcArc) or defined(gcOrc):
+      conf.m.fileInfos[info.fileIndex.int32].lines[info.line.int-1] = move x
+    else:
+      system.shallowCopy(conf.m.fileInfos[info.fileIndex.int32].lines[info.line.int-1], x)
     conf.m.fileInfos[info.fileIndex.int32].dirty = true
     #if newSym.s == "File": writeStackTrace()
 
@@ -35,5 +38,8 @@ proc replaceComment*(conf: ConfigRef; info: TLineInfo) =
   if line[first] != '#': inc first
 
   var x = line.substr(0, first-1) & "discard " & line.substr(first+1).escape
-  system.shallowCopy(conf.m.fileInfos[info.fileIndex.int32].lines[info.line.int-1], x)
+  when defined(gcArc) or defined(gcOrc):
+    conf.m.fileInfos[info.fileIndex.int32].lines[info.line.int-1] = move x
+  else:
+    system.shallowCopy(conf.m.fileInfos[info.fileIndex.int32].lines[info.line.int-1], x)
   conf.m.fileInfos[info.fileIndex.int32].dirty = true

compiler/pragmas.nim

@@ -511,7 +511,10 @@ proc processCompile(c: PContext, n: PNode) =
     n[i] = c.semConstExpr(c, n[i])
     case n[i].kind
     of nkStrLit, nkRStrLit, nkTripleStrLit:
-      shallowCopy(result, n[i].strVal)
+      when defined(gcArc) or defined(gcOrc):
+        result = n[i].strVal
+      else:
+        shallowCopy(result, n[i].strVal)
     else:
       localError(c.config, n.info, errStringLiteralExpected)
       result = ""

compiler/vm.nim

@@ -114,8 +114,12 @@ template decodeBx(k: untyped) {.dirty.} =
   let rbx = instr.regBx - wordExcess
   ensureKind(k)
 
-template move(a, b: untyped) {.dirty.} = system.shallowCopy(a, b)
-# XXX fix minor 'shallowCopy' overloading bug in compiler
+template move(a, b: untyped) {.dirty.} =
+  when defined(gcArc) or defined(gcOrc):
+    a = move b
+  else:
+    system.shallowCopy(a, b)
+    # XXX fix minor 'shallowCopy' overloading bug in compiler
 
 proc derefPtrToReg(address: BiggestInt, typ: PType, r: var TFullReg, isAssign: bool): bool =
   # nim bug: `isAssign: static bool` doesn't work, giving odd compiler error

lib/pure/asynchttpserver.nim

@@ -170,7 +170,7 @@ proc processRequest(
   server: AsyncHttpServer,
   req: FutureVar[Request],
   client: AsyncSocket,
-  address: string,
+  address: sink string,
   lineFut: FutureVar[string],
   callback: proc (request: Request): Future[void] {.closure, gcsafe.},
 ): Future[bool] {.async.} =
@@ -184,7 +184,10 @@ proc processRequest(
   # \n
   request.headers.clear()
   request.body = ""
-  request.hostname.shallowCopy(address)
+  when defined(gcArc) or defined(gcOrc):
+    request.hostname = address
+  else:
+    request.hostname.shallowCopy(address)
   assert client != nil
   request.client = client
 

lib/pure/marshal.nim

@@ -288,7 +288,7 @@ proc load*[T](s: Stream, data: var T) =
   var tab = initTable[BiggestInt, pointer]()
   loadAny(s, toAny(data), tab)
 
-proc store*[T](s: Stream, data: T) =
+proc store*[T](s: Stream, data: sink T) =
   ## Stores `data` into the stream `s`. Raises `IOError` in case of an error.
   runnableExamples:
     import std/streams
@@ -301,13 +301,16 @@ proc store*[T](s: Stream, data: T) =
 
   var stored = initIntSet()
   var d: T
-  shallowCopy(d, data)
+  when defined(gcArc) or defined(gcOrc):
+    d = data
+  else:
+    shallowCopy(d, data)
   storeAny(s, toAny(d), stored)
 
 proc loadVM[T](typ: typedesc[T], x: T): string =
   discard "the implementation is in the compiler/vmops"
 
-proc `$$`*[T](x: T): string =
+proc `$$`*[T](x: sink T): string =
   ## Returns a string representation of `x` (serialization, marshalling).
   ##
   ## **Note:** to serialize `x` to JSON use `%x` from the `json` module
@@ -327,7 +330,10 @@ proc `$$`*[T](x: T): string =
   else:
     var stored = initIntSet()
     var d: T
-    shallowCopy(d, x)
+    when defined(gcArc) or defined(gcOrc):
+      d = x
+    else:
+      shallowCopy(d, x)
     var s = newStringStream()
     storeAny(s, toAny(d), stored)
     result = s.data

lib/system.nim

@@ -464,17 +464,16 @@ proc low*(x: string): int {.magic: "Low", noSideEffect.}
   ##  var str = "Hello world!"
   ##  low(str) # => 0
 
-proc shallowCopy*[T](x: var T, y: T) {.noSideEffect, magic: "ShallowCopy".}
-  ## Use this instead of `=` for a `shallow copy`:idx:.
-  ##
-  ## The shallow copy only changes the semantics for sequences and strings
-  ## (and types which contain those).
-  ##
-  ## Be careful with the changed semantics though!
-  ## There is a reason why the default assignment does a deep copy of sequences
-  ## and strings.
-  ##
-  ## .. warning:: `shallowCopy` does a deep copy with ARC/ORC.
+when not defined(gcArc) and not defined(gcOrc):
+  proc shallowCopy*[T](x: var T, y: T) {.noSideEffect, magic: "ShallowCopy".}
+    ## Use this instead of `=` for a `shallow copy`:idx:.
+    ##
+    ## The shallow copy only changes the semantics for sequences and strings
+    ## (and types which contain those).
+    ##
+    ## Be careful with the changed semantics though!
+    ## There is a reason why the default assignment does a deep copy of sequences
+    ## and strings.
 
 # :array|openArray|string|seq|cstring|tuple
 proc `[]`*[I: Ordinal;T](a: T; i: I): T {.
@@ -492,9 +491,12 @@ proc arrPut[I: Ordinal;T,S](a: T; i: I;
 proc `=destroy`*[T](x: var T) {.inline, magic: "Destroy".} =
   ## Generic `destructor`:idx: implementation that can be overridden.
   discard
-proc `=sink`*[T](x: var T; y: T) {.inline, magic: "Asgn".} =
+proc `=sink`*[T](x: var T; y: T) {.inline, nodestroy, magic: "Asgn".} =
   ## Generic `sink`:idx: implementation that can be overridden.
-  shallowCopy(x, y)
+  when defined(gcArc) or defined(gcOrc):
+    x = y
+  else:
+    shallowCopy(x, y)
 
 when defined(nimHasTrace):
   proc `=trace`*[T](x: var T; env: pointer) {.inline, magic: "Trace".} =
@@ -2852,7 +2854,10 @@ when hasAlloc or defined(nimscript):
     setLen(x, xl+item.len)
     var j = xl-1
     while j >= i:
-      shallowCopy(x[j+item.len], x[j])
+      when defined(gcArc) or defined(gcOrc):
+        x[j+item.len] = move x[j]
+      else:
+        shallowCopy(x[j+item.len], x[j])
       dec(j)
     j = 0
     while j < item.len:

tests/gc/cyclecollector.nim

@@ -9,7 +9,10 @@ type
 proc createCycle(leaf: string): Node =
   new result
   result.a = result
-  shallowCopy result.leaf, leaf
+  when defined(gcArc) or defined(gcOrc):
+    result.leaf = leaf
+  else:
+    shallowCopy result.leaf, leaf
 
 proc main =
   for i in 0 .. 100_000: