mir: add a dedicated handle conversion operation (#935)

## Summary

Introduce a dedicated handle conversion operator (`mnkPathConv`) to the
MIR, leaving `mnkConv` to only represent conversions producing new (non
handle-like) values.

This clarifies the semantics around MIR-level conversions a bit, fixing
the internal-only issue of `mnkConv` and `mnkStdConv` being sometimes
treated as if they always represented handle conversion.

## Details

A value conversion (e.g., `int(f)` where `f` is a float value) produces
a new value -- the result cannot be assigned to. A *handle* conversion
produces a new *handle* to the same location as that of the input
value.

The distinction is important at the MIR level, as it affects ownership
and evaluation semantics. `mnkConv` was previously used to represent
both kinds of conversions, but since the type comparison required for
detecting the conversion kind is rather costly, `mnkConv` was so far
almost always treated as the one the context required.

The current structure of the MIR lets one get away with this, but it's
still incorrect, and it's also in the way of MIR improvements. In
addition,  `mnkStdConv`  always represents value conversions, and it
being
used/skipped where  `mnkConv`  is treated as a handle conversion is
always
wrong.

In order to remove the ambiguity, the new `mnkPathConv` operator is
introduced. Since it too represents a path/projection, it is grouped
together with the other path operations. For simplicity, `mnkPathConv`
also applies to first-class handle-like values (`ref`, `ptr`, `var`,
etc.) for now -- they are presently treated the same as L-value
conversions.

* add the `mnkPathConv` operation. Syntax-wise, it's a single-input,
  result-yielding operation
* add `mnkPathConv` support to the MIR construction DSL
* emit an `mnkPathConv` instead of `mnkConv` for object/ref up- and
  down-conversions
* emit a `mnkPathConv` instead of `mnkConv` for conversions between
  distinct types and their base (and vice versa)

The places where `mnkConv` was previously treated as a handle
conversions are changed to use `mnkPathConv`. `mirpasses.isRvalue`,
which had to conservatively exclude it previously, now includes the
`mnkConv` operation.

There's also an issue in `transf` that had to be fixed: all up- and
down- conversions are expected to use `nkObjUpConv` and `nkObjDownConv`
nodes past `transf` (see `transformConv`), but lowering an exception
handler branch (`transformExceptBranch`) emitted a raw
`nkHiddenSubConv` for down conversions.

compiler/backend/cgirgen.nim

@@ -764,8 +764,8 @@ proc handleSpecialConv(c: ConfigRef, n: CgNode, info: TLineInfo,
     result = nil
 
 proc tbConv(cl: TranslateCl, n: CgNode, info: TLineInfo, dest: PType): CgNode =
-  ## Generates the IR for an expression that performs a type conversion for
-  ## `n` to type `dest`
+  ## Generates the ``CgNode`` IR for an ``mnkPathConv`` operation (handle
+  ## conversion).
   result = handleSpecialConv(cl.graph.config, n, info, dest)
   if result == nil:
     # no special conversion is used
@@ -1150,7 +1150,7 @@ proc tbInOut(tree: TreeWithSource, cl: var TranslateCl, prev: sink Values,
   of mnkCast:
     toValues newOp(cnkCast, info, n.typ, prev.single)
   of mnkConv:
-    toValues tbConv(cl, prev.single, info, n.typ)
+    toValues newOp(cnkConv, info, n.typ, prev.single)
   of mnkStdConv:
     let
       opr = prev.single
@@ -1214,6 +1214,8 @@ proc tbInOut(tree: TreeWithSource, cl: var TranslateCl, prev: sink Values,
 
   of mnkPathArray:
     toValues newExpr(cnkBracketAccess, info, n.typ, move prev.list)
+  of mnkPathConv:
+    toValues tbConv(cl, prev.single, info, n.typ)
   of mnkAddr:
     toValues newOp(cnkAddr, info, n.typ, prev.single)
   of mnkDeref:

compiler/mir/analysis.nim

@@ -117,15 +117,11 @@ const
     ## if an lvalue is used as an operand to these operators, the value stored
     ## in the named location is considered to be consumed (ownership over it
     ## transfered to the operation)
-  UseContext* = {mnkArg, mnkDeref, mnkDerefView, mnkCast, mnkVoid, mnkIf,
-                 mnkCase} + ConsumeCtx
+  UseContext* = {mnkArg, mnkDeref, mnkDerefView, mnkStdConv, mnkConv, mnkCast,
+                 mnkVoid, mnkIf, mnkCase} + ConsumeCtx
     ## using an lvalue as the operand to one of these operators means that
     ## the content of the location is observed (when control-flow reaches the
-    ## operator). In other words, applying the operator result in a read
-    # FIXME: none-lvalue-conversions also count as reads, but because no
-    #        distinction is being made between lvalue- and value-conversions
-    #        at the MIR level, they're currently not considered. This is an
-    #        issue and it needs to be fixed
+    ## operator). In other words, applying the operator results in a read
 
   OpsWithEffects = {mnkCall, mnkMagic, mnkAsgn, mnkFastAsgn, mnkSwitch,
                     mnkInit, mnkRegion}
@@ -135,9 +131,10 @@ const
 func hash(x: Operation): Hash {.borrow.}
 
 func skipConversions(tree: MirTree, val: OpValue): OpValue =
-  ## Returns the value without conversions applied
+  ## Returns the producing operation after skipping handle-only
+  ## conversions.
   var p = NodePosition(val)
-  while tree[p].kind in {mnkStdConv, mnkConv}:
+  while tree[p].kind == mnkPathConv:
     p = previous(tree, p)
 
   result = OpValue(p)
@@ -256,7 +253,9 @@ func computeValuesAndEffects*(body: MirTree): Values =
         if directViewType(n.typ) != noView: Owned.no
         else: Owned.weak
     of mnkStdConv, mnkConv:
-      inherit(i, i - 1)
+      # non-lvalue conversions produces a new unique value, meaning that
+      # the result is always owned
+      start: Owned.yes
     of mnkAddr, mnkView, mnkPathPos, mnkPathVariant:
       inheritDecay(i, i - 1)
     of mnkPathArray:
@@ -267,6 +266,8 @@ func computeValuesAndEffects*(body: MirTree): Values =
       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)
 
     of mnkArgBlock:
       num.add stack.len.uint16 # remember the current top-of-stack
@@ -603,14 +604,13 @@ proc doesGlobalEscape*(tree: MirTree, scope: Slice[NodePosition],
 
 func isConsumed*(tree: MirTree, val: OpValue): bool =
   ## Computes if `val` is definitely consumed. This is the case if it's
-  ## directly used in a consume context, ignoring lvalue conversions
+  ## directly used in a consume context, ignoring handle conversions.
   var dest = NodePosition(val)
   while true:
     dest = sibling(tree, dest)
 
     case tree[dest].kind
-    of mnkConv, mnkStdConv:
-      # XXX: only lvalue conversions should be skipped
+    of mnkPathConv:
       discard "skip conversions"
     of ConsumeCtx:
       return true

compiler/mir/mirconstr.nim

@@ -242,6 +242,12 @@ func pathVariant*(s: var MirNodeSeq, objType: PType, field: PSym, val: var EValu
   s.add MirNode(kind: mnkPathVariant, typ: objType, field: field)
   val.typ = objType
 
+func pathConv*(s: var MirNodeSeq, typ: PType, val: var EValue) =
+  ## Emits an ``mnkPathConv`` node, representing a handle-conversion to
+  ## `typ`.
+  s.add MirNode(kind: mnkPathConv, typ: typ)
+  val.typ = typ
+
 func unaryMagicCall*(s: var MirNodeSeq, m: TMagic, typ: PType, val: var EValue) =
   assert typ != nil
   s.add MirNode(kind: mnkMagic, typ: typ, magic: m)
@@ -324,6 +330,7 @@ genValueAdapter1(viewOp, typ)
 genValueAdapter1(derefOp, typ)
 genValueAdapter1(derefViewOp, typ)
 genValueAdapter1(pathObj, field)
+genValueAdapter1(pathConv, typ)
 genValueAdapter2(pathPos, typ, pos)
 genValueAdapter2(pathVariant, typ, field)
 genValueAdapter2(unaryMagicCall, m, typ)

compiler/mir/mirgen.nim

@@ -1538,7 +1538,7 @@ proc genx(c: var TCtx, n: PNode, consume: bool): EValue =
   of nkBracketExpr:
     genBracketExpr(c, n)
   of nkObjDownConv, nkObjUpConv:
-    eval(c): genx(c, n[0], consume) => convOp(n.typ)
+    eval(c): genx(c, n[0], consume) => pathConv(n.typ)
   of nkAddr:
     eval(c): genx(c, n[0]) => addrOp(n.typ)
   of nkHiddenAddr:
@@ -1571,7 +1571,12 @@ proc genx(c: var TCtx, n: PNode, consume: bool): EValue =
   of nkHiddenStdConv:
     eval(c): genx(c, n[1], consume) => stdConvOp(n.typ)
   of nkHiddenSubConv, nkConv:
-    eval(c): genx(c, n[1], consume) => convOp(n.typ)
+    if compareTypes(n.typ, n[1].typ, dcEqIgnoreDistinct, {IgnoreTupleFields}):
+      # it's an lvalue-preserving conversion
+      eval(c): genx(c, n[1], consume) => pathConv(n.typ)
+    else:
+      # it's a conversion that produces a new rvalue
+      eval(c): genx(c, n[1], consume) => convOp(n.typ)
   of nkLambdaKinds:
     procLit(c, n[namePos].sym)
   of nkChckRangeF, nkChckRange64, nkChckRange:

compiler/mir/mirpasses.nim

@@ -53,8 +53,8 @@ func isRvalue(tree: MirTree, a: OpValue): bool {.inline.} =
   ## doesn't have a name and cannot be assigned to. Only checks the
   ## immediate operator that yield the value.
   tree[a].kind in { mnkNone, mnkProc, mnkType, mnkLiteral, mnkConstr,
-                    mnkObjConstr, mnkCall, mnkStdConv, mnkCast, mnkAddr,
-                    mnkView }
+                    mnkObjConstr, mnkCall, mnkStdConv, mnkConv, mnkCast,
+                    mnkAddr, mnkView }
 
 func isArgBlock(tree: MirTree, n: NodePosition): bool =
   ## Returns whether the node `n` is the end-node of an arg-block.
@@ -66,16 +66,16 @@ func getRoot(tree: MirTree, n: OpValue): NodePosition =
   ## - the name of a location (e.g., ``a`` in ``a.b.c``)
   ## - an r-value (e.g., ``call()`` in ``call().b.c``)
   const PathNodes = { mnkPathArray, mnkPathNamed, mnkPathPos, mnkPathVariant,
-                      mnkConv }
+                      mnkPathConv }
     ## all operations that (can) take an lvalue as input and produce
-    ## lvalue
+    ## an lvalue
 
   var i = n
   while tree[i].kind in PathNodes:
     case tree[i].kind
     of mnkPathNamed, mnkPathPos, mnkPathVariant:
       i = OpValue(NodePosition(i) - 1)
-    of mnkConv:
+    of mnkPathConv:
       i = unaryOperand(tree, Operation(i))
     of mnkPathArray:
       i = operand(tree, Operation(i), 0)
@@ -357,7 +357,7 @@ proc fixupCallArguments(tree: MirTree, config: ConfigRef,
       while true:
         case tree[x].kind
         of mnkAddr, mnkView, mnkDeref, mnkDerefView, mnkConv, mnkStdConv,
-           mnkTag:
+           mnkTag, mnkPathConv:
           x = unaryOperand(tree, x)
         of mnkPathNamed, mnkPathPos, mnkPathVariant:
           result.add NodePosition(x)

compiler/mir/mirtrees.nim

@@ -108,6 +108,9 @@ type
                    ##      record-case part of an object should be its own
                    ##      dedicated object type, which can then be addressed
                    ##      as a normal field
+    mnkPathConv  ## an handle conversion. That is, a conversion that produces a
+                 ## *handle*, and not a new *value*. At present, this operator
+                 ## also applies to first-class handles, like ``ref``.
 
     mnkAddr   ## ``addr(x)``; creates a first-class unsafe alias/handle (i.e.
               ## pointer) from the input lvalue `x`
@@ -126,10 +129,10 @@ type
     # XXX: ``mnkDerefView`` is not used for ``openArray`` right now, due to
     #      the latter's interactions with ``var`` and ``lent``
 
-    mnkStdConv    ## ``stdConv(x)``; a standard conversion. Depending on the
-                  ## source and target type, lvalue-ness is preserved
-    mnkConv       ## ``conv(x)``; a conversion. Depending on the source and
-                  ## target type, lvalue-ness is preserved
+    mnkStdConv    ## ``stdConv(x)``; a standard conversion.Produce a new value.
+                  ## Also used for to-slice (``openArray``) conversions, in
+                  ## which case the semantics are still fuzzy.
+    mnkConv       ## ``conv(x)``; a conversion. Produces a new value.
     # XXX: distinguishing between ``stdConv`` and ``conv`` is only done to
     #      make ``cgirgen`` a bit more efficient. Further progress should focus
     #      on removing the need for it
@@ -339,7 +342,7 @@ const
   InputNodes* = {mnkProc..mnkNone, mnkArgBlock}
     ## Nodes that can appear in the position of inputs/operands but that
     ## themselves don't have any operands
-  InOutNodes* = {mnkMagic, mnkCall, mnkPathNamed..mnkPathVariant, mnkConstr,
+  InOutNodes* = {mnkMagic, mnkCall, mnkPathNamed..mnkPathConv, mnkConstr,
                  mnkObjConstr, mnkView, mnkTag, mnkCast, mnkDeref, mnkAddr,
                  mnkDerefView, mnkStdConv, mnkConv}
     ## Operations that act as both input and output
@@ -357,8 +360,8 @@ const
 
   SingleInputNodes* = {mnkAddr, mnkDeref, mnkDerefView, mnkCast, mnkConv,
                        mnkStdConv, mnkPathNamed, mnkPathPos, mnkPathVariant,
-                       mnkTag, mnkIf, mnkCase, mnkRaise, mnkVoid} +
-                      ArgumentNodes
+                       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,

compiler/sem/aliasanalysis.nim

@@ -75,14 +75,13 @@ proc compareLvalues(body: MirTree, a, b: NodePosition,
     a = a + 1
     b = b + 1
 
-  const IgnoreSet = {mnkStdConv, mnkConv, mnkAddr, mnkView}
-    ## we can skip all conversions, since we know that they must be lvalue
-    ## conversions; ``compareLvalues`` is only used for lvalues, which
-    ## can't result from non-lvalue conversions.
+  const IgnoreSet = {mnkPathConv, mnkStdConv, mnkAddr, mnkView}
+    ## We skip l-value conversions, since they don't change the location.
     ## Both the 'addr' and 'view' operatio create a first-class handle to
-    ## the reference location, so we also skip them. This allows for ``a.x``
+    ## the referenced location, so we also skip them. This allows for ``a.x``
     ## to be treated as refering to same location as ``a.x.addr`` (which is
-    ## correct)
+    ## correct). For to-openArray conversions, this is the case too, so
+    ## ``mnkStdConv`` is also included here.
 
   var overlaps = yes # until proven otherwise
   while overlaps != no and a <= lastA and b <= lastB:

compiler/sem/injectdestructors.nim

@@ -613,9 +613,9 @@ func undoConversions(buf: var MirNodeSeq, tree: MirTree, src: OpValue) =
   ## the conversions in *reverse*. ``cgirgen`` detects this pattern and removes
   ## the conversions that cancel each other out.
   var p = NodePosition(src)
-  while tree[p].kind in {mnkStdConv, mnkConv}:
+  while tree[p].kind == mnkPathConv:
     p = previous(tree, p)
-    buf.add MirNode(kind: mnkConv, typ: tree[p].typ)
+    buf.add MirNode(kind: mnkPathConv, typ: tree[p].typ)
 
 template voidCallWithArgs(buf: var MirNodeSeq, body: untyped) =
   argBlock(buf):

compiler/sem/transf.nim

@@ -1057,8 +1057,8 @@ proc transformExceptBranch(c: PTransf, n: PNode): PNode =
     # -> getCurrentException()
     let excCall = callCodegenProc(c.graph, "getCurrentException")
     # -> (excType)
-    let convNode = newTreeIT(nkHiddenSubConv, n[1].info, excTypeNode.typ.toRef(c.idgen)):
-      [newNodeI(nkEmpty, n.info), excCall]
+    let convNode = newTreeIT(nkObjDownConv, n[1].info, excTypeNode.typ.toRef(c.idgen)):
+      [excCall]
     # -> let exc = ...
     let identDefs = newTreeI(nkIdentDefs, n[1].info):
       [n[0][2], newNodeI(nkEmpty, n.info), convNode]

doc/mir.rst

@@ -151,8 +151,8 @@ in memory:
   out-op    = "void" | "raise" | "init" | "fastAsgn" | "asgn" | "switch" |
               "asm" | "emit" | def | if-stmt | case-stmt | region
   in-out-op = "magic" | "call" | "pathNamed" | "pathPos" | "pathArray" |
-              "pathVariant" | "constr" | obj-constr | "cast" | "deref" |
-              "addr" | "stdConv" | "conv" | "view" | "derefView"
+              "pathVariant" | "pathConv" | "constr" | obj-constr | "cast" |
+              "deref" | "addr" | "stdConv" | "conv" | "view" | "derefView"
 
   sequence = in-op, {in-out-op}, out-op