From c8e6b059a4378f75569e3dd1a8ae356b1f1a574a Mon Sep 17 00:00:00 2001 From: ringabout <43030857+ringabout@users.noreply.github.com> Date: Thu, 9 Apr 2026 17:07:04 +0800 Subject: [PATCH 1/4] optimizes `setLen` for orc; disabling overflow checks (#25722) ref https://github.com/nim-lang/Nim/issues/25695 ref https://github.com/nim-lang/Nim/pull/25715 This pull request introduces a minor but important change to the `setLen` procedure in `lib/system/seqs_v2.nim`. The main update is the temporary disabling of overflow checks during the initialization loop when extending the sequence length, which can improve performance and avoid unnecessary checks during this operation. Memory and performance improvement: * Disabled overflow checks for the loop that initializes new elements to their default value when increasing the length of a sequence in `setLen`, by wrapping the loop with `{.push overflowChecks: off.}` and `{.pop.}`. --- lib/system/seqs_v2.nim | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/system/seqs_v2.nim b/lib/system/seqs_v2.nim index fefb6e914c..511bb87d82 100644 --- a/lib/system/seqs_v2.nim +++ b/lib/system/seqs_v2.nim @@ -262,8 +262,11 @@ proc setLen[T](s: var seq[T], newlen: Natural) {.nodestroy.} = if xu.p == nil or (xu.p.cap and not strlitFlag) < newlen: xu.p = cast[typeof(xu.p)](prepareSeqAddUninit(oldLen, xu.p, newlen - oldLen, sizeof(T), alignof(T))) xu.len = newlen + + {.push overflowChecks: off.} for i in oldLen.. Date: Thu, 9 Apr 2026 17:08:03 +0800 Subject: [PATCH 2/4] fixes #25697; {.borrow.} on iterator for distinct seq triggers internal error (#25709) fixes #25697 This pull request improves the handling of borrowed routines in the compiler transformation phase, making the code more robust and maintainable. The main change is the introduction of a helper function to properly resolve borrowed routine symbols, which is then used in multiple places to ensure correct symbol resolution. Additionally, a new test case is added to cover a previously reported bug related to borrowed iterators on distinct types. **Compiler improvements:** * Added `resolveBorrowedRoutineSym` helper function to follow borrow aliases and retrieve the underlying implementation symbol for borrowed routines. This centralizes and clarifies the logic for resolving borrowed symbols. * Updated `transformSymAux` and `transformFor` to use the new helper function, replacing duplicated logic and improving correctness when handling borrowed routines. [[1]](diffhunk://#diff-c7b80f51fb685eb22c5b56ee2f320d6c708706f3ae7293478ecd104a2b5b8096L139-R154) [[2]](diffhunk://#diff-c7b80f51fb685eb22c5b56ee2f320d6c708706f3ae7293478ecd104a2b5b8096L788-R795) **Testing:** * Added a test case for bug #25697 to `tests/distinct/tborrow.nim`, ensuring that iteration over a distinct type with a borrowed iterator works as expected. --- compiler/transf.nim | 34 ++++++++++++++++++++++------------ tests/distinct/tborrow.nim | 11 +++++++++++ 2 files changed, 33 insertions(+), 12 deletions(-) diff --git a/compiler/transf.nim b/compiler/transf.nim index 049ed4fa5b..e85ecd3e07 100644 --- a/compiler/transf.nim +++ b/compiler/transf.nim @@ -118,6 +118,24 @@ proc newAsgnStmt(c: PTransf, kind: TNodeKind, le: PNode, ri: PNode; isFirstWrite le.flags.incl nfFirstWrite result[1] = ri +proc resolveBorrowedRoutineSym(c: PTransf; s: PSym; info: TLineInfo): PSym = + # Follow borrow aliases to the underlying implementation symbol. + var s = s + while true: + # Skips over all borrowed procs getting the last proc symbol without an implementation + let body = getBody(c.graph, s) + if body.kind == nkSym and sfBorrow in body.sym.flags and getBody(c.graph, body.sym).kind == nkSym: + s = body.sym + else: + break + + let body = getBody(c.graph, s) + if body.kind == nkSym: + result = body.sym + else: + result = nil + internalError(c.graph.config, info, "wrong AST for borrowed symbol") + proc transformSymAux(c: PTransf, n: PNode): PNode = let s = n.sym if s.typ != nil and s.typ.callConv == ccClosure: @@ -136,17 +154,7 @@ proc transformSymAux(c: PTransf, n: PNode): PNode = var tc = c.transCon if sfBorrow in s.flags and s.kind in routineKinds: # simply exchange the symbol: - var s = s - while true: - # Skips over all borrowed procs getting the last proc symbol without an implementation - let body = getBody(c.graph, s) - if body.kind == nkSym and sfBorrow in body.sym.flags and getBody(c.graph, body.sym).kind == nkSym: - s = body.sym - else: - break - b = getBody(c.graph, s) - if b.kind != nkSym: internalError(c.graph.config, n.info, "wrong AST for borrowed symbol") - b = newSymNode(b.sym, n.info) + b = newSymNode(resolveBorrowedRoutineSym(c, s, n.info), n.info) elif c.inlining > 0: # see bug #13596: we use ref-based equality in the DFA for destruction # injections so we need to ensure unique nodes after iterator inlining @@ -785,7 +793,9 @@ proc transformFor(c: PTransf, n: PNode): PNode = discard c.breakSyms.pop - let iter = call[0].sym + var iter = call[0].sym + if sfBorrow in iter.flags and iter.kind in routineKinds: + iter = resolveBorrowedRoutineSym(c, iter, n.info) var v = newNodeI(nkVarSection, n.info) for i in 0.. Date: Thu, 9 Apr 2026 05:09:34 -0400 Subject: [PATCH 3/4] fix #25627 (#25700) @demotomohiro this was caused by your PR please review #25627 --- compiler/semtypes.nim | 31 ++++++++++++++++++++++++++----- tests/objects/t25627.nim | 22 ++++++++++++++++++++++ 2 files changed, 48 insertions(+), 5 deletions(-) create mode 100644 tests/objects/t25627.nim diff --git a/compiler/semtypes.nim b/compiler/semtypes.nim index e2f91587ff..93be5d56de 100644 --- a/compiler/semtypes.nim +++ b/compiler/semtypes.nim @@ -1693,6 +1693,22 @@ proc containsGenericInvocationWithForward(n: PNode): bool = return true return false +proc containsRecWhen(n: PNode): bool = + if n == nil: + return false + case n.kind + of nkRecWhen: + return true + else: + for i in 0.. 2 and + containsRecWhen(n.sym.ast[2]) + proc semGeneric(c: PContext, n: PNode, s: PSym, prev: PType): PType = if s.typ == nil: localError(c.config, n.info, "cannot instantiate the '$1' $2" % @@ -1772,11 +1788,16 @@ proc semGeneric(c: PContext, n: PNode, s: PSym, prev: PType): PType = # XXX: What kind of error is this? is it still relevant? localError(c.config, n.info, errCannotInstantiateX % s.name.s) result = newOrPrevType(tyError, prev, c) - elif containsGenericInvocationWithForward(n[0]) or hasForwardTypeParam: - # isConcrete == false means this generic type is not instanciated here because it invoked with generic parameters. - # Even if isConcrete == true, don't instanciate it now if there are any `tyForward` type params. - # Such `tyForward` type params will be semchecked later and we can instanciate this next time. - # Some generic types like std/options.Option[T] needs a type kinds of the given type argument. + elif containsGenericInvocationWithForward(n[0]) or + (hasForwardTypeParam and requiresForwardTypeDelay(n[0])): + # isConcrete == false means this generic type is not instanciated here because + # it invoked with generic parameters. + # Even if isConcrete == true, don't instanciate it now if the type + # shape depends on unresolved `tyForward` type params. + # Such `tyForward` type params will be semchecked later and we can + # instanciate this next time. + # Some generic types like std/options.Option[T] need the kind of the + # given type argument before their fields can be resolved. # return `tyForward` instead of `tyGenericInvocation` because: # ```nim diff --git a/tests/objects/t25627.nim b/tests/objects/t25627.nim new file mode 100644 index 0000000000..57fa0ceb2c --- /dev/null +++ b/tests/objects/t25627.nim @@ -0,0 +1,22 @@ +# issue #25627 + +import std/tables + +type + FsoKind = enum + fsoFile + fsoDir + fsoLink + + FakeFso = ref object + kind: FsoKind + dirName: string + files: OrderedTable[string, FakeFso] + + DirStruct = object + root = FakeFso(kind: fsoDir, dirName: "/") + +let dir = DirStruct() +doAssert dir.root.kind == fsoDir +doAssert dir.root.dirName == "/" +doAssert dir.root.files.len == 0 From 188aa1714e0bd95caca408265d49c720ae739449 Mon Sep 17 00:00:00 2001 From: ringabout <43030857+ringabout@users.noreply.github.com> Date: Thu, 9 Apr 2026 17:11:06 +0800 Subject: [PATCH 4/4] fixes #25719; optimizes setLenSeqCall for orc (#25721) fixes #25719 This pull request updates the logic for resizing sequences during certain copy operations in the `compiler/liftdestructors.nim` file. The main improvement is that the code now distinguishes between regular and uninitialized resizing based on whether the sequence's element type supports bulk memory copying, which can lead to more efficient code generation. **Improvements to sequence resizing and copying logic:** * Modified `setLenSeqCall` to accept a `noinit` parameter, allowing it to choose between `setLen` and `setLenUninit` operations, and to select the appropriate magic for each case. * Updated `fillSeqOp` to determine if bulk memory copy is supported and, if so, call `setLenSeqCall` with `noinit = true` and perform a bulk copy; otherwise, it defaults to element-wise copying. This logic is now applied in both relevant locations in the function. [[1]](diffhunk://#diff-456118dde9a4e21f1b351fd72504d62fc16e9c30354dbb9a3efcb95a29067863L646-R650) [[2]](diffhunk://#diff-456118dde9a4e21f1b351fd72504d62fc16e9c30354dbb9a3efcb95a29067863L661-R666) --- compiler/liftdestructors.nim | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/compiler/liftdestructors.nim b/compiler/liftdestructors.nim index a2f3c94cde..15c60363f8 100644 --- a/compiler/liftdestructors.nim +++ b/compiler/liftdestructors.nim @@ -592,10 +592,12 @@ proc setLenStrCall(c: var TLiftCtx; x, y: PNode): PNode = result = genBuiltin(c, mSetLengthStr, "setLen", x) # genAddr(g, x)) result.add lenCall -proc setLenSeqCall(c: var TLiftCtx; t: PType; x, y: PNode): PNode = +proc setLenSeqCall(c: var TLiftCtx; t: PType; x, y: PNode; noinit = false): PNode = let lenCall = genBuiltin(c, mLengthSeq, "len", y) lenCall.typ = getSysType(c.g, x.info, tyInt) - var op = getSysMagic(c.g, x.info, "setLen", mSetLengthSeq) + let name = if noinit: "setLenUninit" else: "setLen" + let magic = if noinit: mSetLengthSeqUninit else: mSetLengthSeq + var op = getSysMagic(c.g, x.info, name, magic) op = instantiateGeneric(c, op, t, t) result = newTree(nkCall, newSymNode(op, x.info), x, lenCall) @@ -643,8 +645,9 @@ proc genBulkCopySeq(c: var TLiftCtx; t: PType; body, x, y: PNode) = proc fillSeqOp(c: var TLiftCtx; t: PType; body, x, y: PNode) = case c.kind of attachedDup: - body.add setLenSeqCall(c, t, x, y) - if supportsCopyMem(t.elementType): + let bulkCopy = supportsCopyMem(t.elementType) + body.add setLenSeqCall(c, t, x, y, noinit = bulkCopy) + if bulkCopy: genBulkCopySeq(c, t, body, x, y) else: forallElements(c, t, body, x, y) @@ -658,8 +661,9 @@ proc fillSeqOp(c: var TLiftCtx; t: PType; body, x, y: PNode) = # This is usually more efficient than a destroy/create pair. # For trivially copyable types, use bulk copyMem instead of element loop. checkSelfAssignment(c, t, body, x, y) - body.add setLenSeqCall(c, t, x, y) - if supportsCopyMem(t.elementType): + let bulkCopy = supportsCopyMem(t.elementType) + body.add setLenSeqCall(c, t, x, y, noinit = bulkCopy) + if bulkCopy: genBulkCopySeq(c, t, body, x, y) else: forallElements(c, t, body, x, y)