Finer analysis for array access (#16787)

* Refine the analysis for array access
* Cleanup
* Add comments

compiler/dfa.nim

@@ -574,32 +574,84 @@ proc skipConvDfa*(n: PNode): PNode =
       result = result[1]
     else: break
 
-proc aliases*(obj, field: PNode): bool =
+type AliasKind* = enum
-  var n = field
+  yes, no, maybe
-  var obj = obj
+
-  while true:
+proc aliases*(obj, field: PNode): AliasKind =
-    case obj.kind
+  # obj -> field:
-    of {nkObjDownConv, nkObjUpConv, nkAddr, nkHiddenAddr, nkDerefExpr, nkHiddenDeref}:
+  # x -> x: true
-      obj = obj[0]
+  # x -> x.f: true
-    of PathKinds1:
+  # x.f -> x: false
-      obj = obj[1]
+  # x.f -> x.f: true
-    else: break
+  # x.f -> x.v: false
-  while true:
+  # x -> x[0]: true
-    if sameTrees(obj, n): return true
+  # x[0] -> x: false
-    case n.kind
+  # x[0] -> x[0]: true
-    of PathKinds0:
+  # x[0] -> x[1]: false
-      n = n[0]
+  # x -> x[i]: true
-    of PathKinds1:
+  # x[i] -> x: false
-      n = n[1]
+  # x[i] -> x[i]: maybe; Further analysis could make this return true when i is a runtime-constant
-    else: break
+  # x[i] -> x[j]: maybe; also returns maybe if only one of i or j is a compiletime-constant
+  template collectImportantNodes(result, n) =
+    var result: seq[PNode]
+    var n = n
+    while true:
+      case n.kind
+      of PathKinds0 - {nkDotExpr, nkCheckedFieldExpr, nkBracketExpr}:
+        n = n[0]
+      of PathKinds1:
+        n = n[1]
+      of nkDotExpr, nkCheckedFieldExpr, nkBracketExpr:
+        result.add n
+        n = n[0]
+      of nkSym:
+        result.add n; break
+      else: return no
+
+  collectImportantNodes(objImportantNodes, obj)
+  collectImportantNodes(fieldImportantNodes, field)
+
+  # If field is less nested than obj, then it cannot be part of/aliased by obj
+  if fieldImportantNodes.len < objImportantNodes.len: return no
+
+  result = yes
+  for i in 1..objImportantNodes.len:
+    # We compare the nodes leading to the location of obj and field
+    # with each other.
+    # We continue until they diverge, in which case we return no, or
+    # until we reach the location of obj, in which case we do not need
+    # to look further, since field must be part of/aliased by obj now.
+    # If we encounter an element access using an index which is a runtime value,
+    # we simply return maybe instead of yes; should further nodes not diverge.
+    let currFieldPath = fieldImportantNodes[^i]
+    let currObjPath = objImportantNodes[^i]
+
+    if currFieldPath.kind != currObjPath.kind:
+      return no
+
+    case currFieldPath.kind
+    of nkSym:
+      if currFieldPath.sym != currObjPath.sym: return no
+    of nkDotExpr, nkCheckedFieldExpr:
+      if currFieldPath[1].sym != currObjPath[1].sym: return no
+    of nkBracketExpr:
+      if currFieldPath[1].kind in nkLiterals and currObjPath[1].kind in nkLiterals:
+        if currFieldPath[1].intVal != currObjPath[1].intVal:
+          return no
+      else:
+        result = maybe
+    else: assert false # unreachable
 
 type InstrTargetKind* = enum
   None, Full, Partial
 
 proc instrTargets*(insloc, loc: PNode): InstrTargetKind =
-  if sameTrees(insloc, loc) or insloc.aliases(loc):
+  case insloc.aliases(loc)
+  of yes:
     Full    # x -> x; x -> x.f
-  elif loc.aliases(insloc):
+  of maybe:
+    Partial # We treat this like a partial write/read
+  elif loc.aliases(insloc) != no:
     Partial # x.f -> x
   else: None
 
@@ -607,16 +659,10 @@ proc isAnalysableFieldAccess*(orig: PNode; owner: PSym): bool =
   var n = orig
   while true:
     case n.kind
-    of PathKinds0 - {nkBracketExpr, nkHiddenDeref, nkDerefExpr}:
+    of PathKinds0 - {nkHiddenDeref, nkDerefExpr}:
       n = n[0]
     of PathKinds1:
       n = n[1]
-    of nkBracketExpr:
-      # in a[i] the 'i' must be known
-      if n.len > 1 and n[1].kind in {nkCharLit..nkUInt64Lit}:
-        n = n[0]
-      else:
-        return false
     of nkHiddenDeref, nkDerefExpr:
       # We "own" sinkparam[].loc but not ourVar[].location as it is a nasty
       # pointer indirection.

compiler/injectdestructors.nim

@@ -969,7 +969,7 @@ proc p(n: PNode; c: var Con; s: var Scope; mode: ProcessMode): PNode =
 
 proc sameLocation*(a, b: PNode): bool =
   proc sameConstant(a, b: PNode): bool =
-    a.kind in nkLiterals and exprStructuralEquivalent(a, b)
+    a.kind in nkLiterals and a.intVal == b.intVal
 
   const nkEndPoint = {nkSym, nkDotExpr, nkCheckedFieldExpr, nkBracketExpr}
   if a.kind in nkEndPoint and b.kind in nkEndPoint:
@@ -1006,7 +1006,7 @@ proc moveOrCopy(dest, ri: PNode; c: var Con; s: var Scope, isDecl = false): PNod
         # unpacking of tuple: take over the elements
         result = c.genSink(dest, p(ri, c, s, consumed), isDecl)
       elif isAnalysableFieldAccess(ri, c.owner) and isLastRead(ri, c):
-        if not aliases(dest, ri):
+        if aliases(dest, ri) == no:
           # Rule 3: `=sink`(x, z); wasMoved(z)
           var snk = c.genSink(dest, ri, isDecl)
           result = newTree(nkStmtList, snk, c.genWasMoved(ri))

tests/arc/tmovebug.nim

@@ -92,10 +92,20 @@ copy
 destroy
 destroy
 destroy
+sink
+sink
+destroy
+copy
+(f: 1)
+destroy
+destroy
 part-to-whole assigment:
 sink
 (children: @[])
 destroy
+sink
+(children: @[])
+destroy
 copy
 destroy
 '''
@@ -675,6 +685,16 @@ proc caseNotAConstant =
 
 caseNotAConstant()
 
+proc potentialSelfAssign(i: var int) =
+  var a: array[2, OO]
+  a[i] = OO(f: 1)
+  a[1] = OO(f: 2)
+  a[i+1] = a[i] # This must not =sink, but =copy
+  inc i
+  echo a[i-1] # (f: 1)
+
+potentialSelfAssign (var xi = 0; xi)
+
 
 #--------------------------------------------------------------------
 echo "part-to-whole assigment:"
@@ -700,6 +720,17 @@ proc partToWholeSeq =
 
 partToWholeSeq()
 
+proc partToWholeSeqRTIndex =
+  var i = 0
+  var t = Tree(children: @[Tree()])
+  t = t.children[i] # See comment in partToWholeSeq
+
+  var tc = TreeDefaultHooks(children: @[TreeDefaultHooks()])
+  tc = tc.children[i] # See comment in partToWholeSeq
+  echo tc
+
+partToWholeSeqRTIndex()
+
 type List = object
   next: ref List