cgirgen: remove the var expression fixup (#1482)

## Summary

Remove a fixup in `cgirgen` related to `var` parameters in `cgirgen`
and correct the AST construction necessitating it. There's no change in
language semantics.

## Details

In typed `PNode` AST, `var` parameters *always* have to be dereferenced
first, including when being passed to other `var` parameters (resulting
in `(HiddenAddr (HiddenDeref x))` trees). Multiple transformations /
places where AST is constructed in the compiler didn't produce AST
adhering to this.

The result was incorrect MIR code being generated (because the location
*storing* the `var` parameter was being passed to another parameter,
not the location the `var` parameter *references*), though this didn't
cause any trouble so far. `cgirgen` detected this situation and
adjusted its output accordingly, producing well-formed CGIR AST.

The sources of the problematic `PNode` AST were:
* parameter hoisting in `semexprs`
* hook lifting in `liftdestructors`
* method dispatcher generation in `cgmeth`

They're changed to produce AST adhering to the aforementioned
requirements, and the fixup in `cgirgen` is removed.

---------

Co-authored-by: Saem Ghani <saemghani+github@gmail.com>

compiler/backend/cgirgen.nim

@@ -166,18 +166,6 @@ proc newDefaultCall(info: TLineInfo, typ: PType): CgNode =
   ## Produces the tree for a ``default`` magic call.
   newExpr(cnkCall, info, typ, [newMagicNode(mDefault, info)])
 
-proc wrapInHiddenAddr(cl: TranslateCl, n: CgNode): CgNode =
-  ## Restores the ``cnkHiddenAddr`` around lvalue expressions passed to ``var``
-  ## parameters. The code-generators operating on ``CgNode``-IR depend on the
-  ## hidden addr to be present
-  if n.typ.skipTypes(abstractInst).kind != tyVar:
-    newOp(cnkHiddenAddr, n.info, makeVarType(cl.owner, n.typ, cl.idgen), n)
-  else:
-    # XXX: is this case ever reached? It should not be. Raw ``var`` values
-    #      must never be passed directly to ``var`` parameters at the MIR
-    #      level
-    n
-
 proc genObjConv(n: CgNode, to: PType, info: TLineInfo): CgNode =
   ## Depending on the type relationship between `n` and `to`, wraps `n` in
   ## either an up- or down-conversion. Returns `nil` if no up- or down-
@@ -419,7 +407,10 @@ proc callToIr(tree: MirBody, cl: var TranslateCl, n: MirNode,
         # XXX: prevent this case from happening
         arg = newOp(cnkDerefView, arg.info, arg.typ.base, arg)
     elif mutable:
-      arg = wrapInHiddenAddr(cl, arg)
+      # much like in PNode AST, the CGIR AST also needs a ``cnkHiddenAddr``
+      # tree wrapped around expressions in var argument positions
+      arg = newOp(cnkHiddenAddr, arg.info,
+                  makeVarType(cl.owner, arg.typ, cl.idgen), arg)
 
     result.add arg
 

compiler/backend/cgmeth.nim

@@ -284,15 +284,23 @@ proc genDispatcher(g: ModuleGraph; methods: seq[PSym], relevantCols: IntSet) =
       if param.typ.skipTypes(abstractInst).kind in {tyRef, tyPtr}:
         nilchecks.add newTree(nkCall,
             newSymNode(getCompilerProc(g, "chckNilDisp")), newSymNode(param))
+
+  proc param(s: PSym): PNode {.nimcall.} =
+    if s.typ.kind == tyVar:
+      newTreeIT(nkHiddenAddr, s.info, s.typ,
+        newTreeIT(nkHiddenDeref, s.info, s.typ.base,
+          newSymNode(s)))
+    else:
+      newSymNode(s)
+
   for meth in 0..high(methods):
     var curr = methods[meth]      # generate condition:
     var cond: PNode = nil
     for col in 1..<paramLen:
       if contains(relevantCols, col):
         var isn = newNodeIT(nkCall, base.info, boolType)
         isn.add newSymNode(iss)
-        let param = base.typ.n[col].sym
-        isn.add newSymNode(param)
+        isn.add param(base.typ.n[col].sym)
         isn.add newNodeIT(nkType, base.info, curr.typ[col])
         if cond != nil:
           var a = newNodeIT(nkCall, base.info, boolType)
@@ -306,8 +314,8 @@ proc genDispatcher(g: ModuleGraph; methods: seq[PSym], relevantCols: IntSet) =
     let call = newNodeIT(nkCall, base.info, retTyp)
     call.add newSymNode(curr)
     for col in 1..<paramLen:
-      call.add genConv(newSymNode(base.typ.n[col].sym),
-                           curr.typ[col], true, g.config)
+      call.add genConv(param(base.typ.n[col].sym),
+                       curr.typ[col], true, g.config)
     var ret: PNode
     if retTyp != nil:
       var a = newNodeI(nkFastAsgn, base.info)

compiler/sem/liftdestructors.nim

@@ -106,11 +106,7 @@ proc defaultOp(c: var TLiftCtx; t: PType; body, x, y: PNode) =
     body.add newAsgnStmt(x, call)
 
 proc genAddr(c: var TLiftCtx; x: PNode): PNode =
-  if x.kind == nkHiddenDeref:
-    checkSonsLen(x, 1, c.g.config)
-    result = x[0]
-  else:
-    result = newTreeIT(nkHiddenAddr, x.info, makeVarType(x.typ.owner, x.typ, c.idgen)): x
+  newTreeIT(nkHiddenAddr, x.info, makeVarType(x.typ.owner, x.typ, c.idgen), x)
 
 proc genWhileLoop(c: var TLiftCtx; i, dest: PNode): PNode =
   let cmp = genBuiltin(c, mLtI, "<", i)
@@ -268,12 +264,7 @@ proc getCycleParam(c: TLiftCtx): PNode =
 proc newHookCall(c: var TLiftCtx; op: PSym; x, y: PNode): PNode =
   #if sfError in op.flags:
   #  localReport(c.config, x.info, "usage of '$1' is a user-defined error" % op.name.s)
-  result = newNodeI(nkCall, x.info)
-  result.add newSymNode(op)
-  if op.typ.sons[1].kind == tyVar:
-    result.add genAddr(c, x)
-  else:
-    result.add x
+  result = newTreeI(nkCall, x.info, newSymNode(op), genAddr(c, x))
   if y != nil:
     result.add y
   if op.typ.len == 4:
@@ -889,7 +880,8 @@ proc produceSym(g: ModuleGraph; c: PContext; typ: PType; kind: TTypeAttachedOp;
   if kind == attachedSink and destructorOverriden(g, typ):
     ## compiler can use a combination of `=destroy` and memCopy for sink op
     dest.flags.incl sfCursor
-    result.ast[bodyPos].add newOpCall(a, getAttachedOp(g, typ, attachedDestructor), d[0])
+    result.ast[bodyPos].add newOpCall(a, getAttachedOp(g, typ, attachedDestructor),
+                                      genAddr(a, d))
     result.ast[bodyPos].add newAsgnStmt(d, src)
   else:
     let (tk, baseTyp) =

compiler/sem/semexprs.nim

@@ -3491,10 +3491,22 @@ proc hoistParamsUsedInDefault(c: PContext, call, letSection, defExpr: var PNode)
         newNodeI(nkEmpty, letSection.info),
         call[paramPos])
 
-      call[paramPos] = newSymNode(hoistedVarSym) # Refer the original arg to its hoisted sym
+      if hoistedVarSym.typ.kind == tyVar:
+        # only ``HiddenAddr`` expressions may be of ``var`` type in argument
+        # positions
+        call[paramPos] =
+          newTreeIT(nkHiddenAddr, letSection.info, hoistedVarSym.typ,
+            newDeref(newSymNode(hoistedVarSym)))
+      else:
+        # Refer the original arg to its hoisted sym
+        call[paramPos] = newSymNode(hoistedVarSym)
 
-    # arg we refer to is a sym, wether introduced by hoisting or not doesn't matter, we simply reuse it
-    defExpr = call[paramPos]
+    # arg is either a sym, whether introduced by hoisting or not doesn't
+    # matter, or a ``(HiddenAddr (HiddenDeref sym))`` introduced by hoisting
+    if call[paramPos].kind == nkHiddenAddr:
+      defExpr = call[paramPos][0][0] # retrieve the symbol
+    else:
+      defExpr = call[paramPos] # must be a symbol
   else:
     for i in 0..<defExpr.safeLen:
       hoistParamsUsedInDefault(c, call, letSection, defExpr[i])