Skip to content

Commit 5724c68

Browse files
ringaboutnarimiran
authored andcommitted
fixes #24760; Noncopyable base type ignored (#24777)
fixes #24760 I tried `incl` `tfHasAsgn` to nontrivial assignment, but that solution seems to break too many things. Instead, in this PR, `passCopyToSink` now checks nontrivial assignment (cherry picked from commit e958f4a)
1 parent 5d3d8b5 commit 5724c68

File tree

4 files changed

+33
-9
lines changed

4 files changed

+33
-9
lines changed

compiler/injectdestructors.nim

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -163,9 +163,13 @@ proc isLastReadImpl(n: PNode; c: var Con; scope: var Scope): bool =
163163
else:
164164
result = false
165165

166+
template hasDestructorOrAsgn(c: var Con, typ: PType): bool =
167+
# bug #23354; an object type could have a non-trivial assignements when it is passed to a sink parameter
168+
hasDestructor(c, typ) or (c.graph.config.selectedGC in {gcArc, gcOrc, gcAtomicArc} and
169+
typ.kind == tyObject and not isTrivial(getAttachedOp(c.graph, typ, attachedAsgn)))
170+
166171
proc isLastRead(n: PNode; c: var Con; s: var Scope): bool =
167-
# bug #23354; an object type could have a non-trival assignements when it is passed to a sink parameter
168-
if not hasDestructor(c, n.typ) and (n.typ.kind != tyObject or isTrival(getAttachedOp(c.graph, n.typ, attachedAsgn))): return true
172+
if not hasDestructorOrAsgn(c, n.typ): return true
169173

170174
let m = skipConvDfa(n)
171175
result = isLastReadImpl(n, c, s)
@@ -456,7 +460,7 @@ proc passCopyToSink(n: PNode; c: var Con; s: var Scope): PNode =
456460
result = newNodeIT(nkStmtListExpr, n.info, n.typ)
457461
let nTyp = n.typ.skipTypes(tyUserTypeClasses)
458462
let tmp = c.getTemp(s, nTyp, n.info)
459-
if hasDestructor(c, nTyp):
463+
if hasDestructorOrAsgn(c, nTyp):
460464
let typ = nTyp.skipTypes({tyGenericInst, tyAlias, tySink})
461465
let op = getAttachedOp(c.graph, typ, attachedDup)
462466
if op != nil and tfHasOwned notin typ.flags:

compiler/liftdestructors.nim

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1286,7 +1286,7 @@ proc inst(g: ModuleGraph; c: PContext; t: PType; kind: TTypeAttachedOp; idgen: I
12861286
else:
12871287
localError(g.config, info, "unresolved generic parameter")
12881288

1289-
proc isTrival*(s: PSym): bool {.inline.} =
1289+
proc isTrivial*(s: PSym): bool {.inline.} =
12901290
s == nil or (s.ast != nil and s.ast[bodyPos].len == 0)
12911291

12921292
proc createTypeBoundOps(g: ModuleGraph; c: PContext; orig: PType; info: TLineInfo;
@@ -1341,8 +1341,8 @@ proc createTypeBoundOps(g: ModuleGraph; c: PContext; orig: PType; info: TLineInf
13411341
if canon != orig:
13421342
setAttachedOp(g, idgen.module, orig, k, getAttachedOp(g, canon, k))
13431343

1344-
if not isTrival(getAttachedOp(g, orig, attachedDestructor)):
1345-
#or not isTrival(orig.assignment) or
1346-
# not isTrival(orig.sink):
1344+
if not isTrivial(getAttachedOp(g, orig, attachedDestructor)):
1345+
#or not isTrivial(orig.assignment) or
1346+
# not isTrivial(orig.sink):
13471347
orig.flags.incl tfHasAsgn
13481348
# ^ XXX Breaks IC!

compiler/sempass2.nim

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -706,7 +706,7 @@ proc isNoEffectList(n: PNode): bool {.inline.} =
706706
assert n.kind == nkEffectList
707707
n.len == 0 or (n[tagEffects] == nil and n[exceptionEffects] == nil and n[forbiddenEffects] == nil)
708708

709-
proc isTrival(caller: PNode): bool {.inline.} =
709+
proc isTrivial(caller: PNode): bool {.inline.} =
710710
result = caller.kind == nkSym and caller.sym.magic in {mEqProc, mIsNil, mMove, mWasMoved, mSwap}
711711

712712
proc trackOperandForIndirectCall(tracked: PEffects, n: PNode, formals: PType; argIndex: int; caller: PNode) =
@@ -715,7 +715,7 @@ proc trackOperandForIndirectCall(tracked: PEffects, n: PNode, formals: PType; ar
715715
let param = if formals != nil and formals.n != nil and argIndex < formals.n.len: formals.n[argIndex].sym else: nil
716716
# assume indirect calls are taken here:
717717
if op != nil and op.kind == tyProc and n.skipConv.kind != nkNilLit and
718-
not isTrival(caller) and
718+
not isTrivial(caller) and
719719
((param != nil and sfEffectsDelayed in param.flags) or laxEffects in tracked.c.config.legacyFeatures):
720720

721721
internalAssert tracked.config, op.n[0].kind == nkEffectList

tests/arc/t24760.nim

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
discard """
2+
matrix: "--mm:orc"
3+
errormsg: "=dup' is not available for type <B>, which is inferred from unavailable '=copy'; requires a copy because it's not the last read of 'b'; another read is done here: t24760.nim(19, 8); routine: g"
4+
"""
5+
6+
type
7+
A {.inheritable.} = object
8+
B = object of A
9+
10+
proc `=copy`(a: var A, x: A) {.error.}
11+
#proc `=copy`(a: var B, x: B) {.error.}
12+
13+
proc ffff(v: sink B) =
14+
echo v
15+
16+
proc g() =
17+
var b: B
18+
ffff(b)
19+
ffff(b)
20+
g()

0 commit comments

Comments
 (0)