fixes #24720; std lib iterators unnecessarily require value copies

compiler/semmagic.nim

@@ -65,7 +65,7 @@ proc semTypeOf(c: PContext; n: PNode): PNode =
       t.incl tfNonConstExpr
     else:
       t = base
-  result.typ = makeTypeDesc(c, t)
+  result.typ = makeTypeDesc(c, decayTypeOfView(c, t))
 
 type
   SemAsgnMode = enum asgnNormal, noOverloadedSubscript, noOverloadedAsgn

compiler/semstmts.nim

@@ -815,6 +815,14 @@ proc makeVarTupleSection(c: PContext, n, a, def: PNode, typ: PType, symkind: TSy
       lastDef[^1] = val
     result.add(lastDef)
 
+proc materializeDirectView(n: PNode): PNode =
+  let t = n.typ.skipTypes({tyGenericInst, tyAlias, tySink, tyOwned})
+  if t.kind in {tyVar, tyLent}:
+    result = newNodeIT(nkHiddenDeref, n.info, t.elementType)
+    result.add n
+  else:
+    result = n
+
 proc semVarOrLet(c: PContext, n: PNode, symkind: TSymKind): PNode =
   var b: PNode
   result = copyNode(n)
@@ -881,6 +889,10 @@ proc semVarOrLet(c: PContext, n: PNode, symkind: TSymKind): PNode =
           #changeType(def.skipConv, typ, check=true)
       else:
         typ = def.typ.skipTypes({tyStatic, tySink}).skipIntLit(c.idgen)
+        let directTyp = typ.skipTypes({tyGenericInst, tyAlias, tySink, tyOwned})
+        if directTyp.kind in {tyVar, tyLent}:
+          def = materializeDirectView(def)
+          typ = def.typ.skipTypes({tyStatic, tySink}).skipIntLit(c.idgen)
         if typ.kind in tyUserTypeClasses and typ.isResolvedUserTypeClass:
           typ = typ.last
         if hasEmpty(typ):
@@ -908,7 +920,8 @@ proc semVarOrLet(c: PContext, n: PNode, symkind: TSymKind): PNode =
 
     if c.matchedConcept != nil:
       typFlags.incl taConcept
-    typeAllowedCheck(c, a.info, typ, symkind, typFlags)
+    if a.kind != nkVarTuple:
+      typeAllowedCheck(c, a.info, typ, symkind, typFlags)
 
     var tup = skipTypes(typ, {tyGenericInst, tyAlias, tySink})
     if a.kind == nkVarTuple:

compiler/semtypes.nim

@@ -1879,6 +1879,38 @@ proc fixupTypeOf(c: PContext, prev: PType, typ: PType) =
     if prev.kind != tyGenericBody:
       assignType(prev, result)
 
+proc decayTypeOfView(c: PContext, typ: PType): PType =
+  if typ == nil: return nil
+  let t = typ.skipTypes({tyGenericInst, tyAlias, tySink})
+  case t.kind
+  of tyVar, tyLent:
+    result = decayTypeOfView(c, t.elementType)
+  of tyTuple:
+    var changed = false
+    var kids = newSeq[PType](t.len)
+    for i in 0..<t.len:
+      kids[i] = decayTypeOfView(c, t[i])
+      if kids[i] != t[i]: changed = true
+    if changed:
+      result = copyType(t, c.idgen, t.owner)
+      for i in 0..<kids.len:
+        result[i] = kids[i]
+      if t.n != nil:
+        result.n = copyNode(t.n)
+        for it in t.n:
+          if it.kind == nkSym and it.sym.kind == skField:
+            let field = copySym(it.sym, c.idgen)
+            field.ast = it.sym.ast
+            if field.position >= 0 and field.position < kids.len:
+              field.typ = kids[field.position]
+            result.n.add newSymNode(field, it.info)
+          else:
+            result.n.add copyTree(it)
+    else:
+      result = typ
+  else:
+    result = typ
+
 proc semTypeExpr(c: PContext, n: PNode; prev: PType): PType =
   var n = semExprWithType(c, n, {efDetermineType})
   if n.typ.kind == tyTypeDesc:
@@ -2078,6 +2110,7 @@ proc semTypeOf(c: PContext; n: PNode; prev: PType): PType =
       result.incl tfNonConstExpr
     else:
       result = base
+  result = decayTypeOfView(c, result)
   fixupTypeOf(c, prev, result)
 
 proc semTypeOf2(c: PContext; n: PNode; prev: PType): PType =
@@ -2103,6 +2136,7 @@ proc semTypeOf2(c: PContext; n: PNode; prev: PType): PType =
       result.incl tfNonConstExpr
     else:
       result = base
+  result = decayTypeOfView(c, result)
   fixupTypeOf(c, prev, result)
 
 proc semTypeIdent(c: PContext, n: PNode): PSym =

compiler/sigmatch.nim

@@ -15,6 +15,8 @@ import
   magicsys, idents, lexer, options, parampatterns, trees,
   linter, lineinfos, lowerings, modulegraphs, concepts, layeredtable
 
+import typeallowed
+
 import std/[intsets, strutils, tables]
 
 when defined(nimPreviewSlimSystem):
@@ -123,6 +125,58 @@ proc initCandidate*(ctx: PContext, callee: PType): TCandidate =
   result.calleeSym = nil
   result.bindings = initLayeredTypeMap()
 
+proc materializeTupleViewType(t: PType; idgen: IdGenerator): PType =
+  case t.kind
+  of tyVar, tyLent:
+    result = materializeTupleViewType(t.elementType, idgen)
+  of tyTuple:
+    if classifyViewType(t) == noView:
+      result = t
+    else:
+      result = copyType(t, idgen, t.owner)
+      for i in 0..<t.len:
+        result[i] = materializeTupleViewType(t[i], idgen)
+      if t.n != nil:
+        result.n = copyNode(t.n)
+        for it in t.n:
+          if it.kind == nkSym and it.sym.kind == skField:
+            let field = copySym(it.sym, idgen)
+            field.ast = it.sym.ast
+            if field.position >= 0 and field.position < result.len:
+              field.typ = result[field.position]
+            result.n.add newSymNode(field, it.info)
+          else:
+            result.n.add copyTree(it)
+  else:
+    result = t
+
+proc materializeTupleViewArg(c: PContext; targetType: PType; arg: PNode): PNode =
+  let targetTuple = targetType.skipTypes({tyGenericInst, tyAlias, tySink, tyDistinct, tyInferred})
+  var tupleArg = arg
+  var prefix: PNode = nil
+  if targetTuple.len > 1 and arg.kind notin {nkHiddenAddr, nkSym}:
+    prefix = evalOnce(c.graph, arg, c.idgen, getCurrOwner(c))
+    tupleArg = prefix[^1]
+
+  let tupleConstr = newNodeIT(nkTupleConstr, arg.info, targetType)
+  for i in 0..<targetTuple.len:
+    let targetField = targetTuple[i]
+    var field = newTupleAccess(c.graph, tupleArg, i)
+    let sourceField = field.typ.skipTypes({tyGenericInst, tyAlias, tySink, tyDistinct, tyInferred})
+    if sourceField.kind in {tyVar, tyLent}:
+      field = newDeref(field)
+    elif targetField.kind == tyTuple and classifyViewType(sourceField) != noView:
+      field = materializeTupleViewArg(c, targetField, field)
+    tupleConstr.add field
+
+  if prefix == nil:
+    result = tupleConstr
+  else:
+    result = newNodeIT(nkStmtListExpr, arg.info, targetType)
+    for i in 0..<(prefix.len - 1):
+      result.add prefix[i]
+    result.add tupleConstr
+
 proc put(c: var TCandidate, key, val: PType) {.inline.} =
   ## Given: proc foo[T](x: T); foo(4)
   ## key: 'T'
@@ -135,7 +189,12 @@ proc put(c: var TCandidate, key, val: PType) {.inline.} =
         writeStackTrace()
     if c.c.module.name.s == "temp3":
       echo "binding ", key, " -> ", val
-  put(c.bindings, key, val.skipIntLit(c.c.idgen))
+
+  let normalized = val.skipIntLit(c.c.idgen)
+  if normalized.kind == tyTuple and classifyViewType(normalized) != noView:
+    put(c.bindings, key, materializeTupleViewType(normalized, c.c.idgen))
+  else:
+    put(c.bindings, key, normalized)
 
 proc typeRel*(c: var TCandidate, f, aOrig: PType,
               flags: TTypeRelFlags = {}): TTypeRelation
@@ -2200,7 +2259,16 @@ proc implicitConv(kind: TNodeKind, f: PType, arg: PNode, m: TCandidate,
 
   if result.typ == nil: internalError(c.graph.config, arg.info, "implicitConv")
   result.add c.graph.emptyNode
-  if arg.typ != nil and arg.typ.kind == tyLent:
+  let targetTuple = result.typ.skipTypes({tyVar, tyGenericInst, tyAlias, tySink, tyDistinct, tyInferred})
+  let sourceTuple =
+    if arg.typ != nil:
+      arg.typ.skipTypes({tyGenericInst, tyAlias, tySink, tyDistinct, tyInferred})
+    else:
+      nil
+  if sourceTuple != nil and sourceTuple.kind == tyTuple and targetTuple.kind == tyTuple and
+      classifyViewType(arg.typ) != noView and classifyViewType(result.typ) == noView:
+    result.add materializeTupleViewArg(c, targetTuple, arg)
+  elif arg.typ != nil and arg.typ.kind == tyLent:
     let a = newNodeIT(nkHiddenDeref, arg.info, arg.typ.elementType)
     a.add arg
     result.add a

lib/pure/collections/tables.nim

@@ -739,7 +739,7 @@ template withValue*[A, B](t: Table[A, B], key: A,
     discard
 
 
-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:
@@ -1201,7 +1201,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:
@@ -1789,7 +1789,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.
   ##
@@ -2212,7 +2212,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.
   ##
@@ -2622,7 +2622,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:
@@ -2899,7 +2899,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:

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