From bd95f88f7412b80b15bc0d667f39169e6f8fc05f Mon Sep 17 00:00:00 2001 From: Zoom Date: Wed, 26 Aug 2026 20:01:06 +0400 Subject: [PATCH] js: fix `var openArray` write-through for `toOpenArray` (#26086) In the JS backend `toOpenArray` used `slice` (a copy), so writes through a `var openArray` parameter silently vanished. This emits `subarray` (a live shared-buffer view) for homogeneous numeric arrays, otherwise such parameters are passed as a `{base, off, len}` view that always aliases the caller's storage. Sliced seq/array args become `{base, off, len}`, whole values `{base, off:0, len}`, re-slices rebase. Un-skips the JS guard in tests/openarray/topenarray.nim Fixes #15952. --- changelog.md | 7 +++ compiler/jsgen.nim | 104 +++++++++++++++++++++++++++++++-- tests/openarray/topenarray.nim | 21 ++++++- 3 files changed, 126 insertions(+), 6 deletions(-) diff --git a/changelog.md b/changelog.md index d6561b42d2..c38267aa3e 100644 --- a/changelog.md +++ b/changelog.md @@ -150,6 +150,13 @@ parameter and result types, not just their source-level shape. Use The issue was that `hasValuelessStatics` in `semtypinst.nim` didn't recognize `tyTypeDesc(tyGenericParam)` as an unresolved generic parameter. +- The JS backend now implements write-through for `var openArray` parameters that + receive a `toOpenArray` view (bug #15952): mutations reach the caller's storage + instead of silently writing to a copy. Fixed homogeneous numeric arrays + (`array[N, T]`, JS typed arrays) slice via `subarray`; `seq` and non-numeric + arrays slice via a `{base, off, len}` view. This also covers seq/non-numeric-array + write-through, pass-through, re-slicing and `@` (openArray-to-seq) of such views. + ## Tool changes - Added `--raw` flag when generating JSON docs to not render markup. diff --git a/compiler/jsgen.nim b/compiler/jsgen.nim index 963c147fac..5decd63ef6 100644 --- a/compiler/jsgen.nim +++ b/compiler/jsgen.nim @@ -1450,6 +1450,20 @@ proc genCheckedFieldOp(p: PProc, n: PNode, addrTyp: PType, r: var TCompRes) = r.res = "$1.$2" % [tmp, field.loc.snippet] r.kind = resExpr +proc isVarOpenArrayParam(n: PNode): bool = + ## True if `n` resolves to a `var openArray` parameter. The JS backend + ## represents such parameters as a `{base, off, len}` slice view so that + ## writes through a `toOpenArray` view reach the caller's storage (bug #15952). + var it = n + while true: + case it.kind + of nkHiddenDeref, nkDerefExpr, nkHiddenAddr, nkAddr: it = it[0] + of nkHiddenStdConv, nkConv, nkObjDownConv, nkObjUpConv: it = it[1] + else: break + result = it.kind == nkSym and it.sym.kind == skParam and + it.sym.typ != nil and it.sym.typ.kind == tyVar and + it.sym.typ.len > 0 and it.sym.typ[0].kind == tyOpenArray + proc genArrayAddr(p: PProc, n: PNode, r: var TCompRes) = var a, b: TCompRes = default(TCompRes) @@ -1458,6 +1472,19 @@ proc genArrayAddr(p: PProc, n: PNode, r: var TCompRes) = let m = if n.kind == nkHiddenAddr: n[0] else: n gen(p, m[0], a) gen(p, m[1], b) + if isVarOpenArrayParam(m[0]): + # `var openArray` param is a `{base, off, len}` view; index the base with + # the offset applied. `m[0]` is a plain param name, safe to reference + # repeatedly (no side effects, so no temp needed). + let pn = a.rdLoc + r.address = "($1).base" % [pn] + if optBoundsCheck in p.options: + useMagic(p, "chckIndx") + r.res = "($1).off + chckIndx($2, 0, ($1).len - 1)" % [pn, b.rdLoc] + else: + r.res = "($1).off + ($2)" % [pn, b.rdLoc] + r.kind = resExpr + return #internalAssert p.config, a.typ != etyBaseIndex and b.typ != etyBaseIndex let (x, tmp) = maybeMakeTemp(p, m[0], a) r.address = x @@ -1726,8 +1753,47 @@ proc genArgNoParam(p: PProc, n: PNode, r: var TCompRes) = else: r.res.add(a.res) +proc genVarOpenArrayArg(p: PProc, n: PNode, r: var TCompRes) = + ## Emit a `{base, off, len}` slice view for an argument to a `var openArray` + ## parameter (bug #15952). The view always aliases the base storage, so writes + ## through the callee's `openArray` reach the caller's array/seq/typed array. + var b, lo, hi, v: TCompRes = default(TCompRes) + # the argument reaches codegen as `addr(toOpenArray(x, lo, hi))` (possibly + # under conversions); unwrap to the actual `toOpenArray` call. + var sl = n + while true: + case sl.kind + of nkHiddenAddr, nkAddr, nkHiddenDeref, nkDerefExpr: sl = sl[0] + of nkHiddenStdConv, nkConv, nkObjDownConv, nkObjUpConv: sl = sl[1] + else: break + if sl.kind in nkCallKinds and getMagic(sl) == mSlice: + gen(p, sl[1], b) + gen(p, sl[2], lo) + gen(p, sl[3], hi) + if isVarOpenArrayParam(sl[1]): + # slicing a `var openArray` view: rebase onto the same underlying storage + r.res = "{base: ($1).base, off: ($1).off + $2, len: $3 - $2 + 1}" % [ + b.rdLoc, lo.rdLoc, hi.rdLoc] + else: + r.res = "{base: $1, off: $2, len: $3 - $2 + 1}" % [ + b.rdLoc, lo.rdLoc, hi.rdLoc] + elif isVarOpenArrayParam(sl): + # already a view from another `var openArray` param: forward it unchanged + gen(p, sl, b) + r.res = b.rdLoc + else: + # a whole array/seq/typed-array value: wrap with a zero offset + gen(p, n, v) + r.res = "{base: $1, off: 0, len: ($1).length}" % [v.rdLoc] + r.kind = resExpr + proc genArg(p: PProc, n: PNode, param: PSym, r: var TCompRes; emitted: ptr int = nil) = var a: TCompRes = default(TCompRes) + if param.typ != nil and param.typ.kind == tyVar and param.typ[0].kind == tyOpenArray: + # `var openArray` params are passed as a `{base, off, len}` slice view. + genVarOpenArrayArg(p, n, a) + r.res.add(a.rdLoc) + return gen(p, n, a) if skipTypes(param.typ, abstractVar).kind in {tyOpenArray, tyVarargs} and a.typ == etyBaseIndex: @@ -1737,6 +1803,13 @@ proc genArg(p: PProc, n: PNode, param: PSym, r: var TCompRes; emitted: ptr int = r.res.add(", ") r.res.add(a.res) if emitted != nil: inc emitted[] + elif skipTypes(param.typ, abstractVar).kind == tyOpenArray and + isVarOpenArrayParam(n): + # a `var openArray` view passed to a read-only `openArray` param: materialize + # a snapshot so the callee sees a plain array. + var w: TCompRes = default(TCompRes) + gen(p, n, w) + r.res.add("(($1).base).slice(($1).off, ($1).off + ($1).len)" % [w.rdLoc]) elif n.typ.kind in {tyVar, tyPtr, tyRef, tyLent, tyOwned} and n.kind in nkCallKinds and mapType(param.typ) == etyBaseIndex: # this fixes bug #5608: @@ -2371,13 +2444,21 @@ proc genMagic(p: PProc, n: PNode, r: var TCompRes) = useMagic(p, "nimCopy") r.res = "nimCopy(null, $1, $2)" % [x.rdLoc, genTypeInfo(p, n.typ)] of mOpenArrayToSeq: - genCall(p, n, r) + if isVarOpenArrayParam(n[1]): + var x: TCompRes = default(TCompRes) + gen(p, n[1], x) + r.res = "(($1).base).slice(($1).off, ($1).off + ($1).len)" % [x.rdLoc] + r.kind = resExpr + else: + genCall(p, n, r) of mDestroy, mTrace: discard "ignore calls to the default destructor" of mOrd: genOrd(p, n, r) of mLengthStr, mLengthSeq, mLengthOpenArray, mLengthArray: var x: TCompRes = default(TCompRes) gen(p, n[1], x) - if skipTypes(n[1].typ, abstractInst).kind == tyCstring: + if isVarOpenArrayParam(n[1]): + r.res = "($1).len" % [x.rdLoc] + elif skipTypes(n[1].typ, abstractInst).kind == tyCstring: let (a, tmp) = maybeMakeTemp(p, n[1], x) r.res = "(($1) == null ? 0 : ($2).length)" % [a, tmp] else: @@ -2386,7 +2467,9 @@ proc genMagic(p: PProc, n: PNode, r: var TCompRes) = of mHigh: var x: TCompRes = default(TCompRes) gen(p, n[1], x) - if skipTypes(n[1].typ, abstractInst).kind == tyCstring: + if isVarOpenArrayParam(n[1]): + r.res = "($1).len - 1" % [x.rdLoc] + elif skipTypes(n[1].typ, abstractInst).kind == tyCstring: let (a, tmp) = maybeMakeTemp(p, n[1], x) r.res = "(($1) == null ? -1 : ($2).length - 1)" % [a, tmp] else: @@ -2469,11 +2552,24 @@ proc genMagic(p: PProc, n: PNode, r: var TCompRes) = genCall(p, n, r) of mSlice: # arr.slice([begin[, end]]): 'end' is exclusive + # Fixed homogeneous numeric arrays lower to JS typed arrays; `slice` + # copies, which silently breaks `var openArray` write-through (bug #15952). + # `subarray` returns a live shared-buffer view with the same + # exclusive-end signature, so use it there; keep `slice` for seqs/strings. var x, y, z: TCompRes = default(TCompRes) gen(p, n[1], x) gen(p, n[2], y) gen(p, n[3], z) - r.res = "($1.slice($2, $3 + 1))" % [x.rdLoc, y.rdLoc, z.rdLoc] + if isVarOpenArrayParam(n[1]): + # re-slicing a `var openArray` view: materialize from the view's base/offset + r.res = "(($1).base).slice(($1).off + $2, ($1).off + $3 + 1)" % [ + x.rdLoc, y.rdLoc, z.rdLoc] + else: + let baseTy = skipTypes(n[1].typ, abstractVarRange + {tyLent}) + if baseTy.kind == tyArray and arrayTypeForElemType(p.config, elemType(baseTy)).len > 0: + r.res = "($1.subarray($2, $3 + 1))" % [x.rdLoc, y.rdLoc, z.rdLoc] + else: + r.res = "($1.slice($2, $3 + 1))" % [x.rdLoc, y.rdLoc, z.rdLoc] r.kind = resExpr of mMove: genMove(p, n, r) diff --git a/tests/openarray/topenarray.nim b/tests/openarray/topenarray.nim index 25b983651a..01c063d3fb 100644 --- a/tests/openarray/topenarray.nim +++ b/tests/openarray/topenarray.nim @@ -11,6 +11,9 @@ proc fn2[T](a: var openArray[T]): seq[T] = proc fn3[T](a: var openArray[T]) = for i, ai in mpairs(a): ai = i * 10 +proc wr[T](a: var openArray[T]; v: T) = + a[0] = v + proc main = var a = [1,2,3,4,5] @@ -20,8 +23,22 @@ proc main = doAssert fn2(a.toOpenArray(1,3)) == @[2,3,4] fn3(a.toOpenArray(1,3)) - when defined(js): discard # xxx bug #15952: `a` left unchanged - else: doAssert a == [1, 0, 10, 20, 5] + doAssert a == [1, 0, 10, 20, 5] + + block: # bug #15952: `toOpenArray` slices are live views on JS + # Fixed homogeneous numeric arrays lower to JS typed arrays; seqs and + # non-numeric fixed arrays lower to plain JS arrays. In all cases a slice + # passed to a `var openArray` must alias the source so writes propagate + # (JS: subarray view for typed arrays, {base,off,len} view otherwise). + var si = @[1, 2, 3, 4, 5] + fn3(si.toOpenArray(1, 3)) + doAssert si == @[1, 0, 10, 20, 5] + var ss = ["a", "b", "c", "d", "e"] + wr(ss.toOpenArray(1, 3), "Z") + doAssert ss == ["a", "Z", "c", "d", "e"] + # read-only slicing must still work and never throw, on every backend. + doAssert fn1(@[1, 2, 3, 4, 5].toOpenArray(1, 3)) == @[2, 3, 4] + doAssert fn1(["a", "b", "c", "d", "e"].toOpenArray(1, 3)) == @["b", "c", "d"] block: # bug #12521 block: