From 43ac102ca87cacb0858f47388d7ce3af590687b2 Mon Sep 17 00:00:00 2001
From: ringabout <43030857+ringabout@users.noreply.github.com>
Date: Thu, 21 May 2026 19:42:38 +0800
Subject: [PATCH] fixes #25800; move now uses its declaration for overridden
 =wasMoved (#25809)

fixes #25800
closes https://github.com/nim-lang/Nim/pull/25807
ref https://github.com/nim-lang/Nim/issues/25800

This pull request improves the handling of move semantics and the
`=wasMoved` hook in the Nim compiler, especially for C++ code generation
and user-defined types. It refactors the move operation logic to better
support custom hooks, adds new tests for edge cases, and ensures that
the `move` operation is safer and more predictable.

**Move semantics and `=wasMoved` handling:**

* Refactored the move operation in `compiler/ccgexprs.nim` by
introducing helper procs (`canGenMoveCall`, `genMoveCall`,
`genWasMovedCall`, `genMoveWithWasMoved`) to better handle cases with
user-defined `=wasMoved` hooks, especially for generics and C++ interop.
The logic now distinguishes between simple assignments and when to call
custom hooks, improving correctness and maintainability.
[[1]](diffhunk://#diff-4509107d295d7d32b1887c8993cd0f56113ae60f36113e7d8778646dabd92ebcL2818-R2851)
[[2]](diffhunk://#diff-4509107d295d7d32b1887c8993cd0f56113ae60f36113e7d8778646dabd92ebcL2841-R2882)
* Updated the `move` proc in `lib/system.nim` to include the `nodestroy`
pragma, preventing double destruction and making move semantics safer.

**Testing and validation:**

* Added a new test (`tests/ccgbugs2/t25800.nim`) to ensure that
user-defined `=wasMoved` hooks with `{.importcpp.}` are correctly
generated and invoked in C++ code, addressing a specific bug with
invalid preprocessor directives.
* Expanded `tests/destructor/twasmoved.nim` with additional test cases
for objects with and without custom `=wasMoved` hooks, including
multithreaded scenarios using `threadpool`, to verify correct behavior
in a variety of contexts.

**Minor cleanup:**

* Added a blank line for code style consistency in
`compiler/semmagic.nim`.
---
 compiler/ccgexprs.nim          | 27 +++++--------------
 compiler/spawn.nim             | 24 ++++++++++++++---
 lib/system.nim                 |  2 +-
 tests/ccgbugs2/m25800.h        |  7 +++++
 tests/ccgbugs2/t25800.nim      | 23 ++++++++++++++++
 tests/destructor/twasmoved.nim | 48 ++++++++++++++++++++++++++++++++++
 6 files changed, 107 insertions(+), 24 deletions(-)
 create mode 100644 tests/ccgbugs2/m25800.h
 create mode 100644 tests/ccgbugs2/t25800.nim

diff --git a/compiler/ccgexprs.nim b/compiler/ccgexprs.nim
index 2cf187e687..62302146f1 100644
--- a/compiler/ccgexprs.nim
+++ b/compiler/ccgexprs.nim
@@ -2816,9 +2816,9 @@ proc genWasMoved(p: BProc; n: PNode) =
     #  [addrLoc(p.config, a), getTypeDesc(p.module, a.t)])
 
 proc genMove(p: BProc; n: PNode; d: var TLoc) =
-  var a: TLoc = initLocExpr(p, n[1].skipAddr, {lfEnforceDeref, lfPrepareForMutation})
   if n.len == 4:
     # generated by liftdestructors:
+    var a: TLoc = initLocExpr(p, n[1].skipAddr, {lfEnforceDeref, lfPrepareForMutation})
     var src: TLoc = initLocExpr(p, n[2])
     let destVal = rdLoc(a)
     let srcVal = rdLoc(src)
@@ -2838,29 +2838,16 @@ proc genMove(p: BProc; n: PNode; d: var TLoc) =
   else:
     if d.k == locNone: d = getTemp(p, n.typ)
     if p.config.selectedGC in {gcArc, gcAtomicArc, gcOrc, gcYrc}:
-      genAssignment(p, d, a, {})
       var op = getAttachedOp(p.module.g.graph, n.typ, attachedWasMoved)
-      if op == nil:
+      if op == nil or sfOverridden notin op.flags:
+        var a: TLoc = initLocExpr(p, n[1].skipAddr, {lfEnforceDeref, lfPrepareForMutation})
+        genAssignment(p, d, a, {})
         resetLoc(p, a)
       else:
-        var b = initLocExpr(p, newSymNode(op))
-        case skipTypes(a.t, abstractVar+{tyStatic}).kind
-        of tyOpenArray, tyVarargs: # todo fixme generated `wasMoved` hooks for
-                                   # openarrays, but it probably shouldn't?
-          let ra = rdLoc(a)
-          var s: string
-          if reifiedOpenArray(a.lode):
-            if a.t.kind in {tyVar, tyLent}:
-              s = derefField(ra, "Field0") & cArgumentSeparator & derefField(ra, "Field1")
-            else:
-              s = dotField(ra, "Field0") & cArgumentSeparator & dotField(ra, "Field1")
-          else:
-            s = ra & cArgumentSeparator & ra & "Len_0"
-          p.s(cpsStmts).addCallStmt(rdLoc(b), s)
-        else:
-          let val = if p.module.compileToCpp: rdLoc(a) else: byRefLoc(p, a)
-          p.s(cpsStmts).addCallStmt(rdLoc(b), val)
+        n[1] = makeAddr(n[1], p.module.idgen)
+        genCall(p, n, d)
     else:
+      var a: TLoc = initLocExpr(p, n[1].skipAddr, {lfEnforceDeref, lfPrepareForMutation})
       genAssignment(p, d, a, {})
       resetLoc(p, a)
 
diff --git a/compiler/spawn.nim b/compiler/spawn.nim
index 1318ad4b76..cbadc807c6 100644
--- a/compiler/spawn.nim
+++ b/compiler/spawn.nim
@@ -10,7 +10,7 @@
 ## This module implements threadpool's ``spawn``.
 
 import ast, types, idents, magicsys, msgs, options, modulegraphs,
-  lowerings, liftdestructors, renderer
+  lowerings, liftdestructors, renderer, trees
 from trees import getMagic, getRoot
 
 proc callProc(a: PNode): PNode =
@@ -53,6 +53,24 @@ proc typeNeedsNoDeepCopy(t: PType): bool =
   if t.kind in {tyVar, tyLent, tySequence}: t = t.elementType
   result = not containsGarbageCollectedRef(t)
 
+proc newSpawnMoveStmt(g: ModuleGraph; idgen: IdGenerator; le, ri: PNode): PNode =
+  let op = getAttachedOp(g, ri.typ.skipTypes({tyGenericInst, tyAlias, tyVar, tySink}), attachedWasMoved)
+  if op != nil and sfOverridden in op.flags:
+    result = newNodeI(nkStmtList, le.info)
+    result.add newFastAsgnStmt(le, ri)
+
+    let wasMovedCall = newNodeI(nkCall, ri.info)
+    wasMovedCall.add newSymNode(op)
+    
+    if op.typ != nil and op.typ.signatureLen > 1 and op.typ.firstParamType.kind != tyVar:
+      wasMovedCall.add ri.skipAddr
+    else:
+      wasMovedCall.add makeAddr(ri.skipAddr, idgen)
+    
+    result.add wasMovedCall
+  else:
+    result = newFastMoveStmt(g, le, ri)
+
 proc addLocalVar(g: ModuleGraph; varSection, varInit: PNode; idgen: IdGenerator; owner: PSym; typ: PType;
                  v: PNode; useShallowCopy=false): PSym =
   result = newSym(skTemp, getIdent(g.cache, genPrefix), idgen, owner, varSection.info,
@@ -68,10 +86,10 @@ proc addLocalVar(g: ModuleGraph; varSection, varInit: PNode; idgen: IdGenerator;
   if varInit != nil:
     if g.config.selectedGC in {gcArc, gcOrc, gcAtomicArc, gcYrc}:
       # inject destructors pass will do its own analysis
-      varInit.add newFastMoveStmt(g, newSymNode(result), v)
+      varInit.add newSpawnMoveStmt(g, idgen, newSymNode(result), v)
     else:
       if useShallowCopy and typeNeedsNoDeepCopy(typ) or optTinyRtti in g.config.globalOptions:
-        varInit.add newFastMoveStmt(g, newSymNode(result), v)
+        varInit.add newSpawnMoveStmt(g, idgen, newSymNode(result), v)
       else:
         let deepCopyCall = newNodeI(nkCall, varInit.info, 3)
         deepCopyCall[0] = newSymNode(getSysMagic(g, varSection.info, "deepCopy", mDeepCopy))
diff --git a/lib/system.nim b/lib/system.nim
index 63989b1502..26232971bd 100644
--- a/lib/system.nim
+++ b/lib/system.nim
@@ -166,7 +166,7 @@ proc wasMoved*[T](obj: var T) {.magic: "WasMoved", noSideEffect.}
   ## it was "moved" and to signify its destructor should do nothing and
   ## ideally be optimized away.
 
-proc move*[T](x: var T): T {.magic: "Move", noSideEffect.} =
+proc move*[T](x: var T): T {.magic: "Move", noSideEffect, nodestroy.} =
   result = x
   {.cast(raises: []), cast(tags: []).}:
     `=wasMoved`(x)
diff --git a/tests/ccgbugs2/m25800.h b/tests/ccgbugs2/m25800.h
new file mode 100644
index 0000000000..7961eceda0
--- /dev/null
+++ b/tests/ccgbugs2/m25800.h
@@ -0,0 +1,7 @@
+/*TYPESECTION*/
+struct CppRef {
+  int* data;
+  CppRef() : data(new int(42)) {}
+  ~CppRef() { delete data; data = nullptr; }
+  void reset() { delete data; data = nullptr; }
+};
\ No newline at end of file
diff --git a/tests/ccgbugs2/t25800.nim b/tests/ccgbugs2/t25800.nim
new file mode 100644
index 0000000000..9574c35009
--- /dev/null
+++ b/tests/ccgbugs2/t25800.nim
@@ -0,0 +1,23 @@
+discard """
+  cmd: "nim cpp $file"
+  action: "compile"
+"""
+
+# Bug Report 1: {.importcpp.} on =wasMoved generates invalid preprocessor directive #.
+
+
+type CppRef* {.importcpp, bycopy, noInit, header: "m25800.h".} = object
+
+proc `=destroy`(x: var CppRef) {.importcpp: "#.~CppRef()".}
+proc `=wasMoved`(x: var CppRef) {.importcpp: "#.reset()".}
+proc `=copy`(dest: var CppRef; src: CppRef) {.importcpp: "dest = src".}
+proc `=sink`(dest: var CppRef; src: CppRef) {.importcpp: "dest = std::move(src)".}
+
+# This triggers =wasMoved when passing to sink parameter
+proc consume(x: sink CppRef) = discard
+
+proc test() =
+  var x: CppRef
+  consume(move(x))  # =wasMoved MUST be called here after the move
+
+test()
\ No newline at end of file
diff --git a/tests/destructor/twasmoved.nim b/tests/destructor/twasmoved.nim
index 5663227022..f9a0ad363a 100644
--- a/tests/destructor/twasmoved.nim
+++ b/tests/destructor/twasmoved.nim
@@ -12,3 +12,51 @@ proc foo =
   doAssert m.id == 999
 
 foo()
+
+block:
+  type Foo = object
+    a,b,c: int
+
+  var dest: Foo
+
+  # proc `=wasMoved`(x: var Foo) =
+  #   debugEcho "wasMoved called"
+
+  proc main() =
+    var x = Foo(a:11, b:12, c:13)
+    dest = move(x)
+
+  main()
+
+block:
+  type Foo = object
+    a,b,c: int
+
+  var dest: Foo
+
+  proc `=wasMoved`(x: var Foo) =
+    discard "wasMoved called"
+
+  proc main() =
+    var x = Foo(a:11, b:12, c:13)
+    dest = move(x)
+
+  main()
+
+
+import std/threadpool
+
+block:
+  type Foo = object
+    data: string
+
+  proc `=wasMoved`(x: var Foo) =
+    discard
+
+  proc work(x: Foo) =
+    discard
+
+  var x = Foo(data: "hello")
+  spawn work(x)
+  sync()
+