diff options
| -rw-r--r-- | changelog.md | 4 | ||||
| -rw-r--r-- | compiler/injectdestructors.nim | 60 | ||||
| -rw-r--r-- | compiler/liftdestructors.nim | 30 | ||||
| -rw-r--r-- | doc/destructors.rst | 8 | ||||
| -rw-r--r-- | tests/destructor/smart_ptr.nim | 6 | ||||
| -rw-r--r-- | tests/destructor/tdestructor.nim | 10 |
6 files changed, 56 insertions, 62 deletions
diff --git a/changelog.md b/changelog.md index 34ecd66a0..1c0a8ca6c 100644 --- a/changelog.md +++ b/changelog.md @@ -75,6 +75,10 @@ - An `align` pragma can now be used for variables and object fields, similar to the `alignas` declaration modifier in C/C++. +- `=sink` type bound operator is now optional. Compiler can now use combination + of `=destroy` and `copyMem` to move objects efficiently. + + ## Language changes - Unsigned integer operators have been fixed to allow promotion of the first operand. diff --git a/compiler/injectdestructors.nim b/compiler/injectdestructors.nim index 127cc7723..27171ced6 100644 --- a/compiler/injectdestructors.nim +++ b/compiler/injectdestructors.nim @@ -230,6 +230,10 @@ proc genOp(c: Con; t: PType; kind: TTypeAttachedOp; dest, ri: PNode): PNode = addrExp.add(dest) result = newTree(nkCall, newSymNode(op), addrExp) +proc genDestroy(c: Con; dest: PNode): PNode = + let t = dest.typ.skipTypes({tyGenericInst, tyAlias, tySink}) + result = genOp(c, t, attachedDestructor, dest, nil) + when false: proc preventMoveRef(dest, ri: PNode): bool = let lhs = dest.typ.skipTypes({tyGenericInst, tyAlias, tySink}) @@ -248,18 +252,17 @@ proc canBeMoved(c: Con; t: PType): bool {.inline.} = proc genSink(c: var Con; dest, ri: PNode): PNode = if isFirstWrite(dest, c): # optimize sink call into a bitwise memcopy - result = newTree(nkFastAsgn, dest) + result = newTree(nkFastAsgn, dest, ri) else: let t = dest.typ.skipTypes({tyGenericInst, tyAlias, tySink}) - let k = if t.attachedOps[attachedSink] != nil: attachedSink - else: attachedAsgn - if t.attachedOps[k] != nil: - result = genOp(c, t, k, dest, ri) + if t.attachedOps[attachedSink] != nil: + result = genOp(c, t, attachedSink, dest, ri) + result.add ri else: - # in rare cases only =destroy exists but no sink or assignment - # (see Pony object in tmove_objconstr.nim) - # we generate a fast assignment in this case: - result = newTree(nkFastAsgn, dest) + # the default is to use combination of `=destroy(dest)` and + # and copyMem(dest, source). This is efficient. + let snk = newTree(nkFastAsgn, dest, ri) + result = newTree(nkStmtList, genDestroy(c, dest), snk) proc genCopyNoCheck(c: Con; dest, ri: PNode): PNode = let t = dest.typ.skipTypes({tyGenericInst, tyAlias, tySink}) @@ -273,10 +276,6 @@ proc genCopy(c: var Con; dest, ri: PNode): PNode = checkForErrorPragma(c, t, ri, "=") result = genCopyNoCheck(c, dest, ri) -proc genDestroy(c: Con; dest: PNode): PNode = - let t = dest.typ.skipTypes({tyGenericInst, tyAlias, tySink}) - result = genOp(c, t, attachedDestructor, dest, nil) - proc addTopVar(c: var Con; v: PNode) = c.topLevelVars.add newTree(nkIdentDefs, v, c.emptyNode, c.emptyNode) @@ -298,8 +297,6 @@ proc genDefaultCall(t: PType; c: Con; info: TLineInfo): PNode = proc destructiveMoveVar(n: PNode; c: var Con): PNode = # generate: (let tmp = v; reset(v); tmp) - # XXX: Strictly speaking we can only move if there is a ``=sink`` defined - # or if no ``=sink`` is defined and also no assignment. if not hasDestructor(n.typ): result = copyTree(n) else: @@ -440,9 +437,7 @@ proc ensureDestruction(arg: PNode; c: var Con): PNode = # This was already done in the sink parameter handling logic. result = newNodeIT(nkStmtListExpr, arg.info, arg.typ) let tmp = getTemp(c, arg.typ, arg.info) - var sinkExpr = genSink(c, tmp, arg) - sinkExpr.add arg - result.add sinkExpr + result.add genSink(c, tmp, arg) result.add tmp c.destroys.add genDestroy(c, tmp) else: @@ -598,8 +593,7 @@ proc p(n: PNode; c: var Con; mode: ProcessMode): PNode = if ri.kind == nkEmpty and c.inLoop > 0: ri = genDefaultCall(v.typ, c, v.info) if ri.kind != nkEmpty: - let r = moveOrCopy(v, ri, c) - result.add r + result.add moveOrCopy(v, ri, c) else: # keep the var but transform 'ri': var v = copyNode(n) var itCopy = copyNode(it) @@ -667,8 +661,7 @@ proc moveOrCopy(dest, ri: PNode; c: var Con): PNode = if isUnpackedTuple(dest): result = newTree(nkFastAsgn, dest, p(ri, c, consumed)) else: - result = genSink(c, dest, ri) - result.add p(ri, c, consumed) + result = genSink(c, dest, p(ri, c, consumed)) of nkBracketExpr: if isUnpackedTuple(ri[0]): # unpacking of tuple: take over elements @@ -677,7 +670,6 @@ proc moveOrCopy(dest, ri: PNode; c: var Con): PNode = not aliases(dest, ri): # Rule 3: `=sink`(x, z); wasMoved(z) var snk = genSink(c, dest, ri) - snk.add ri result = newTree(nkStmtList, snk, genWasMoved(ri, c)) else: result = genCopy(c, dest, ri) @@ -686,24 +678,21 @@ proc moveOrCopy(dest, ri: PNode; c: var Con): PNode = # array constructor if ri.len > 0 and isDangerousSeq(ri.typ): result = genCopy(c, dest, ri) + result.add p(ri, c, consumed) else: - result = genSink(c, dest, ri) - result.add p(ri, c, consumed) + result = genSink(c, dest, p(ri, c, consumed)) of nkObjConstr, nkTupleConstr, nkClosure, nkCharLit..nkNilLit: - result = genSink(c, dest, ri) - result.add p(ri, c, consumed) + result = genSink(c, dest, p(ri, c, consumed)) of nkSym: if isSinkParam(ri.sym): # Rule 3: `=sink`(x, z); wasMoved(z) sinkParamIsLastReadCheck(c, ri) - var snk = genSink(c, dest, ri) - snk.add ri + let snk = genSink(c, dest, ri) result = newTree(nkStmtList, snk, genWasMoved(ri, c)) elif ri.sym.kind != skParam and ri.sym.owner == c.owner and isLastRead(ri, c) and canBeMoved(c, dest.typ): # Rule 3: `=sink`(x, z); wasMoved(z) - var snk = genSink(c, dest, ri) - snk.add ri + let snk = genSink(c, dest, ri) result = newTree(nkStmtList, snk, genWasMoved(ri, c)) else: result = genCopy(c, dest, ri) @@ -716,8 +705,7 @@ proc moveOrCopy(dest, ri: PNode; c: var Con): PNode = copyRi[1] = result[^1] result[^1] = copyRi else: - result = genSink(c, dest, ri) - result.add p(ri, c, sinkArg) + result = genSink(c, dest, p(ri, c, sinkArg)) of nkObjDownConv, nkObjUpConv: when false: result = moveOrCopy(dest, ri[0], c) @@ -725,16 +713,14 @@ proc moveOrCopy(dest, ri: PNode; c: var Con): PNode = copyRi[0] = result[^1] result[^1] = copyRi else: - result = genSink(c, dest, ri) - result.add p(ri, c, sinkArg) + result = genSink(c, dest, p(ri, c, sinkArg)) of nkStmtListExpr, nkBlockExpr, nkIfExpr, nkCaseStmt: handleNested(ri): moveOrCopy(dest, node, c) else: if isAnalysableFieldAccess(ri, c.owner) and isLastRead(ri, c) and canBeMoved(c, dest.typ): # Rule 3: `=sink`(x, z); wasMoved(z) - var snk = genSink(c, dest, ri) - snk.add ri + let snk = genSink(c, dest, ri) result = newTree(nkStmtList, snk, genWasMoved(ri, c)) else: result = genCopy(c, dest, ri) diff --git a/compiler/liftdestructors.nim b/compiler/liftdestructors.nim index def557402..64dc3ad45 100644 --- a/compiler/liftdestructors.nim +++ b/compiler/liftdestructors.nim @@ -775,18 +775,26 @@ proc produceSym(g: ModuleGraph; c: PContext; typ: PType; kind: TTypeAttachedOp; # register this operation already: typ.attachedOps[kind] = result - var tk: TTypeKind - if g.config.selectedGC in {gcArc, gcOrc, gcHooks}: - tk = skipTypes(typ, {tyOrdinal, tyRange, tyInferred, tyGenericInst, tyStatic, tyAlias, tySink}).kind - else: - tk = tyNone # no special casing for strings and seqs - case tk - of tySequence: - fillSeqOp(a, typ, body, d, newSymNode(src)) - of tyString: - fillStrOp(a, typ, body, d, newSymNode(src)) + + if kind == attachedSink and typ.attachedOps[attachedDestructor] != nil and + sfOverriden in typ.attachedOps[attachedDestructor].flags: + ## compiler can use a combination of `=destroy` and memCopy for sink op + dest.flags.incl sfCursor + body.add newOpCall(typ.attachedOps[attachedDestructor], d[0]) + body.add newAsgnStmt(d, newSymNode(src)) else: - fillBody(a, typ, body, d, newSymNode(src)) + var tk: TTypeKind + if g.config.selectedGC in {gcArc, gcOrc, gcHooks}: + tk = skipTypes(typ, {tyOrdinal, tyRange, tyInferred, tyGenericInst, tyStatic, tyAlias, tySink}).kind + else: + tk = tyNone # no special casing for strings and seqs + case tk + of tySequence: + fillSeqOp(a, typ, body, d, newSymNode(src)) + of tyString: + fillStrOp(a, typ, body, d, newSymNode(src)) + else: + fillBody(a, typ, body, d, newSymNode(src)) var n = newNodeI(nkProcDef, info, bodyPos+1) for i in 0..<n.len: n[i] = newNodeI(nkEmpty, info) diff --git a/doc/destructors.rst b/doc/destructors.rst index 1ca51ddf1..d2027102a 100644 --- a/doc/destructors.rst +++ b/doc/destructors.rst @@ -53,7 +53,8 @@ written as: a.data[i] = b.data[i] proc `=sink`*[T](a: var myseq[T]; b: myseq[T]) = - # move assignment + # move assignment, optional. + # Compiler is using `=destroy` and `copyMem` when not provided `=destroy`(a) a.len = b.len a.cap = b.cap @@ -130,7 +131,10 @@ A `=sink` hook moves an object around, the resources are stolen from the source and passed to the destination. It is ensured that source's destructor does not free the resources afterwards by setting the object to its default value (the value the object's state started in). Setting an object ``x`` back to its -default value is written as ``wasMoved(x)``. +default value is written as ``wasMoved(x)``. When not provided the compiler +is using a combination of `=destroy` and `copyMem` instead. This is efficient +hence users rarely need to implement their own `=sink` operator, it is enough to +provide `=destroy` and `=`, compiler will take care about the rest. The prototype of this hook for a type ``T`` needs to be: diff --git a/tests/destructor/smart_ptr.nim b/tests/destructor/smart_ptr.nim index 7c3141d22..5079dc9db 100644 --- a/tests/destructor/smart_ptr.nim +++ b/tests/destructor/smart_ptr.nim @@ -20,12 +20,6 @@ proc `=`*[T](dest: var SharedPtr[T], src: SharedPtr[T]) {.inline.} = discard atomicInc(src.val[].atomicCounter) dest.val = src.val -proc `=sink`*[T](dest: var SharedPtr[T], src: SharedPtr[T]) {.inline.} = - if dest.val != src.val: - if dest.val != nil: - `=destroy`(dest) - dest.val = src.val - proc newSharedPtr*[T](val: sink T): SharedPtr[T] = result.val = cast[type(result.val)](allocShared0(sizeof(result.val[]))) result.val.atomicCounter = 1 diff --git a/tests/destructor/tdestructor.nim b/tests/destructor/tdestructor.nim index 780a45288..5cfecea4e 100644 --- a/tests/destructor/tdestructor.nim +++ b/tests/destructor/tdestructor.nim @@ -20,14 +20,10 @@ mygeneric2 destroyed ---- ---- myobj destroyed -myobj destroyed -myobj destroyed -myobj destroyed mygeneric1 destroyed --- myobj destroyed myobj destroyed -myobj destroyed ''' """ @@ -37,8 +33,10 @@ type p: pointer proc `=destroy`(o: var TMyObj) = - if o.p != nil: dealloc o.p - echo "myobj destroyed" + if o.p != nil: + dealloc o.p + o.p = nil + echo "myobj destroyed" type TMyGeneric1[T] = object |