fixes #19291; implements wasMoved hook (#21303)

* fixes #19291; implements `wasMoved` hook

* basics

* checkpoint

* finish `wasMoved`

* add a test for #19291

* add documentation and changelog

* work `attachedWasMoved` with generics

* fixes optimizer

* register `=wasMoved`

* handle wasMoved magcis

* check another round

* some patches

* try `op == nil`

* nicer

* generate `wasMoved` before `destroy`

* try again

* fixes tests

* default wasMoved

* Update tests/destructor/tv2_cast.nim

* Update tests/destructor/tv2_cast.nim

* Update tests/arc/topt_refcursors.nim

changelogs/changelog_2_0_0.md

@@ -341,6 +341,8 @@
 
 - IBM Z architecture and macOS m1 arm64 architecture are supported.
 
+- `=wasMoved` can be overridden by users.
+
 ## Compiler changes
 
 - The `gc` switch has been renamed to `mm` ("memory management") in order to reflect the

compiler/ast.nim

@@ -935,6 +935,7 @@ type
   TTypeSeq* = seq[PType]
 
   TTypeAttachedOp* = enum ## as usual, order is important here
+    attachedWasMoved,
     attachedDestructor,
     attachedAsgn,
     attachedSink,
@@ -1502,7 +1503,7 @@ proc newProcNode*(kind: TNodeKind, info: TLineInfo, body: PNode,
 
 const
   AttachedOpToStr*: array[TTypeAttachedOp, string] = [
-    "=destroy", "=copy", "=sink", "=trace", "=deepcopy"]
+    "=wasMoved", "=destroy", "=copy", "=sink", "=trace", "=deepcopy"]
 
 proc `$`*(s: PSym): string =
   if s != nil:

compiler/injectdestructors.nim

@@ -354,11 +354,18 @@ It is best to factor out piece of object that needs custom destructor into separ
   result.add newTree(nkFastAsgn, le, tmp)
 
 proc genWasMoved(c: var Con, n: PNode): PNode =
-  result = newNodeI(nkCall, n.info)
-  result.add(newSymNode(createMagic(c.graph, c.idgen, "wasMoved", mWasMoved)))
-  result.add copyTree(n) #mWasMoved does not take the address
-  #if n.kind != nkSym:
-  #  message(c.graph.config, n.info, warnUser, "wasMoved(" & $n & ")")
+  let typ = n.typ.skipTypes({tyGenericInst, tyAlias, tySink})
+  let op = getAttachedOp(c.graph, n.typ, attachedWasMoved)
+  if op != nil:
+    if sfError in op.flags:
+      c.checkForErrorPragma(n.typ, n, "=wasMoved")
+    result = genOp(c, op, n)
+  else:
+    result = newNodeI(nkCall, n.info)
+    result.add(newSymNode(createMagic(c.graph, c.idgen, "wasMoved", mWasMoved)))
+    result.add copyTree(n) #mWasMoved does not take the address
+    #if n.kind != nkSym:
+    #  message(c.graph.config, n.info, warnUser, "wasMoved(" & $n & ")")
 
 proc genDefaultCall(t: PType; c: Con; info: TLineInfo): PNode =
   result = newNodeI(nkCall, info)

compiler/liftdestructors.nim

@@ -88,6 +88,8 @@ proc defaultOp(c: var TLiftCtx; t: PType; body, x, y: PNode) =
     let call = genBuiltin(c, mDefault, "default", x)
     call.typ = t
     body.add newAsgnStmt(x, call)
+  elif c.kind == attachedWasMoved:
+    body.add genBuiltin(c, mWasMoved, "wasMoved", x)
 
 proc genAddr(c: var TLiftCtx; x: PNode): PNode =
   if x.kind == nkHiddenDeref:
@@ -145,6 +147,11 @@ proc destructorCall(c: var TLiftCtx; op: PSym; x: PNode): PNode =
   else:
     result = destroy
 
+proc genWasMovedCall(c: var TLiftCtx; op: PSym; x: PNode): PNode =
+  result = newNodeIT(nkCall, x.info, op.typ[0])
+  result.add(newSymNode(op))
+  result.add genAddr(c, x)
+
 proc fillBodyObj(c: var TLiftCtx; n, body, x, y: PNode; enforceDefaultOp: bool) =
   case n.kind
   of nkSym:
@@ -442,6 +449,20 @@ proc considerUserDefinedOp(c: var TLiftCtx; t: PType; body, x, y: PNode): bool =
       body.add newDeepCopyCall(c, op, x, y)
       result = true
 
