diff options
author | ringabout <43030857+ringabout@users.noreply.github.com> | 2023-07-29 16:57:03 +0800 |
---|---|---|
committer | GitHub <noreply@github.com> | 2023-07-29 10:57:03 +0200 |
commit | f0f3904ff04a46bae6f876b0326162354466f415 (patch) | |
tree | c83cd3d31c646221488c82e90fd48461d5028c5c | |
parent | f1ac979184ad7fa0d8c44415e781181a00a0095f (diff) | |
download | Nim-f0f3904ff04a46bae6f876b0326162354466f415.tar.gz |
implement `ensureMove` (#22339)
* implement `ensureMove` * use an additional flag * improve some logics * progress: fixes discard ensureMove * forbids nested expressions * improve error messages * checkpoint * fixes cursor * ADD MORE TESTS * fixes cursorinference again * tiny cleanup * improve error messages * fixes docs * implement comments add more tests * fixes js
-rw-r--r-- | compiler/ast.nim | 2 | ||||
-rw-r--r-- | compiler/ccgexprs.nim | 2 | ||||
-rw-r--r-- | compiler/condsyms.nim | 1 | ||||
-rw-r--r-- | compiler/injectdestructors.nim | 41 | ||||
-rw-r--r-- | compiler/jsgen.nim | 2 | ||||
-rw-r--r-- | compiler/lineinfos.nim | 2 | ||||
-rw-r--r-- | compiler/semmagic.nim | 4 | ||||
-rw-r--r-- | compiler/varpartitions.nim | 4 | ||||
-rw-r--r-- | compiler/vmgen.nim | 2 | ||||
-rw-r--r-- | lib/system.nim | 10 | ||||
-rw-r--r-- | tests/system/tensuremove.nim | 130 | ||||
-rw-r--r-- | tests/system/tensuremove1.nim | 16 | ||||
-rw-r--r-- | tests/system/tensuremove2.nim | 15 | ||||
-rw-r--r-- | tests/system/tensuremove3.nim | 28 |
14 files changed, 255 insertions, 4 deletions
diff --git a/compiler/ast.nim b/compiler/ast.nim index 706c0d38f..539b6e954 100644 --- a/compiler/ast.nim +++ b/compiler/ast.nim @@ -690,7 +690,7 @@ type mIsPartOf, mAstToStr, mParallel, mSwap, mIsNil, mArrToSeq, mOpenArrayToSeq, mNewString, mNewStringOfCap, mParseBiggestFloat, - mMove, mWasMoved, mDup, mDestroy, mTrace, + mMove, mEnsureMove, mWasMoved, mDup, mDestroy, mTrace, mDefault, mUnown, mFinished, mIsolate, mAccessEnv, mAccessTypeField, mReset, mArray, mOpenArray, mRange, mSet, mSeq, mVarargs, mRef, mPtr, mVar, mDistinct, mVoid, mTuple, diff --git a/compiler/ccgexprs.nim b/compiler/ccgexprs.nim index 2b9f4221f..712b874d9 100644 --- a/compiler/ccgexprs.nim +++ b/compiler/ccgexprs.nim @@ -2622,6 +2622,8 @@ proc genMagicExpr(p: BProc, e: PNode, d: var TLoc, op: TMagic) = of mAccessTypeField: genAccessTypeField(p, e, d) of mSlice: genSlice(p, e, d) of mTrace: discard "no code to generate" + of mEnsureMove: + expr(p, e[1], d) else: when defined(debugMagics): echo p.prc.name.s, " ", p.prc.id, " ", p.prc.flags, " ", p.prc.ast[genericParamsPos].kind diff --git a/compiler/condsyms.nim b/compiler/condsyms.nim index c68050449..1146bed14 100644 --- a/compiler/condsyms.nim +++ b/compiler/condsyms.nim @@ -157,3 +157,4 @@ proc initDefines*(symbols: StringTableRef) = defineSymbol("nimHasSendable") defineSymbol("nimAllowNonVarDestructor") defineSymbol("nimHasQuirky") + defineSymbol("nimHasEnsureMove") diff --git a/compiler/injectdestructors.nim b/compiler/injectdestructors.nim index d7f4e38d2..590012806 100644 --- a/compiler/injectdestructors.nim +++ b/compiler/injectdestructors.nim @@ -36,6 +36,7 @@ type body: PNode otherUsage: TLineInfo inUncheckedAssignSection: int + inEnsureMove: int Scope = object # we do scope-based memory management. # a scope is comparable to an nkStmtListExpr like @@ -332,6 +333,9 @@ proc genCopyNoCheck(c: var Con; dest, ri: PNode; a: TTypeAttachedOp): PNode = assert ri.typ != nil proc genCopy(c: var Con; dest, ri: PNode; flags: set[MoveOrCopyFlag]): PNode = + if c.inEnsureMove > 0: + localError(c.graph.config, ri.info, errFailedMove, "cannot move '" & $ri & + "', which introduces an implicit copy") let t = dest.typ if tfHasOwned in t.flags and ri.kind != nkNilLit: # try to improve the error message here: @@ -400,7 +404,7 @@ proc genDefaultCall(t: PType; c: Con; info: TLineInfo): PNode = proc destructiveMoveVar(n: PNode; c: var Con; s: var Scope): PNode = # generate: (let tmp = v; reset(v); tmp) - if not hasDestructor(c, n.typ): + if (not hasDestructor(c, n.typ)) and c.inEnsureMove == 0: assert n.kind != nkSym or not hasDestructor(c, n.sym.typ) result = copyTree(n) else: @@ -419,7 +423,8 @@ proc destructiveMoveVar(n: PNode; c: var Con; s: var Scope): PNode = result.add v let nn = skipConv(n) - c.genMarkCyclic(result, nn) + if hasDestructor(c, n.typ): + c.genMarkCyclic(result, nn) let wasMovedCall = c.genWasMoved(nn) result.add wasMovedCall result.add tempAsNode @@ -462,6 +467,9 @@ proc passCopyToSink(n: PNode; c: var Con; s: var Scope): PNode = message(c.graph.config, n.info, hintPerformance, ("passing '$1' to a sink parameter introduces an implicit copy; " & "if possible, rearrange your program's control flow to prevent it") % $n) + if c.inEnsureMove > 0: + localError(c.graph.config, n.info, errFailedMove, + ("cannot move '$1', passing '$1' to a sink parameter introduces an implicit copy") % $n) else: if c.graph.config.selectedGC in {gcArc, gcOrc, gcAtomicArc}: assert(not containsManagedMemory(n.typ)) @@ -765,7 +773,15 @@ proc p(n: PNode; c: var Con; s: var Scope; mode: ProcessMode; tmpFlags = {sfSing result = passCopyToSink(n, c, s) elif n.kind in {nkBracket, nkObjConstr, nkTupleConstr, nkClosure, nkNilLit} + nkCallKinds + nkLiterals: - result = p(n, c, s, consumed) + if n.kind in nkCallKinds and n[0].kind == nkSym: + if n[0].sym.magic == mEnsureMove: + inc c.inEnsureMove + result = p(n[1], c, s, sinkArg) + dec c.inEnsureMove + else: + result = p(n, c, s, consumed) + else: + result = p(n, c, s, consumed) elif ((n.kind == nkSym and isSinkParam(n.sym)) or isAnalysableFieldAccess(n, c.owner)) and isLastRead(n, c, s) and not (n.kind == nkSym and isCursor(n)): # Sinked params can be consumed only once. We need to reset the memory @@ -837,6 +853,12 @@ proc p(n: PNode; c: var Con; s: var Scope; mode: ProcessMode; tmpFlags = {sfSing if mode == normal and isRefConstr: result = ensureDestruction(result, n, c, s) of nkCallKinds: + if n[0].kind == nkSym and n[0].sym.magic == mEnsureMove: + inc c.inEnsureMove + result = p(n[1], c, s, sinkArg) + dec c.inEnsureMove + return + let inSpawn = c.inSpawn if n[0].kind == nkSym and n[0].sym.magic == mSpawn: c.inSpawn.inc @@ -1069,6 +1091,11 @@ proc genFieldAccessSideEffects(c: var Con; s: var Scope; dest, ri: PNode; flags: result = newTree(nkStmtList, v, snk, c.genWasMoved(newAccess)) proc moveOrCopy(dest, ri: PNode; c: var Con; s: var Scope, flags: set[MoveOrCopyFlag] = {}): PNode = + var ri = ri + var isEnsureMove = 0 + if ri.kind in nkCallKinds and ri[0].kind == nkSym and ri[0].sym.magic == mEnsureMove: + ri = ri[1] + isEnsureMove = 1 if sameLocation(dest, ri): # rule (self-assignment-removal): result = newNodeI(nkEmpty, dest.info) @@ -1103,13 +1130,17 @@ proc moveOrCopy(dest, ri: PNode; c: var Con; s: var Scope, flags: set[MoveOrCopy else: result = c.genSink(s, dest, destructiveMoveVar(ri, c, s), flags) else: + inc c.inEnsureMove, isEnsureMove result = c.genCopy(dest, ri, flags) + dec c.inEnsureMove, isEnsureMove result.add p(ri, c, s, consumed) c.finishCopy(result, dest, isFromSink = false) of nkBracket: # array constructor if ri.len > 0 and isDangerousSeq(ri.typ): + inc c.inEnsureMove, isEnsureMove result = c.genCopy(dest, ri, flags) + dec c.inEnsureMove, isEnsureMove result.add p(ri, c, s, consumed) c.finishCopy(result, dest, isFromSink = false) else: @@ -1127,7 +1158,9 @@ proc moveOrCopy(dest, ri: PNode; c: var Con; s: var Scope, flags: set[MoveOrCopy let snk = c.genSink(s, dest, ri, flags) result = newTree(nkStmtList, snk, c.genWasMoved(ri)) else: + inc c.inEnsureMove, isEnsureMove result = c.genCopy(dest, ri, flags) + dec c.inEnsureMove, isEnsureMove result.add p(ri, c, s, consumed) c.finishCopy(result, dest, isFromSink = false) of nkHiddenSubConv, nkHiddenStdConv, nkConv, nkObjDownConv, nkObjUpConv, nkCast: @@ -1145,7 +1178,9 @@ proc moveOrCopy(dest, ri: PNode; c: var Con; s: var Scope, flags: set[MoveOrCopy let snk = c.genSink(s, dest, ri, flags) result = newTree(nkStmtList, snk, c.genWasMoved(ri)) else: + inc c.inEnsureMove, isEnsureMove result = c.genCopy(dest, ri, flags) + dec c.inEnsureMove, isEnsureMove result.add p(ri, c, s, consumed) c.finishCopy(result, dest, isFromSink = false) diff --git a/compiler/jsgen.nim b/compiler/jsgen.nim index ce1fdb1a5..8be4d9d07 100644 --- a/compiler/jsgen.nim +++ b/compiler/jsgen.nim @@ -2405,6 +2405,8 @@ proc genMagic(p: PProc, n: PNode, r: var TCompRes) = genMove(p, n, r) of mDup: genDup(p, n, r) + of mEnsureMove: + gen(p, n[1], r) else: genCall(p, n, r) #else internalError(p.config, e.info, 'genMagic: ' + magicToStr[op]); diff --git a/compiler/lineinfos.nim b/compiler/lineinfos.nim index 37adc5660..785b10197 100644 --- a/compiler/lineinfos.nim +++ b/compiler/lineinfos.nim @@ -44,6 +44,7 @@ type errRstSandboxedDirective, errProveInit, # deadcode errGenerated, + errFailedMove, errUser, # warnings warnCannotOpenFile = "CannotOpenFile", warnOctalEscape = "OctalEscape", @@ -128,6 +129,7 @@ const errRstSandboxedDirective: "disabled directive: '$1'", errProveInit: "Cannot prove that '$1' is initialized.", # deadcode errGenerated: "$1", + errFailedMove: "$1", errUser: "$1", warnCannotOpenFile: "cannot open '$1'", warnOctalEscape: "octal escape sequences do not exist; leading zero is ignored", diff --git a/compiler/semmagic.nim b/compiler/semmagic.nim index ad7e9821b..97a320774 100644 --- a/compiler/semmagic.nim +++ b/compiler/semmagic.nim @@ -666,5 +666,9 @@ proc magicsAfterOverloadResolution(c: PContext, n: PNode, result = n if result.typ != nil and expectedType != nil and result.typ.kind == tySequence and expectedType.kind == tySequence and result.typ[0].kind == tyEmpty: result.typ = expectedType # type inference for empty sequence # bug #21377 + of mEnsureMove: + result = n + if isAssignable(c, n[1]) notin {arLValue, arLocalLValue}: + localError(c.config, n.info, "'" & $n[1] & "'" & " is not a mutable location; it cannot be moved") else: result = n diff --git a/compiler/varpartitions.nim b/compiler/varpartitions.nim index 6598ef508..6290b311f 100644 --- a/compiler/varpartitions.nim +++ b/compiler/varpartitions.nim @@ -707,6 +707,10 @@ proc traverse(c: var Partitions; n: PNode) = let L = if parameters != nil: parameters.len else: 0 let m = getMagic(n) + if m == mEnsureMove and n[1].kind == nkSym: + # we know that it must be moved so it cannot be a cursor + noCursor(c, n[1].sym) + for i in 1..<n.len: let it = n[i] if i < L: diff --git a/compiler/vmgen.nim b/compiler/vmgen.nim index 50331b971..3a09345ee 100644 --- a/compiler/vmgen.nim +++ b/compiler/vmgen.nim @@ -1390,6 +1390,8 @@ proc genMagic(c: PCtx; n: PNode; dest: var TDest; m: TMagic) = of mRunnableExamples: discard "just ignore any call to runnableExamples" of mDestroy, mTrace: discard "ignore calls to the default destructor" + of mEnsureMove: + gen(c, n[1], dest) of mMove: let arg = n[1] let a = c.genx(arg) diff --git a/lib/system.nim b/lib/system.nim index fc8476b40..25133fca5 100644 --- a/lib/system.nim +++ b/lib/system.nim @@ -156,6 +156,16 @@ proc move*[T](x: var T): T {.magic: "Move", noSideEffect.} = {.cast(raises: []), cast(tags: []).}: `=wasMoved`(x) +when defined(nimHasEnsureMove): + proc ensureMove*[T](x: T): T {.magic: "EnsureMove", noSideEffect.} = + ## Ensures that `x` is moved to the new location, otherwise it gives + ## an error at the compile time. + runnableExamples: + var x = "Hello" + let y = ensureMove(x) + doAssert y == "Hello" + discard "implemented in injectdestructors" + type range*[T]{.magic: "Range".} ## Generic type to construct range types. array*[I, T]{.magic: "Array".} ## Generic type to construct diff --git a/tests/system/tensuremove.nim b/tests/system/tensuremove.nim new file mode 100644 index 000000000..980f2ea58 --- /dev/null +++ b/tests/system/tensuremove.nim @@ -0,0 +1,130 @@ +discard """ + target: "c js" + matrix: "--cursorinference:on; --cursorinference:off" +""" + +block: + type + X = object + s: string + + proc `=copy`(x: var X, y: X) = + x.s = "copied " & y.s + + proc `=sink`(x: var X, y: X) = + `=destroy`(x) + wasMoved(x) + x.s = "moved " & y.s + + proc consume(x: sink X) = + discard x.s + + proc main = + var x = X(s: "abcdefg") + consume(ensureMove x) + + static: main() + main() + +block: + type + String = object + id: string + + proc hello = + var s = String(id: "1") + var m = ensureMove s + doAssert m.id == "1" + + hello() + +block: + type + String = object + id: string + + proc hello = + var n = "1" + var s = String(id: ensureMove n) + var m = ensureMove s + doAssert m.id == "1" + + hello() + +block: + type + String = object + id: string + + proc hello = + var n = "1" + var s = [ensureMove n] + var m = ensureMove s + doAssert m[0] == "1" + + hello() + +block: + type + String = object + id: string + + proc hello = + var n = "1" + var s = @[ensureMove n] + var m = ensureMove s + doAssert m[0] == "1" + + hello() + +block: + type + String = object + id: string + + proc hello = + var s = String(id: "1") + var m = ensureMove s.id + doAssert m == "1" + + hello() + +block: + proc foo = + var x = 1 + let y = ensureMove x # move + when not defined(js): + doAssert (y, x) == (1, 0) # (1, 0) + foo() + +block: + proc foo = + var x = 1 + let y = ensureMove x # move + doAssert y == 1 + foo() + +block: + proc foo = + var x = @[1, 2, 3] + let y = ensureMove x[0] # move + doAssert y == 1 + when not defined(js): + doAssert x == @[0, 2, 3] + foo() + +block: + proc foo = + var x = [1, 2, 3] + let y = ensureMove x[0] # move + doAssert y == 1 + when not defined(js): + doAssert x == @[0, 2, 3] + foo() + +block: + proc foo = + var x = @["1", "2", "3"] + let y = ensureMove x[0] # move + doAssert y == "1" + foo() diff --git a/tests/system/tensuremove1.nim b/tests/system/tensuremove1.nim new file mode 100644 index 000000000..b7e19c4fb --- /dev/null +++ b/tests/system/tensuremove1.nim @@ -0,0 +1,16 @@ +discard """ + errormsg: "cannot move 's', which introduces an implicit copy" + matrix: "--cursorinference:on; --cursorinference:off" +""" + +type + String = object + id: string + +proc hello = + var s = String(id: "1") + var m = ensureMove s + discard m + discard s + +hello() \ No newline at end of file diff --git a/tests/system/tensuremove2.nim b/tests/system/tensuremove2.nim new file mode 100644 index 000000000..1fcbc1c0f --- /dev/null +++ b/tests/system/tensuremove2.nim @@ -0,0 +1,15 @@ +discard """ + errormsg: "'if true: s else: String()' is not a mutable location; it cannot be moved" +""" + +type + String = object + id: string + +proc hello = + var s = String(id: "1") + var m = ensureMove(if true: s else: String()) + discard m + discard s + +hello() \ No newline at end of file diff --git a/tests/system/tensuremove3.nim b/tests/system/tensuremove3.nim new file mode 100644 index 000000000..eca032673 --- /dev/null +++ b/tests/system/tensuremove3.nim @@ -0,0 +1,28 @@ +discard """ + errormsg: "cannot move 'x', passing 'x' to a sink parameter introduces an implicit copy" + matrix: "--cursorinference:on; --cursorinference:off" +""" + +block: + type + X = object + s: string + + proc `=copy`(x: var X, y: X) = + x.s = "copied " & y.s + + proc `=sink`(x: var X, y: X) = + `=destroy`(x) + wasMoved(x) + x.s = "moved " & y.s + + proc consume(x: sink X) = + discard x.s + + proc main = + var s = "abcdefg" + var x = X(s: ensureMove s) + consume(ensureMove x) + discard x + + main() \ No newline at end of file |