From 57b7841c184e9f185d6d2404a1320e5d089e0e0f Mon Sep 17 00:00:00 2001
From: Andreas Rumpf <rumpf_a@web.de>
Date: Sun, 27 Sep 2020 09:20:10 +0200
Subject: [PATCH] better support for slices as views (#15414)

* moved view tests to tests/views
* refactoring
* more refactorings
* better support for system.toOpenArray for first class view types
---
 compiler/ccgcalls.nim                       | 85 ++++++++++++---------
 compiler/ccgexprs.nim                       | 12 ++-
 compiler/semexprs.nim                       | 10 ++-
 tests/{effects => views}/tcannot_borrow.nim |  0
 tests/views/tsplit_into_openarray.nim       | 37 +++++++++
 tests/{ccgbugs => views}/tviews1.nim        |  0
 6 files changed, 100 insertions(+), 44 deletions(-)
 rename tests/{effects => views}/tcannot_borrow.nim (100%)
 create mode 100644 tests/views/tsplit_into_openarray.nim
 rename tests/{ccgbugs => views}/tviews1.nim (100%)

diff --git a/compiler/ccgcalls.nim b/compiler/ccgcalls.nim
index 6d254ca6f..31e5e5924 100644
--- a/compiler/ccgcalls.nim
+++ b/compiler/ccgcalls.nim
@@ -148,9 +148,52 @@ proc reifiedOpenArray(n: PNode): bool {.inline.} =
   else:
     result = true
 
-proc openArrayLoc(p: BProc, formalType: PType, n: PNode): Rope =
-  var a: TLoc
+proc genOpenArraySlice(p: BProc; q: PNode; formalType, destType: PType): (Rope, Rope) =
+  var a, b, c: TLoc
+  initLocExpr(p, q[1], a)
+  initLocExpr(p, q[2], b)
+  initLocExpr(p, q[3], c)
+  # but first produce the required index checks:
+  if optBoundsCheck in p.options:
+    genBoundsCheck(p, a, b, c)
+  let ty = skipTypes(a.t, abstractVar+{tyPtr})
+  let dest = getTypeDesc(p.module, destType)
+  let lengthExpr = "($1)-($2)+1" % [rdLoc(c), rdLoc(b)]
+  case ty.kind
+  of tyArray:
+    let first = toInt64(firstOrd(p.config, ty))
+    if first == 0:
+      result = ("($3*)(($1)+($2))" % [rdLoc(a), rdLoc(b), dest],
+                lengthExpr)
+    else:
+      result = ("($4*)($1)+(($2)-($3))" %
+        [rdLoc(a), rdLoc(b), intLiteral(first), dest],
+        lengthExpr)
+  of tyOpenArray, tyVarargs:
+    if reifiedOpenArray(q[1]):
+      result = ("($3*)($1.d)+($2)" % [rdLoc(a), rdLoc(b), dest],
+                lengthExpr)
+    else:
+      result = ("($3*)($1)+($2)" % [rdLoc(a), rdLoc(b), dest],
+                lengthExpr)
+  of tyUncheckedArray, tyCString:
+    result = ("($3*)($1)+($2)" % [rdLoc(a), rdLoc(b), dest],
+              lengthExpr)
+  of tyString, tySequence:
+    let atyp = skipTypes(a.t, abstractInst)
+    if formalType.skipTypes(abstractInst).kind in {tyVar} and atyp.kind == tyString and
+        optSeqDestructors in p.config.globalOptions:
+      linefmt(p, cpsStmts, "#nimPrepareStrMutationV2($1);$n", [byRefLoc(p, a)])
+    if atyp.kind in {tyVar} and not compileToCpp(p.module):
+      result = ("($4*)(*$1)$3+($2)" % [rdLoc(a), rdLoc(b), dataField(p), dest],
+                lengthExpr)
+    else:
+      result = ("($4*)$1$3+($2)" % [rdLoc(a), rdLoc(b), dataField(p), dest],
+                lengthExpr)
+  else:
+    internalError(p.config, "openArrayLoc: " & typeToString(a.t))
 
+proc openArrayLoc(p: BProc, formalType: PType, n: PNode): Rope =
   var q = skipConv(n)
   var skipped = false
   while q.kind == nkStmtListExpr and q.len > 0:
@@ -164,42 +207,10 @@ proc openArrayLoc(p: BProc, formalType: PType, n: PNode): Rope =
         for i in 0..<q.len-1:
           genStmts(p, q[i])
         q = q.lastSon
-    var b, c: TLoc
-    initLocExpr(p, q[1], a)
-    initLocExpr(p, q[2], b)
-    initLocExpr(p, q[3], c)
-    # but first produce the required index checks:
-    if optBoundsCheck in p.options:
-      genBoundsCheck(p, a, b, c)
-    let ty = skipTypes(a.t, abstractVar+{tyPtr})
-    let dest = getTypeDesc(p.module, n.typ[0])
-    case ty.kind
-    of tyArray:
-      let first = toInt64(firstOrd(p.config, ty))
-      if first == 0:
-        result = "($4*)(($1)+($2)), ($3)-($2)+1" % [rdLoc(a), rdLoc(b), rdLoc(c), dest]
-      else:
-        result = "($5*)($1)+(($2)-($4)), ($3)-($2)+1" %
-          [rdLoc(a), rdLoc(b), rdLoc(c), intLiteral(first), dest]
-    of tyOpenArray, tyVarargs:
-      if reifiedOpenArray(q[1]):
-        result = "($4*)($1.d)+($2), ($3)-($2)+1" % [rdLoc(a), rdLoc(b), rdLoc(c), dest]
-      else:
-        result = "($4*)($1)+($2), ($3)-($2)+1" % [rdLoc(a), rdLoc(b), rdLoc(c), dest]
-    of tyUncheckedArray, tyCString:
-      result = "($4*)($1)+($2), ($3)-($2)+1" % [rdLoc(a), rdLoc(b), rdLoc(c), dest]
-    of tyString, tySequence:
-      let atyp = skipTypes(a.t, abstractInst)
-      if formalType.skipTypes(abstractInst).kind in {tyVar} and atyp.kind == tyString and
-          optSeqDestructors in p.config.globalOptions:
-        linefmt(p, cpsStmts, "#nimPrepareStrMutationV2($1);$n", [byRefLoc(p, a)])
-      if atyp.kind in {tyVar} and not compileToCpp(p.module):
-        result = "($5*)(*$1)$4+($2), ($3)-($2)+1" % [rdLoc(a), rdLoc(b), rdLoc(c), dataField(p), dest]
-      else:
-        result = "($5*)$1$4+($2), ($3)-($2)+1" % [rdLoc(a), rdLoc(b), rdLoc(c), dataField(p), dest]
-    else:
-      internalError(p.config, "openArrayLoc: " & typeToString(a.t))
+    let (x, y) = genOpenArraySlice(p, q, formalType, n.typ[0])
+    result = x & ", " & y
   else:
+    var a: TLoc
     initLocExpr(p, if n.kind == nkHiddenStdConv: n[1] else: n, a)
     case skipTypes(a.t, abstractVar).kind
     of tyOpenArray, tyVarargs:
diff --git a/compiler/ccgexprs.nim b/compiler/ccgexprs.nim
index 83f7f7278..f348a7e26 100644
--- a/compiler/ccgexprs.nim
+++ b/compiler/ccgexprs.nim
@@ -2215,6 +2215,14 @@ proc genDispose(p: BProc; n: PNode) =
       # destructor, but it uses the runtime type. Afterwards the memory is freed:
       lineCg(p, cpsStmts, ["#nimDestroyAndDispose($#)", rdLoc(a)])
 
+proc genSlice(p: BProc; e: PNode; d: var TLoc) =
+  let (x, y) = genOpenArraySlice(p, e, e.typ, e.typ.lastSon)
+  if d.k == locNone: getTemp(p, e.typ, d)
+  linefmt(p, cpsStmts, "$1.d = $2; $1.l = $3;$n", [rdLoc(d), x, y])
+  when false:
+    localError(p.config, e.info, "invalid context for 'toOpenArray'; " &
+      "'toOpenArray' is only valid within a call expression")
+
 proc genEnumToStr(p: BProc, e: PNode, d: var TLoc) =
   const ToStringProcSlot = -4
   let t = e[1].typ.skipTypes(abstractInst+{tyRange})
@@ -2396,9 +2404,7 @@ proc genMagicExpr(p: BProc, e: PNode, d: var TLoc, op: TMagic) =
   of mMove: genMove(p, e, d)
   of mDestroy: genDestroy(p, e)
   of mAccessEnv: unaryExpr(p, e, d, "$1.ClE_0")
-  of mSlice:
-    localError(p.config, e.info, "invalid context for 'toOpenArray'; " &
-      "'toOpenArray' is only valid within a call expression")
+  of mSlice: genSlice(p, e, d)
   else:
     when defined(debugMagics):
       echo p.prc.name.s, " ", p.prc.id, " ", p.prc.flags, " ", p.prc.ast[genericParamsPos].kind
diff --git a/compiler/semexprs.nim b/compiler/semexprs.nim
index d6b146800..e26ea42dd 100644
--- a/compiler/semexprs.nim
+++ b/compiler/semexprs.nim
@@ -1762,7 +1762,7 @@ proc semReturn(c: PContext, n: PNode): PNode =
         a.add newSymNode(c.p.resultSym)
         a.add n[0]
         n[0] = a
-      else:        
+      else:
         localError(c.config, n.info, errNoReturnTypeDeclared)
         return
       result[0] = semAsgn(c, n[0])
@@ -1831,8 +1831,10 @@ proc semYieldVarResult(c: PContext, n: PNode, restype: PType) =
         else:
           localError(c.config, n[0].info, errXExpected, "tuple constructor")
   else:
-    if isViewType(t):
-      n[0] = takeImplicitAddr(c, n[0], false)
+    when false:
+      # XXX investigate what we really need here.
+      if isViewType(t):
+        n[0] = takeImplicitAddr(c, n[0], false)
 
 proc semYield(c: PContext, n: PNode): PNode =
   result = n
@@ -2060,7 +2062,7 @@ proc semQuoteAst(c: PContext, n: PNode): PNode =
                   else:
                     identNodeSym.newSymNode
   quotes[1] = newTreeI(nkCall, n.info, identNode, newStrNode(nkStrLit, "result"))
-  result = newTreeI(nkCall, n.info, 
+  result = newTreeI(nkCall, n.info,
      createMagic(c.graph, "getAst", mExpandToAst).newSymNode,
      newTreeI(nkCall, n.info, quotes))
   result = semExpandToAst(c, result)
diff --git a/tests/effects/tcannot_borrow.nim b/tests/views/tcannot_borrow.nim
similarity index 100%
rename from tests/effects/tcannot_borrow.nim
rename to tests/views/tcannot_borrow.nim
diff --git a/tests/views/tsplit_into_openarray.nim b/tests/views/tsplit_into_openarray.nim
new file mode 100644
index 000000000..3ea290d89
--- /dev/null
+++ b/tests/views/tsplit_into_openarray.nim
@@ -0,0 +1,37 @@
+discard """
+  output: '''asdf
+231
+'''
+  cmd: "nim c --gc:arc -d:useMalloc -g $file"
+  valgrind: true
+"""
+
+{.experimental: "views".}
+
+const
+  Whitespace = {' ', '\t', '\n', '\r'}
+
+iterator split*(s: string, seps: set[char] = Whitespace,
+                maxsplit: int = -1): openArray[char] =
+  var last = 0
+  var splits = maxsplit
+
+  while last <= len(s):
+    var first = last
+    while last < len(s) and s[last] notin seps:
+      inc(last)
+    if splits == 0: last = len(s)
+    yield toOpenArray(s, first, last-1)
+    if splits == 0: break
+    dec(splits)
+    inc(last)
+
+proc `$`(x: openArray[char]): string =
+  result = newString(x.len)
+  for i in 0..<x.len: result[i] = x[i]
+
+proc main() =
+  for x in split("asdf 231"):
+    echo x
+
+main()
diff --git a/tests/ccgbugs/tviews1.nim b/tests/views/tviews1.nim
similarity index 100%
rename from tests/ccgbugs/tviews1.nim
rename to tests/views/tviews1.nim