+  of attachedWasMoved:
+    var op = getAttachedOp(c.g, t, attachedWasMoved)
+    if op != nil and sfOverriden in op.flags:
+
+      if op.ast.isGenericRoutine:
+        # patch generic destructor:
+        op = instantiateGeneric(c, op, t, t.typeInst)
+        setAttachedOp(c.g, c.idgen.module, t, attachedWasMoved, op)
+
+      #markUsed(c.g.config, c.info, op, c.g.usageSym)
+      onUse(c.info, op)
+      body.add genWasMovedCall(c, op, x)
+      result = true
+
 proc declareCounter(c: var TLiftCtx; body: PNode; first: BiggestInt): PNode =
   var temp = newSym(skTemp, getIdent(c.g.cache, lowerings.genPrefix), nextSymId(c.idgen), c.fn, c.info)
   temp.typ = getSysType(c.g, body.info, tyInt)
@@ -524,6 +545,7 @@ proc fillSeqOp(c: var TLiftCtx; t: PType; body, x, y: PNode) =
     if canFormAcycle(t.elemType):
       # follow all elements:
       forallElements(c, t, body, x, y)
+  of attachedWasMoved: body.add genBuiltin(c, mWasMoved, "wasMoved", x)
 
 proc useSeqOrStrOp(c: var TLiftCtx; t: PType; body, x, y: PNode) =
   createTypeBoundOps(c.g, c.c, t, body.info, c.idgen)
@@ -561,6 +583,7 @@ proc useSeqOrStrOp(c: var TLiftCtx; t: PType; body, x, y: PNode) =
       if op == nil:
         return # protect from recursion
       body.add newHookCall(c, op, x, y)
+  of attachedWasMoved: body.add genBuiltin(c, mWasMoved, "wasMoved", x)
 
 proc fillStrOp(c: var TLiftCtx; t: PType; body, x, y: PNode) =
   case c.kind
@@ -576,6 +599,7 @@ proc fillStrOp(c: var TLiftCtx; t: PType; body, x, y: PNode) =
     body.add genBuiltin(c, mDestroy, "destroy", x)
   of attachedTrace:
     discard "strings are atomic and have no inner elements that are to trace"
+  of attachedWasMoved: body.add genBuiltin(c, mWasMoved, "wasMoved", x)
 
 proc cyclicType*(t: PType): bool =
   case t.kind
@@ -674,6 +698,8 @@ proc atomicRefOp(c: var TLiftCtx; t: PType; body, x, y: PNode) =
         # If the ref is polymorphic we have to account for this
         body.add callCodegenProc(c.g, "nimTraceRefDyn", c.info, genAddrOf(x, c.idgen), y)
       #echo "can follow ", elemType, " static ", isFinal(elemType)
+  of attachedWasMoved: body.add genBuiltin(c, mWasMoved, "wasMoved", x)
+
 
 proc atomicClosureOp(c: var TLiftCtx; t: PType; body, x, y: PNode) =
   ## Closures are really like refs except they always use a virtual destructor
@@ -722,6 +748,7 @@ proc atomicClosureOp(c: var TLiftCtx; t: PType; body, x, y: PNode) =
   of attachedDeepCopy: assert(false, "cannot happen")
   of attachedTrace:
     body.add callCodegenProc(c.g, "nimTraceRefDyn", c.info, genAddrOf(xenv, c.idgen), y)
+  of attachedWasMoved: body.add genBuiltin(c, mWasMoved, "wasMoved", x)
 
 proc weakrefOp(c: var TLiftCtx; t: PType; body, x, y: PNode) =
   case c.kind
@@ -746,6 +773,7 @@ proc weakrefOp(c: var TLiftCtx; t: PType; body, x, y: PNode) =
       body.sons.insert(des, 0)
   of attachedDeepCopy: assert(false, "cannot happen")
   of attachedTrace: discard
+  of attachedWasMoved: body.add genBuiltin(c, mWasMoved, "wasMoved", x)
 
 proc ownedRefOp(c: var TLiftCtx; t: PType; body, x, y: PNode) =
   var actions = newNodeI(nkStmtList, c.info)
@@ -771,6 +799,7 @@ proc ownedRefOp(c: var TLiftCtx; t: PType; body, x, y: PNode) =
     body.add genIf(c, x, actions)
   of attachedDeepCopy: assert(false, "cannot happen")
   of attachedTrace: discard
+  of attachedWasMoved: body.add genBuiltin(c, mWasMoved, "wasMoved", x)
 
 proc closureOp(c: var TLiftCtx; t: PType; body, x, y: PNode) =
   if c.kind == attachedDeepCopy:
