better support for view types (#15436)

* you can put borrows into tables

* enforces mutating views only mutate mutable data

compiler/ccgcalls.nim

@@ -142,8 +142,10 @@ proc fixupCall(p: BProc, le, ri: PNode, d: var TLoc,
 proc genBoundsCheck(p: BProc; arr, a, b: TLoc)
 
 proc reifiedOpenArray(n: PNode): bool {.inline.} =
-  let x = trees.getRoot(n)
-  if x != nil and x.kind == skParam:
+  var x = n
+  while x.kind in {nkAddr, nkHiddenAddr, nkHiddenStdConv, nkHiddenDeref}:
+    x = x[0]
+  if x.kind == nkSym and x.sym.kind == skParam:
     result = false
   else:
     result = true

compiler/typeallowed.nim

@@ -121,7 +121,7 @@ proc typeAllowedAux(marker: var IntSet, typ: PType, kind: TSymKind,
       {tyChar, tyEnum, tyInt..tyFloat128, tyInt..tyUInt64}: result = t
   of tyOpenArray:
     # you cannot nest openArrays/sinks/etc.
-    if (kind != skParam and views notin c.features) or taIsOpenArray in flags:
+    if (kind != skParam or taIsOpenArray in flags) and views notin c.features:
       result = t
     else:
       result = typeAllowedAux(marker, t[0], kind, c, flags+{taIsOpenArray})

compiler/varpartitions.nim

@@ -54,7 +54,9 @@ type
   VarFlag = enum
     ownsData,
     preventCursor,
-    isReassigned
+    isReassigned,
+    viewDoesMutate,
+    viewBorrowsFromConst
 
   VarIndexKind = enum
     isEmptyRoot,
@@ -478,9 +480,12 @@ proc pretendOwnsData(c: var Partitions, s: PSym) =
 const
   explainCursors = false
 
+proc isConstSym(s: PSym): bool =
+  result = s.kind in {skConst, skLet} or isConstParam(s)
+
 proc borrowFrom(c: var Partitions; dest: PSym; src: PNode) =
   const
-    url = "; see https://nim-lang.github.io/Nim/manual_experimental.html#view-types-algorithm-path-expressions for details"
+    url = "see https://nim-lang.github.io/Nim/manual_experimental.html#view-types-algorithm-path-expressions for details"
 
   let s = pathExpr(src, c.owner)
   if s == nil:
@@ -499,28 +504,50 @@ proc borrowFrom(c: var Partitions; dest: PSym; src: PNode) =
                         aliveStart: MinTime, aliveEnd: MaxTime)
 
       c.s[vid].borrowsFrom.add sourceIdx
+      if isConstSym(s.sym):
+        c.s[vid].flags.incl viewBorrowsFromConst
   else:
-    discard "a valid borrow location that is a deeply constant expression so we have nothing to track"
+    let vid = variableId(c, dest)
+    if vid >= 0:
+      c.s[vid].flags.incl viewBorrowsFromConst
+    #discard "a valid borrow location that is a deeply constant expression so we have nothing to track"
 
 
 proc borrowingCall(c: var Partitions; destType: PType; n: PNode; i: int) =
   let v = pathExpr(n[i], c.owner)
-  if v.kind == nkSym:
+  if v != nil and v.kind == nkSym:
+    when false:
+      let isView = directViewType(destType) == immutableView
+      if n[0].kind == nkSym and n[0].sym.name.s == "[]=":
+        localError(c.config, n[i].info, "attempt to mutate an immutable view")
+
     for j in i+1..<n.len:
       if getMagic(n[j]) == mSlice:
         borrowFrom(c, v.sym, n[j])
   else:
     localError(c.config, n[i].info, "cannot determine the target of the borrow")
 
-proc trackBorrow(c: var Partitions; dest, src: PNode) =
+proc borrowingAsgn(c: var Partitions; dest, src: PNode) =
   if dest.kind == nkSym:
-    let vk = directViewType(dest.typ)
-    if vk != noView:
+    if directViewType(dest.typ) != noView:
       borrowFrom(c, dest.sym, src)
+  elif dest.kind in {nkHiddenDeref, nkDerefExpr, nkBracketExpr}:
+    case directViewType(dest[0].typ)
+    of mutableView:
+      # we do not borrow, but we use the view to mutate the borrowed
+      # location:
+      let viewOrigin = pathExpr(dest, c.owner)
+      if viewOrigin.kind == nkSym:
+        let vid = variableId(c, viewOrigin.sym)
+        if vid >= 0:
+          c.s[vid].flags.incl viewDoesMutate
+    of immutableView:
+      localError(c.config, dest.info, "attempt to mutate a borrowed location from an immutable view")
+    of noView: discard "nothing to do"
 
 proc deps(c: var Partitions; dest, src: PNode) =
   if not c.performCursorInference:
-    trackBorrow(c, dest, src)
+    borrowingAsgn(c, dest, src)
 
   var targets, sources: seq[PSym]
   allRoots(dest, targets)
@@ -801,6 +828,7 @@ proc checkBorrowedLocations*(par: var Partitions; body: PNode; config: ConfigRef
     if v.kind != skParam and classifyViewType(v.typ) != noView:
       let rid = root(par, i)
       if rid >= 0:
+        var constViolation = false
         for b in par.s[rid].borrowsFrom:
           let sid = root(par, b)
           if sid >= 0:
@@ -809,6 +837,16 @@ proc checkBorrowedLocations*(par: var Partitions; body: PNode; config: ConfigRef
             if par.s[sid].sym.kind != skParam and par.s[sid].aliveEnd < par.s[rid].aliveEnd:
               localError(config, v.info, "'" & v.name.s & "' borrows from location '" & par.s[sid].sym.name.s &
                 "' which does not live long enough")
+            if viewDoesMutate in par.s[rid].flags and isConstSym(par.s[sid].sym):
+              localError(config, v.info, "'" & v.name.s & "' borrows from the immutable location '" &
+                par.s[sid].sym.name.s & "' and attempts to mutate it")
+              constViolation = true
+        if {viewDoesMutate, viewBorrowsFromConst} * par.s[rid].flags == {viewDoesMutate, viewBorrowsFromConst} and
+            not constViolation:
+          # we do not track the constant expressions we allow to borrow from so
+          # we can only produce a more generic error message:
+          localError(config, v.info, "'" & v.name.s &
+              "' borrows from an immutable location and attempts to mutate it")
 
       #if par.s[rid].con.kind == isRootOf and dangerousMutation(par.graphs[par.s[rid].con.graphIndex], par.s[i]):
       #  cannotBorrow(config, s, par.graphs[par.s[rid].con.graphIndex])

tests/views/tdont_mutate.nim

@@ -0,0 +1,48 @@
+discard """
+  cmd: "nim check --hints:off $file"
+"""
+
+import tables
+
+{.experimental: "views".}
+
+const
+  Whitespace = {' ', '\t', '\n', '\r'}
+
+proc split*(s: string, seps: set[char] = Whitespace,
+                maxsplit: int = -1): Table[int, openArray[char]] =
+  var last = 0
+  var splits = maxsplit
+  result = initTable[int, openArray[char]]()
+
+  while last <= len(s):
+    var first = last
+    while last < len(s) and s[last] notin seps:
+      inc(last)
+    if splits == 0: last = len(s)
+    result[first] = toOpenArray(s, first, last-1)
+
+    result[first][0] = 'c' #[tt.Error
+      attempt to mutate a borrowed location from an immutable view
+    ]#
+
+    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 otherTest(x: int) =
+  var y: var int = x #[tt.Error
+    'y' borrows from the immutable location 'x' and attempts to mutate it
+  ]#
+  y = 3
+
+proc main() =
+  let words = split("asdf 231")
+  for i, x in words:
+    echo i, ": ", x
+
+main()

tests/views/tsplit_into_table.nim

@@ -0,0 +1,40 @@
+discard """
+  output: '''5: 231
+0: asdf
+'''
+"""
+
+import tables
+
+{.experimental: "views".}
+
+const
+  Whitespace = {' ', '\t', '\n', '\r'}
+
+proc split*(s: string, seps: set[char] = Whitespace,
+                maxsplit: int = -1): Table[int, openArray[char]] =
+  var last = 0
+  var splits = maxsplit
+  result = initTable[int, openArray[char]]()
+
+  while last <= len(s):
+    var first = last
+    while last < len(s) and s[last] notin seps:
+      inc(last)
+    if splits == 0: last = len(s)
+    result[first] = 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() =
+  let words = split("asdf 231")
+  for i, x in words:
+    echo i, ": ", x
+
+main()