make strict funcs analysis smarter (#18219)

* make strict funcs analysis smarter: varParam[i] = v is different from varParam[i][] = v
* added a test case
* Update compiler/varpartitions.nim

Co-authored-by: Clyybber <darkmine956@gmail.com>

compiler/varpartitions.nim

@@ -175,12 +175,18 @@ proc root(v: var Partitions; start: int): int =
       v.s[it].con = Connection(kind: dependsOn, parent: result)
       it = next
 
-proc potentialMutation(v: var Partitions; s: PSym; info: TLineInfo) =
+proc potentialMutation(v: var Partitions; s: PSym; level: int; info: TLineInfo) =
   let id = variableId(v, s)
   if id >= 0:
     let r = root(v, id)
-    let flags = if s.kind == skParam and isConstParam(s):
+    let flags = if s.kind == skParam:
-                  {isMutated, isMutatedDirectly}
+                  if isConstParam(s):
+                    {isMutated, isMutatedDirectly}
+                  elif s.typ.kind == tyVar and level <= 1:
+                    # varParam[i] = v is different from varParam[i][] = v
+                    {}
+                  else:
+                    {isMutated}
                 else:
                   {isMutated}
 
@@ -339,36 +345,44 @@ proc pathExpr(node: PNode; owner: PSym): PNode =
   if result == nil and borrowFromConstExpr(n):
     result = n
 
-proc allRoots(n: PNode; result: var seq[PSym]; followDotExpr = true) =
+const
+  RootEscapes = 1000 # in 'p(r)' we don't know what p does to our poor root.
+                     # so we assume a high level of indirections
+
+proc allRoots(n: PNode; result: var seq[(PSym, int)]; level: int) =
   case n.kind
   of nkSym:
     if n.sym.kind in {skParam, skVar, skTemp, skLet, skResult, skForVar}:
-      result.add(n.sym)
+      result.add((n.sym, level))
 
-  of nkDotExpr, nkDerefExpr, nkBracketExpr, nkHiddenDeref,
+  of nkDerefExpr, nkHiddenDeref:
-      nkCheckedFieldExpr, nkAddr, nkHiddenAddr:
+    allRoots(n[0], result, level+1)
-    if followDotExpr:
+  of nkBracketExpr, nkDotExpr, nkCheckedFieldExpr, nkAddr, nkHiddenAddr:
-      allRoots(n[0], result, followDotExpr)
+    allRoots(n[0], result, level)
 
   of nkExprEqExpr, nkExprColonExpr, nkHiddenStdConv, nkHiddenSubConv, nkConv,
       nkStmtList, nkStmtListExpr, nkBlockStmt, nkBlockExpr, nkCast,
       nkObjUpConv, nkObjDownConv:
     if n.len > 0:
-      allRoots(n.lastSon, result, followDotExpr)
+      allRoots(n.lastSon, result, level)
   of nkCaseStmt, nkObjConstr:
     for i in 1..<n.len:
-      allRoots(n[i].lastSon, result, followDotExpr)
+      allRoots(n[i].lastSon, result, level)
   of nkIfStmt, nkIfExpr:
     for i in 0..<n.len:
-      allRoots(n[i].lastSon, result, followDotExpr)
+      allRoots(n[i].lastSon, result, level)
   of nkBracket, nkTupleConstr, nkPar:
     for i in 0..<n.len:
-      allRoots(n[i], result, followDotExpr)
+      allRoots(n[i], result, level-1)
 
   of nkCallKinds:
     if n.typ != nil and n.typ.kind in {tyVar, tyLent}:
       if n.len > 1:
-        allRoots(n[1], result, followDotExpr)
+        # XXX We really need the unwritten RFC here and distinguish between
+        #   proc `[]`(x: var Container): var T # resizes the container
+        # and
+        #   proc `[]`(x: Container): var T # only allows for slot mutation
+        allRoots(n[1], result, RootEscapes)
     else:
       let m = getMagic(n)
       case m
@@ -387,12 +401,12 @@ proc allRoots(n: PNode; result: var seq[PSym]; followDotExpr = true) =
             let paramType = typ.n[i].typ
             if not paramType.isCompileTimeOnly and not typ.sons[0].isEmptyType and
                 canAlias(paramType, typ.sons[0]):
-              allRoots(it, result, followDotExpr)
+              allRoots(it, result, RootEscapes)
           else:
-            allRoots(it, result, followDotExpr)
+            allRoots(it, result, RootEscapes)
 
       of mSlice:
-        allRoots(n[1], result, followDotExpr)
+        allRoots(n[1], result, level+1)
       else:
         discard "harmless operation"
   else:
@@ -466,10 +480,10 @@ proc destMightOwn(c: var Partitions; dest: var VarIndex; n: PNode) =
       dest.flags.incl ownsData
     elif n.typ.kind in {tyLent, tyVar}:
       # we know the result is derived from the first argument:
-      var roots: seq[PSym]
+      var roots: seq[(PSym, int)]
-      allRoots(n[1], roots)
+      allRoots(n[1], roots, RootEscapes)
       for r in roots:
-        connect(c, dest.sym, r, n[1].info)
+        connect(c, dest.sym, r[0], n[1].info)
 
     else:
       let magic = if n[0].kind == nkSym: n[0].sym.magic else: mNone
@@ -582,19 +596,19 @@ proc deps(c: var Partitions; dest, src: PNode) =
   if borrowChecking in c.goals:
     borrowingAsgn(c, dest, src)
 
-  var targets, sources: seq[PSym]
+  var targets, sources: seq[(PSym, int)]
-  allRoots(dest, targets)
+  allRoots(dest, targets, 0)
-  allRoots(src, sources)
+  allRoots(src, sources, 0)
 
   let destIsComplex = containsPointer(dest.typ)
 
   for t in targets:
     if dest.kind != nkSym and c.inNoSideEffectSection == 0:
-      potentialMutation(c, t, dest.info)
+      potentialMutation(c, t[0], t[1], dest.info)
 
     if destIsComplex:
       for s in sources:
-        connect(c, t, s, dest.info)
+        connect(c, t[0], s[0], dest.info)
 
   if cursorInference in c.goals and src.kind != nkEmpty:
     let d = pathExpr(dest, c.owner)
@@ -602,7 +616,8 @@ proc deps(c: var Partitions; dest, src: PNode) =
       let vid = variableId(c, d.sym)
       if vid >= 0:
         destMightOwn(c, c.s[vid], src)
-        for s in sources:
+        for source in sources:
+          let s = source[0]
           if s == d.sym:
             discard "assignments like: it = it.next are fine"
           elif {sfGlobal, sfThread} * s.flags != {} or hasDisabledAsgn(c.g, s.typ):
@@ -638,9 +653,9 @@ proc potentialMutationViaArg(c: var Partitions; n: PNode; callee: PType) =
   if constParameters in c.goals and tfNoSideEffect in callee.flags:
     discard "we know there are no hidden mutations through an immutable parameter"
   elif c.inNoSideEffectSection == 0 and containsPointer(n.typ):
-    var roots: seq[PSym]
+    var roots: seq[(PSym, int)]
-    allRoots(n, roots)
+    allRoots(n, roots, RootEscapes)
-    for r in roots: potentialMutation(c, r, n.info)
+    for r in roots: potentialMutation(c, r[0], r[1], n.info)
 
 proc traverse(c: var Partitions; n: PNode) =
   inc c.abstractTime
@@ -682,12 +697,12 @@ proc traverse(c: var Partitions; n: PNode) =
       if i < L:
         let paramType = parameters[i].skipTypes({tyGenericInst, tyAlias})
         if not paramType.isCompileTimeOnly and paramType.kind in {tyVar, tySink, tyOwned}:
-          var roots: seq[PSym]
+          var roots: seq[(PSym, int)]
-          allRoots(it, roots)
+          allRoots(it, roots, RootEscapes)
           if paramType.kind == tyVar:
             if c.inNoSideEffectSection == 0:
-              for r in roots: potentialMutation(c, r, it.info)
+              for r in roots: potentialMutation(c, r[0], r[1], it.info)
-            for r in roots: noCursor(c, r)
+            for r in roots: noCursor(c, r[0])
 
             if borrowChecking in c.goals:
               # a call like 'result.add toOpenArray()' can also be a borrow
@@ -703,10 +718,10 @@ proc traverse(c: var Partitions; n: PNode) =
     when false:
       # XXX investigate if this is required, it doesn't look
       # like it is!
-      var roots: seq[PSym]
+      var roots: seq[(PSym, int)]
-      allRoots(n[0], roots)
+      allRoots(n[0], roots, RootEscapes)
       for r in roots:
-        potentialMutation(c, r, it.info)
+        potentialMutation(c, r[0], r[1], it.info)
 
   of nkTupleConstr, nkBracket:
     for child in n: traverse(c, child)

tests/effects/tfuncs_cannot_mutate2.nim

@@ -0,0 +1,27 @@
+discard """
+  errormsg: "'copy' can have side effects"
+  nimout: '''an object reachable from 'y' is potentially mutated
+tfuncs_cannot_mutate2.nim(15, 7) the mutation is here
+tfuncs_cannot_mutate2.nim(13, 10) is the statement that connected the mutation to the parameter
+'''
+"""
+
+{.experimental: "strictFuncs".}
+
+func copy[T](x: var openArray[T]; y: openArray[T]) =
+  for i in 0..high(x):
+    x[i] = y[i]
+
+  x[0].a = nil
+
+type
+  R = ref object
+    a, b: R
+    data: string
+
+proc main =
+  var a, b: array[3, R]
+  b = [R(data: "a"), R(data: "b"), R(data: "c")]
+  copy a, b
+
+main()

tests/effects/tstrict_funcs.nim

@@ -27,3 +27,20 @@ block:
 
   var x = @[0, 1]
   let z = x &&& 2
+
+
+func copy[T](x: var openArray[T]; y: openArray[T]) =
+  for i in 0..high(x):
+    x[i] = y[i]
+
+type
+  R = ref object
+    a, b: R
+    data: string
+
+proc main =
+  var a, b: array[3, R]
+  b = [R(data: "a"), R(data: "b"), R(data: "c")]
+  copy a, b
+
+main()