@@ -805,6 +834,7 @@ proc closureOp(c: var TLiftCtx; t: PType; body, x, y: PNode) =
         body.sons.insert(des, 0)
     of attachedDeepCopy: assert(false, "cannot happen")
     of attachedTrace: discard
+    of attachedWasMoved: body.add genBuiltin(c, mWasMoved, "wasMoved", x)
 
 proc ownedClosureOp(c: var TLiftCtx; t: PType; body, x, y: PNode) =
   let xx = genBuiltin(c, mAccessEnv, "accessEnv", x)
@@ -820,6 +850,7 @@ proc ownedClosureOp(c: var TLiftCtx; t: PType; body, x, y: PNode) =
     body.add genIf(c, xx, actions)
   of attachedDeepCopy: assert(false, "cannot happen")
   of attachedTrace: discard
+  of attachedWasMoved: body.add genBuiltin(c, mWasMoved, "wasMoved", x)
 
 proc fillBody(c: var TLiftCtx; t: PType; body, x, y: PNode) =
   case t.kind
@@ -935,7 +966,7 @@ proc symPrototype(g: ModuleGraph; typ: PType; owner: PSym; kind: TTypeAttachedOp
 
   result.typ = newProcType(info, nextTypeId(idgen), owner)
   result.typ.addParam dest
-  if kind != attachedDestructor:
+  if kind notin {attachedDestructor, attachedWasMoved}:
     result.typ.addParam src
 
   if kind == attachedAsgn and g.config.selectedGC == gcOrc and
@@ -975,7 +1006,7 @@ proc produceSym(g: ModuleGraph; c: PContext; typ: PType; kind: TTypeAttachedOp;
 
   let dest = result.typ.n[1].sym
   let d = newDeref(newSymNode(dest))
-  let src = if kind == attachedDestructor: newNodeIT(nkSym, info, getSysType(g, info, tyPointer))
+  let src = if kind in {attachedDestructor, attachedWasMoved}: newNodeIT(nkSym, info, getSysType(g, info, tyPointer))
             else: newSymNode(result.typ.n[2].sym)
 
   # register this operation already:
@@ -1103,15 +1134,15 @@ proc createTypeBoundOps(g: ModuleGraph; c: PContext; orig: PType; info: TLineInf
   # bug #15122: We need to produce all prototypes before entering the
   # mind boggling recursion. Hacks like these imply we should rewrite
   # this module.
-  var generics: array[attachedDestructor..attachedTrace, bool]
-  for k in attachedDestructor..lastAttached:
+  var generics: array[attachedWasMoved..attachedTrace, bool]
+  for k in attachedWasMoved..lastAttached:
     generics[k] = getAttachedOp(g, canon, k) != nil
     if not generics[k]:
       setAttachedOp(g, idgen.module, canon, k,
           symPrototype(g, canon, canon.owner, k, info, idgen))
 
   # we generate the destructor first so that other operators can depend on it:
-  for k in attachedDestructor..lastAttached:
+  for k in attachedWasMoved..lastAttached:
     if not generics[k]:
       discard produceSym(g, c, canon, k, info, idgen)
     else:

compiler/optimizer.nim

@@ -16,6 +16,8 @@ import
 
 from trees import exprStructuralEquivalent
 
+import std/strutils
+
 const
   nfMarkForDeletion = nfNone # faster than a lookup table
 
@@ -110,16 +112,17 @@ proc analyse(c: var Con; b: var BasicBlock; n: PNode) =
     var reverse = false
     if n[0].kind == nkSym:
       let s = n[0].sym
-      if s.magic == mWasMoved:
+      let name = s.name.s.normalize
+      if s.magic == mWasMoved or name == "=wasmoved":
         b.wasMovedLocs.add n
         special = true
-      elif s.name.s == "=destroy":
+      elif name == "=destroy":
         if c.inFinally > 0 and (b.hasReturn or b.hasBreak):
           discard "cannot optimize away the destructor"
         else:
           c.wasMovedDestroyPair b, n
         special = true
-      elif s.name.s == "=sink":
+      elif name == "=sink":
         reverse = true
 
     if not special:

compiler/semstmts.nim

@@ -1779,7 +1779,7 @@ proc whereToBindTypeHook(c: PContext; t: PType): PType =
 proc bindTypeHook(c: PContext; s: PSym; n: PNode; op: TTypeAttachedOp) =
   let t = s.typ
   var noError = false
-  let cond = if op == attachedDestructor:
+  let cond = if op in {attachedDestructor, attachedWasMoved}:
                t.len == 2 and t[0] == nil and t[1].kind == tyVar
              elif op == attachedTrace:
                t.len == 3 and t[0] == nil and t[1].kind == tyVar and t[2].kind == tyPointer
@@ -1894,6 +1894,9 @@ proc semOverride(c: PContext, s: PSym, n: PNode) =
   of "=trace":
     if s.magic != mTrace:
       bindTypeHook(c, s, n, attachedTrace)
+  of "=wasmoved":
+    if s.magic != mWasMoved:
+      bindTypeHook(c, s, n, attachedWasMoved)
   else:
     if sfOverriden in s.flags:
       localError(c.config, n.info, errGenerated,

doc/manual.md

@@ -4122,7 +4122,7 @@ the operator is in scope (including if it is private).
   ```
 
 Type bound operators are:
-`=destroy`, `=copy`, `=sink`, `=trace`, `=deepcopy`.
+`=destroy`, `=copy`, `=sink`, `=trace`, `=deepcopy`, `=wasMoved`.
 
 These operations can be *overridden* instead of *overloaded*. This means that
 the implementation is automatically lifted to structured types. For instance,

tests/arc/topt_no_cursor.nim

@@ -1,7 +1,8 @@
 discard """
   nimoutFull: true
   cmd: '''nim c -r --warnings:off --hints:off --gc:arc --expandArc:newTarget --expandArc:delete --expandArc:p1 --expandArc:tt --hint:Performance:off --assertions:off --expandArc:extractConfig --expandArc:mergeShadowScope --expandArc:check $file'''
-  nimout: '''--expandArc: newTarget
+  nimout: '''
+--expandArc: newTarget
 
 var
   splat
@@ -11,9 +12,9 @@ splat = splitDrive do:
   let blitTmp = path
   blitTmp
 :tmp = splat.drive
-wasMoved(splat.drive)
+`=wasMoved`(splat.drive)
 :tmp_1 = splat.path_1
-wasMoved(splat.path_1)
+`=wasMoved`(splat.path_1)
 result = (
   let blitTmp_1 = :tmp
   blitTmp_1,
@@ -60,10 +61,10 @@ var
 try:
   it_cursor = x
   a = (
-    wasMoved(:tmpD)
+    `=wasMoved`(:tmpD)
     `=copy`(:tmpD, it_cursor.key)
     :tmpD,
-    wasMoved(:tmpD_1)
+    `=wasMoved`(:tmpD_1)
     `=copy`(:tmpD_1, it_cursor.val)
     :tmpD_1)
   echo [
@@ -112,7 +113,7 @@ block :tmp:
       var :tmpD
       sym = shadowScope.symbols[i]
       addInterfaceDecl(c):
-        wasMoved(:tmpD)
+        `=wasMoved`(:tmpD)
         `=copy_1`(:tmpD, sym)
         :tmpD
       inc(i, 1)
@@ -125,7 +126,7 @@ this.isValid = fileExists(this.value)
 if dirExists(this.value):
   var :tmpD
   par = (dir:
-    wasMoved(:tmpD)
+    `=wasMoved`(:tmpD)
     `=copy`(:tmpD, this.value)
     :tmpD, front: "") else:
   var
@@ -133,10 +134,10 @@ if dirExists(this.value):
     :tmpD_2
     :tmpD_3
   par = (dir_1: parentDir(this.value), front_1:
-    wasMoved(:tmpD_1)
+    `=wasMoved`(:tmpD_1)
     `=copy`(:tmpD_1,
       :tmpD_3 = splitDrive do:
-        wasMoved(:tmpD_2)
+        `=wasMoved`(:tmpD_2)
         `=copy`(:tmpD_2, this.value)
         :tmpD_2
       :tmpD_3.path)

tests/arc/topt_refcursors.nim

@@ -1,7 +1,8 @@
 discard """
   output: ''''''
   cmd: '''nim c --gc:arc --expandArc:traverse --hint:Performance:off $file'''
-  nimout: '''--expandArc: traverse
+  nimout: '''
+--expandArc: traverse
 
 var
   it_cursor
@@ -22,12 +23,13 @@ try:
         `=copy`(ri_1, jt.ri)
         echo [jt.s]
         `=sink`(jt, ri_1)
-        wasMoved(ri_1)
+        `=wasMoved`(ri_1)
       finally:
         `=destroy`(ri_1)
 finally:
   `=destroy`(jt)
--- end of expandArc ------------------------'''
+-- end of expandArc ------------------------
+'''
 """
 
 type

tests/arc/topt_wasmoved_destroy_pairs.nim

@@ -1,7 +1,8 @@
 discard """
   output: ''''''
   cmd: '''nim c --gc:arc --expandArc:main --expandArc:tfor --hint:Performance:off $file'''
-  nimout: '''--expandArc: main
+  nimout: '''
+--expandArc: main
 
 var
   a
@@ -29,6 +30,7 @@ try:
   x = f()
   block :tmp:
     var i_cursor
+    mixin inc
     var i_1 = 0
     block :tmp_1:
       while i_1 < 4:
@@ -37,25 +39,26 @@ try:
         if i_cursor == 2:
           return
         add(a):
-          wasMoved(:tmpD)
+          `=wasMoved`(:tmpD)
           `=copy`(:tmpD, x)
           :tmpD
         inc i_1, 1
   if cond:
     add(a):
       let blitTmp = x
-      wasMoved(x)
+      `=wasMoved`(x)
       blitTmp
   else:
     add(b):
       let blitTmp_1 = x
-      wasMoved(x)
+      `=wasMoved`(x)
       blitTmp_1
 finally:
   `=destroy`(x)
   `=destroy_1`(b)
   `=destroy_1`(a)
--- end of expandArc ------------------------'''
+-- end of expandArc ------------------------
+'''
 """
 
 proc f(): seq[int] =

tests/destructor/tv2_cast.nim

@@ -4,7 +4,8 @@ discard """
 @[1953719668, 875770417]
 destroying O1'''
   cmd: '''nim c --gc:arc --expandArc:main --expandArc:main1 --expandArc:main2 --expandArc:main3 --hints:off --assertions:off $file'''
-  nimout: '''--expandArc: main
+  nimout: '''
+--expandArc: main
 
 var
   data
@@ -12,7 +13,7 @@ var
   :tmpD_1
   :tmpD_2
 data =
-  wasMoved(:tmpD)
+  `=wasMoved`(:tmpD)
   `=copy`(:tmpD, cast[string](
     :tmpD_2 = encode(cast[seq[byte]](
       :tmpD_1 = newString(100)
@@ -32,7 +33,7 @@ var
   :tmpD_1
 s = newString(100)
 data =
-  wasMoved(:tmpD)
+  `=wasMoved`(:tmpD)
   `=copy`(:tmpD, cast[string](
     :tmpD_1 = encode(toOpenArrayByte(s, 0, len(s) - 1))
     :tmpD_1))
@@ -50,7 +51,7 @@ var
   :tmpD_1
 s = newSeq(100)
 data =
-  wasMoved(:tmpD)
+  `=wasMoved`(:tmpD)
   `=copy`(:tmpD, cast[string](
     :tmpD_1 = encode(s)
     :tmpD_1))
@@ -67,7 +68,7 @@ var
   :tmpD_1
   :tmpD_2
 data =
-  wasMoved(:tmpD)
+  `=wasMoved`(:tmpD)
   `=copy`(:tmpD, cast[string](
     :tmpD_2 = encode do:
       :tmpD_1 = newSeq(100)
@@ -77,7 +78,8 @@ data =
 `=destroy`(:tmpD_2)
 `=destroy`(:tmpD_1)
 `=destroy_1`(data)
--- end of expandArc ------------------------'''
+-- end of expandArc ------------------------
+'''
 """
 
 func encode*(src: openArray[byte]): seq[byte] =

tests/destructor/twasmoved_error.nim

@@ -0,0 +1,37 @@
+discard """
+  cmd: '''nim c --mm:arc $file'''
+  errormsg: "'=wasMoved' is not available for type <Game>; routine: main"
+"""
+
+# bug #19291
+
+const
+  screenWidth = 800
+  screenHeight = 450
+
+var
+  ready = false
+type
+  Game = object
+
+proc `=destroy`(x: var Game) =
+  assert ready, "Window is already opened"
+  ready = false
+
+proc `=sink`(x: var Game; y: Game) {.error.}
+proc `=copy`(x: var Game; y: Game) {.error.}
+proc `=wasMoved`(x: var Game) {.error.}
+
+proc initGame(width, height: int32, title: string): Game =
+  assert not ready, "Window is already closed"
+  ready = true
+
+proc update(x: Game) = discard
+
+proc main =
+  var g = initGame(screenWidth, screenHeight, "Tetris raylib")
+  g.update()
+  var g2 = g
+  echo "hello"
+
+main()