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
This commit is contained in:
ringabout
2023-07-29 16:57:03 +08:00
committed by GitHub
parent f1ac979184
commit f0f3904ff0
14 changed files with 255 additions and 4 deletions

View File

@@ -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,

View File

@@ -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

View File

@@ -157,3 +157,4 @@ proc initDefines*(symbols: StringTableRef) =
defineSymbol("nimHasSendable")
defineSymbol("nimAllowNonVarDestructor")
defineSymbol("nimHasQuirky")
defineSymbol("nimHasEnsureMove")

View File

@@ -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)

View File

@@ -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]);

View File

@@ -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",

View File

@@ -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

View File

@@ -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:

View File

@@ -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)

View File

@@ -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

View File

@@ -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()

View File

@@ -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()

View File

@@ -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()

View File

@@ -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()