From c08f9311bf52bab5f94539663a05e091e55e3e24 Mon Sep 17 00:00:00 2001 From: ringabout <43030857+ringabout@users.noreply.github.com> Date: Tue, 25 Feb 2025 20:14:13 +0800 Subject: [PATCH 1/4] fixes #24720; std lib iterators unnecessarily require value copies --- lib/pure/collections/tables.nim | 12 ++++++------ tests/arc/t24720.nim | 20 ++++++++++++++++++++ 2 files changed, 26 insertions(+), 6 deletions(-) create mode 100644 tests/arc/t24720.nim diff --git a/lib/pure/collections/tables.nim b/lib/pure/collections/tables.nim index 082e1e96f5591..dc123d25d5873 100644 --- a/lib/pure/collections/tables.nim +++ b/lib/pure/collections/tables.nim @@ -677,7 +677,7 @@ template withValue*[A, B](t: var Table[A, B], key: A, body2 -iterator pairs*[A, B](t: Table[A, B]): (A, B) = +iterator pairs*[A, B](t: Table[A, B]): (lent A, lent B) = ## Iterates over any `(key, value)` pair in the table `t`. ## ## See also: @@ -1139,7 +1139,7 @@ proc `==`*[A, B](s, t: TableRef[A, B]): bool = -iterator pairs*[A, B](t: TableRef[A, B]): (A, B) = +iterator pairs*[A, B](t: TableRef[A, B]): (lent A, lent B) = ## Iterates over any `(key, value)` pair in the table `t`. ## ## See also: @@ -1727,7 +1727,7 @@ proc `==`*[A, B](s, t: OrderedTable[A, B]): bool = -iterator pairs*[A, B](t: OrderedTable[A, B]): (A, B) = +iterator pairs*[A, B](t: OrderedTable[A, B]): (lent A, lent B) = ## Iterates over any `(key, value)` pair in the table `t` in insertion ## order. ## @@ -2150,7 +2150,7 @@ proc `==`*[A, B](s, t: OrderedTableRef[A, B]): bool = -iterator pairs*[A, B](t: OrderedTableRef[A, B]): (A, B) = +iterator pairs*[A, B](t: OrderedTableRef[A, B]): (lent A, lent B) = ## Iterates over any `(key, value)` pair in the table `t` in insertion ## order. ## @@ -2564,7 +2564,7 @@ proc `==`*[A](s, t: CountTable[A]): bool = equalsImpl(s, t) -iterator pairs*[A](t: CountTable[A]): (A, int) = +iterator pairs*[A](t: CountTable[A]): (lent A, int) = ## Iterates over any `(key, value)` pair in the table `t`. ## ## See also: @@ -2845,7 +2845,7 @@ proc `==`*[A](s, t: CountTableRef[A]): bool = else: result = s[] == t[] -iterator pairs*[A](t: CountTableRef[A]): (A, int) = +iterator pairs*[A](t: CountTableRef[A]): (lent A, int) = ## Iterates over any `(key, value)` pair in the table `t`. ## ## See also: diff --git a/tests/arc/t24720.nim b/tests/arc/t24720.nim new file mode 100644 index 0000000000000..9f4eb2f698e61 --- /dev/null +++ b/tests/arc/t24720.nim @@ -0,0 +1,20 @@ +discard """ + matrix: "--mm:orc" + output: ''' +found entry +''' +""" + +import std/tables +type NoCopies = object + +proc `=copy`(a: var NoCopies, b: NoCopies) {.error.} + +# bug #24720 +proc foo() = + var t: Table[int, NoCopies] + t[3] = NoCopies() # only moves + for k, v in t.pairs(): # lent values, no need to copy! + echo "found entry" + +foo() From 25493791761490670c36fd0c6ca812f42dc97928 Mon Sep 17 00:00:00 2001 From: ringabout <43030857+ringabout@users.noreply.github.com> Date: Mon, 26 May 2025 22:56:49 +0800 Subject: [PATCH 2/4] fixes `(lent T, lent T)` for iterators --- compiler/semstmts.nim | 2 +- compiler/semtypes.nim | 20 ++++++++++++++++++ compiler/transf.nim | 48 +++++++++++++++++++++++++++++++++++++++++-- 3 files changed, 67 insertions(+), 3 deletions(-) diff --git a/compiler/semstmts.nim b/compiler/semstmts.nim index ce8b59f9cd3e3..af4384b998891 100644 --- a/compiler/semstmts.nim +++ b/compiler/semstmts.nim @@ -1131,7 +1131,7 @@ proc semForVars(c: PContext, n: PNode; flags: TExprFlags): PNode = # BUGFIX: don't use `iter` here as that would strip away # the ``tyGenericInst``! See ``tests/compile/tgeneric.nim`` # for an example: - v.typ = iterBase + v.typ = makeIterTupleType(c, iterBase) n[0] = newSymNode(v) if sfGenSym notin v.flags and not isDiscardUnderscore(v): addDecl(c, v) elif v.owner == nil: setOwner(v, getCurrOwner(c)) diff --git a/compiler/semtypes.nim b/compiler/semtypes.nim index b3839ef6333e7..762ef27c00387 100644 --- a/compiler/semtypes.nim +++ b/compiler/semtypes.nim @@ -1783,6 +1783,23 @@ proc maybeAliasType(c: PContext; typeExpr, prev: PType): PType = else: result = nil +proc makeIterTupleType(c: PContext; typ: PType): PType = + if typ.kind == tyTuple: + var hasView = false + result = newTypeS(tyTuple, c) + + for i in 0.. Date: Mon, 26 May 2025 23:04:05 +0800 Subject: [PATCH 3/4] fixes --- compiler/transf.nim | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/transf.nim b/compiler/transf.nim index b56dd90f8ef0b..98371579c2fb1 100644 --- a/compiler/transf.nim +++ b/compiler/transf.nim @@ -406,7 +406,7 @@ proc makeTupleUnpack(c: PTransf; lhs: PNode; rhs: PNode): PNode = var field: PNode = nil if rhs.typ[i].kind in {tyVar, tyLent}: let tupleType = newTupleAccessRaw(tempNode, i) - tupleType.typ = rhs.typ[i] + tupleType.typ() = rhs.typ[i] field = newDeref(tupleType) else: field = newTupleAccessRaw(tempNode, i) From 934cf4caca2fb95324cb74dc715a2b51ff154fba Mon Sep 17 00:00:00 2001 From: ringabout <43030857+ringabout@users.noreply.github.com> Date: Mon, 26 May 2025 23:09:22 +0800 Subject: [PATCH 4/4] oops --- compiler/transf.nim | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/transf.nim b/compiler/transf.nim index 98371579c2fb1..74139a0355064 100644 --- a/compiler/transf.nim +++ b/compiler/transf.nim @@ -411,7 +411,7 @@ proc makeTupleUnpack(c: PTransf; lhs: PNode; rhs: PNode): PNode = else: field = newTupleAccessRaw(tempNode, i) - field.typ = rhs.typ[i].skipTypes({tyVar, tyLent}) + field.typ() = rhs.typ[i].skipTypes({tyVar, tyLent}) tupleConstr.add field