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

@@ -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.

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)

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: