refactor(mir): use proper operand queries (#943)

## Summary

In the MIR-using passes, replace raw indexing into the tree with proper
operand queries. This makes more of the processing node-layout
agnostic, which is a preparation for reworking the MIR.

## Details

* rename `unaryOperand` to `operand`. It's shorter, and gets the same
  meaning across
* add `mnkView` to the `SingleInputNodes` set -- it was previously
  missing
* allow using values of `NodePosition` and `OpValue` index-types as the
  arguments to `operand`. Requiring a `Operation` index-type value only
  introduced noise
* replace raw single-operand lookup (indexing into the tree with
  `i - 1`) with usage of `operand`

Querying the second operand of a `mnkPathArray` is still done via the
raw-indexing approach (`i - 2`), as usage of `operand(tree, i, 1)`
would still be rather inefficient.

compiler/mir/analysis.nim

@@ -257,22 +257,22 @@ func computeValuesAndEffects*(body: MirTree): Values =
       # the result is always owned
       start: Owned.yes
     of mnkAddr, mnkView, mnkPathPos, mnkPathVariant:
-      inheritDecay(i, i - 1)
+      inheritDecay(i, NodePosition body.operand(i))
     of mnkPathArray:
       # inherit from the first operand (i.e. the array-like value)
       inheritDecay(i, NodePosition operand(body, Operation(i), 0))
     of mnkPathNamed:
-      inheritDecay(i, i - 1)
+      inheritDecay(i, NodePosition body.operand(i))
       if sfCursor in n.field.flags:
         # any lvalue derived from a cursor location is non-owning
         result.values[OpValue i].owns = Owned.no
     of mnkPathConv:
-      inherit(i, i - 1)
+      inherit(i, NodePosition body.operand(i))
 
     of mnkArgBlock:
       num.add stack.len.uint16 # remember the current top-of-stack
     of mnkTag:
-      stack.add Effect(kind: n.effect, loc: OpValue(i - 1))
+      stack.add Effect(kind: n.effect, loc: body.operand(i))
     of mnkEnd:
       if n.start == mnkArgBlock:
         popEffects(Operation(i+1))
@@ -332,7 +332,7 @@ func isAlive*(tree: MirTree, cfg: ControlFlowGraph, v: Values,
         return true
 
     of ConsumeCtx:
-      let opr = unaryOperand(tree, Operation i)
+      let opr = tree.operand(i)
       if v.owned(opr) == Owned.yes:
         if isPartOf(tree, loc, toLvalue opr) == yes:
           # the location's value is consumed and it becomes empty. No operation
@@ -402,13 +402,13 @@ func isLastRead*(tree: MirTree, cfg: ControlFlowGraph, values: Values,
         return false
 
     of UseContext - {mnkDefUnpack}:
-      if overlaps(tree, loc, toLvalue unaryOperand(tree, Operation i)) != no:
+      if overlaps(tree, loc, toLvalue tree.operand(Operation i)) != no:
         return false
 
     of DefNodes:
       # passing a value to a 'def' is also a use
       if hasInput(tree, Operation i) and
-         overlaps(tree, loc, toLvalue unaryOperand(tree, Operation i)) != no:
+         overlaps(tree, loc, toLvalue tree.operand(Operation i)) != no:
         return false
 
     else:
@@ -513,7 +513,7 @@ func computeAliveOp*[T: PSym | TempId](
         result = alive
 
   of ConsumeCtx:
-    let opr = unaryOperand(tree, op)
+    let opr = tree.operand(op)
     if values.owned(opr) == Owned.yes and sameLocation(opr):
       # the location's value is consumed
       result = dead

compiler/mir/mirpasses.nim

@@ -73,10 +73,8 @@ func getRoot(tree: MirTree, n: OpValue): NodePosition =
   var i = n
   while tree[i].kind in PathNodes:
     case tree[i].kind
-    of mnkPathNamed, mnkPathPos, mnkPathVariant:
-      i = OpValue(NodePosition(i) - 1)
-    of mnkPathConv:
-      i = unaryOperand(tree, Operation(i))
+    of mnkPathNamed, mnkPathPos, mnkPathVariant, mnkPathConv:
+      i = tree.operand(i)
     of mnkPathArray:
       i = operand(tree, Operation(i), 0)
     of AllNodeKinds - PathNodes:
@@ -89,7 +87,7 @@ func getOpChain(tree: MirTree, a: OpValue): LvalueExpr {.inline.} =
   (getRoot(tree, a), NodePosition a)
 
 func skipTag(tree: MirTree, a: OpValue): OpValue {.inline.} =
-  if tree[a].kind == mnkTag: OpValue(NodePosition(a) - 1)
+  if tree[a].kind == mnkTag: tree.operand(a)
   else:                      a
 
 func skipOpParam(tree: MirTree, n: OpValue): OpValue =
@@ -133,7 +131,7 @@ func rawNext(iter: var ArgIter, tree: MirTree): NodePosition =
 
 func next(iter: var ArgIter, tree: MirTree): OpValue {.inline.} =
   ## Returns the next operand node.
-  OpValue(rawNext(iter, tree) - 1)
+  tree.operand(rawNext(iter, tree))
 
 func hasNext(iter: ArgIter): bool {.inline.} =
   iter.pos != NodePosition(-1)
@@ -165,7 +163,7 @@ iterator uses(tree: MirTree, start, last: NodePosition): OpValue =
     let kind = tree[i].kind
     if kind in UseContext + ArgumentNodes or
        (kind in DefNodes and i.int > 0 and hasInput(tree, Operation i)):
-      yield skipTag(tree, unaryOperand(tree, Operation(i)))
+      yield skipTag(tree, tree.operand(i))
 
     dec i
 
@@ -181,7 +179,7 @@ iterator potentialMutations(tree: MirTree, start, last: NodePosition): OpValue =
     # all ``mnkTag`` nodes currently imply some sort of mutation/change
     if tree[i].kind in {mnkTag, mnkAddr} or
        (tree[i].kind == mnkView and tree[i].typ.kind == tyVar):
-      yield OpValue(i - 1)
+      yield tree.operand(i)
 
     inc i
 
@@ -358,10 +356,10 @@ proc fixupCallArguments(tree: MirTree, config: ConfigRef,
         case tree[x].kind
         of mnkAddr, mnkView, mnkDeref, mnkDerefView, mnkConv, mnkStdConv,
            mnkTag, mnkPathConv:
-          x = unaryOperand(tree, x)
+          x = tree.operand(x)
         of mnkPathNamed, mnkPathPos, mnkPathVariant:
           result.add NodePosition(x)
-          x = unaryOperand(tree, x)
+          x = tree.operand(x)
         of mnkPathArray:
           result.add NodePosition(x)
           x = operand(tree, x, 0)
@@ -440,7 +438,7 @@ proc fixupCallArguments(tree: MirTree, config: ConfigRef,
         #   mutations
         # If either is the case, we need to introduce a shallow copy and use
         # that as the argument
-        let val = skipTag(tree, unaryOperand(tree, Operation arg))
+        let val = skipTag(tree, tree.operand(arg))
 
         # 1. an r-value is unique, meaning that we know that a temporary is
         #    not needed
@@ -458,9 +456,11 @@ proc fixupCallArguments(tree: MirTree, config: ConfigRef,
           # analysed as part of the ``potentialMutations`` loop below)
           var iter2 = iter
           while iter2.hasNext:
-            let arg2 = rawNext(iter2, tree)
-            if tree[arg2].kind == mnkName and tree[arg2 - 1].kind == mnkTag:
-              if maybeSameMutableLocation(tree, val, OpValue(arg2 - 2)):
+            let
+              arg2   = rawNext(iter2, tree)
+              argVal = tree.operand(arg2)
+            if tree[arg2].kind == mnkName and tree[argVal].kind == mnkTag:
+              if maybeSameMutableLocation(tree, val, tree.operand(argVal)):
                 needsTemp = true
                 break checkIfArgNeedsTemp
 

compiler/mir/mirtrees.nim

@@ -358,10 +358,10 @@ const
     ## Node kinds only allowed in an output context directly inside an
     ## arg-block
 
-  SingleInputNodes* = {mnkAddr, mnkDeref, mnkDerefView, mnkCast, mnkConv,
-                       mnkStdConv, mnkPathNamed, mnkPathPos, mnkPathVariant,
-                       mnkPathConv, mnkTag, mnkIf, mnkCase, mnkRaise,
-                       mnkVoid} + ArgumentNodes
+  SingleInputNodes* = {mnkAddr, mnkDeref, mnkView, mnkDerefView, mnkCast,
+                       mnkConv, mnkStdConv, mnkPathNamed, mnkPathPos,
+                       mnkPathVariant, mnkPathConv, mnkTag, mnkIf, mnkCase,
+                       mnkRaise, mnkVoid} + ArgumentNodes
     ## Operators and statements that must not have argument-blocks as input
 
   StmtNodes* = {mnkScope, mnkRepeat, mnkTry, mnkBlock, mnkBreak, mnkReturn,
@@ -617,10 +617,14 @@ func numArgs*(tree: MirTree, op: Operation): int =
   else:
     unreachable("no arg-block is used")
 
-func unaryOperand*(tree: MirTree, op: Operation): OpValue =
-  # XXX: a 'def' node is not an operation
+func operand*(tree: MirTree, op: Operation|OpValue|NodePosition): OpValue =
+  ## Returns the index (``OpValue``) of operand for the single-input node at
+  ## `op`.
   assert tree[op].kind in SingleInputNodes + DefNodes
-  result = OpValue getStart(tree, NodePosition(op) - 1)
+  let pos =
+    when op is NodePosition: op
+    else:                    NodePosition(op)
+  OpValue getStart(tree, pos - 1)
 
 func hasInput*(tree: MirTree, op: Operation): bool =
   # XXX: a 'def' node is not an operation

compiler/sem/injectdestructors.nim

@@ -329,7 +329,7 @@ func getDefEntity(tree: MirTree, n: NodePosition): NodePosition =
 func skipTag(tree: MirTree, n: Operation): OpValue =
   ## Returns the input to the tag operation `n`
   assert tree[n].kind == mnkTag
-  unaryOperand(tree, n)
+  tree.operand(n)
 
 # --------- compute routines ---------------
 
@@ -434,7 +434,7 @@ func solveOwnership(tree: MirTree, cfg: ControlFlowGraph, values: var Values,
   for i, n in tree.pairs:
     case n.kind
     of ConsumeCtx:
-      let opr = unaryOperand(tree, Operation i)
+      let opr = tree.operand(i)
 
       if values.owned(opr) in {unknown, weak} and hasDestructor(tree[opr].typ):
         # unresolved onwership status and has a destructors
@@ -880,7 +880,7 @@ proc rewriteAssignments(tree: MirTree, ctx: AnalyseCtx, ar: AnalysisResults,
       # passing a value to the ``raise`` operation or as the initial value of
       # a temporary used for tuple unpacking also requires consuming it
       let
-        opr = unaryOperand(tree, Operation i)
+        opr = tree.operand(i)
         typ = tree[opr].typ
 
       if tree[opr].kind == mnkNone or not hasDestructor(typ):
@@ -905,7 +905,7 @@ proc rewriteAssignments(tree: MirTree, ctx: AnalyseCtx, ar: AnalysisResults,
         user = Operation(findEnd(tree, parent(tree, i)) + 1) ## the consumer
         # XXX: 'consume' is not an operation -- it's an argument sink. It
         #      might make sense to introduce a new type for those
-        val = unaryOperand(tree, Operation i)
+        val = tree.operand(i)
 
       case tree[user].kind
       of mnkConstr, mnkObjConstr, mnkCall, mnkMagic: