eraiser: result = v projects onto the value half (#2568)

A raising routine's `result = v` was rebuilt as `result = (Success, v)`.
That nests `v` one level deep, and `cps` only ends a coroutine state
where a suspension point is the ROOT of an assignment's value: a
`.passive` call there was invisible, so `result = passiveCall()` in a
raising coroutine returned 0, or failed with `cps.nim: state != -1` in
tail position.

The rebuild was redundant anyway: `result[0] = Success` is set at the
declaration and every path that changes it leaves the routine, and a
retyped local's check has already passed. So the assignment is simply
`result[1] = v`, which the generic path already produces. `trAsgn` is
gone.

Author: Araq

src/hexer/eraiser.nim

@@ -18,9 +18,14 @@ one pass that implements them.
   stood for becomes `tmp[1]`.
 - `let/var local = rcall(args)` retypes `local` to the success tuple and gets
   that same check; every other use of `local` projects onto `local[1]`.
-- `result = x` and `return x` build the tuple. `result` itself already IS the
-  tuple, so `return result` needs no rebuild — and must not get one, see
-  `trRet`.
+- `result[0] = Success` is set once, at `result`'s declaration: every path
+  that changes the code half leaves the routine right away. So `result = x`,
+  like any other use of `result` (and of a retyped local, whose check has
+  passed), is simply `result[1] = x`. Rebuilding the whole tuple there would
+  nest `x` one level deep, which hides a `.passive` call from `cps`: it ends
+  a state only where a suspension point is the ROOT of the value.
+- `return x` builds the tuple. `result` itself already IS the tuple, so
+  `return result` needs no rebuild — and must not get one, see `trRet`.
 - `raise e` becomes `raise (e, result)`.
 
 **Doing the whole job here is the point.** The control-flow half (the temps
@@ -727,24 +732,6 @@ proc trTry(c: var Context; dest: var TokenBuf; n: var Cursor) =
   n = tryStart
   skip n
 
-proc trAsgn(c: var Context; dest: var TokenBuf; n: var Cursor) =
-  var nn = n.childCursor
-  if nn.kind == Symbol and ((nn.symId == c.resultSym and c.canRaise) or
-                            c.tupleVars.contains(nn.symId)):
-    let isResultSym = nn.symId == c.resultSym
-    copyInto dest, n:
-      dest.addSubtree n  # the destination, NOT projected: it IS the tuple
-      inc n
-      let typ = if isResultSym: c.retType else: getType(c.typeCache, n)
-      let maybeClose = produceSuccessTuple(c, dest, typ, n.info)
-      tr c, dest, n
-      if maybeClose:
-        dest.addParRi() # tuple constructor
-  else:
-    copyInto dest, n:
-      tr c, dest, n
-      tr c, dest, n
-
 proc trBreak(c: var Context; dest: var TokenBuf; n: var Cursor) =
   ## Leaving a `block` or a loop runs the `finally` of every `try` between
   ## here and it — but not of any `try` further out, which we are still in.
@@ -848,8 +835,6 @@ proc tr(c: var Context; dest: var TokenBuf; n: var Cursor) =
         trScope c, dest, n
       of StmtsS:
         trStmtList c, dest, n
-      of AsgnS:
-        trAsgn c, dest, n
       of RetS:
         trRet c, dest, n
       of RaiseS:
@@ -864,7 +849,7 @@ proc tr(c: var Context; dest: var TokenBuf; n: var Cursor) =
         trLoopOrBlock c, dest, n
       of MacroS, TemplateS, TypeS:
         takeTree dest, n
-      of CallS, CmdS, IteratorS, EmitS, IfS, WhenS,
+      of AsgnS, CallS, CmdS, IteratorS, EmitS, IfS, WhenS,
          ContinueS, ForS, CaseS, YldS,
          PragmasS, PragmaxS, InclS, ExclS, IncludeS, ImportS, ImportasS,
          FromimportS, ImportexceptS, ExportS, ExportexceptS, CommentS,

tests/nimony/cps/tpassiveraises_result.nim

@@ -0,0 +1,53 @@
+# `result = passiveCall()` inside a raising coroutine.
+#
+# `cps` ends a state where it finds a suspension point at the ROOT of a
+# statement. The eraiser used to rewrite `result = v` into
+# `result = (Success, v)`, which pushed the passive call one level down: the
+# state boundary then fell after the whole assignment, and the routine either
+# returned 0 or did not compile at all (`cps.nim: state != -1`).
+
+import std / syncio
+
+proc step() {.passive.} = discard
+
+proc get(x: int): int {.passive.} =
+  step()
+  result = x * 10
+
+proc viaResult(x: int): int {.passive, raises.} =
+  if x < 0: raise SyntaxError
+  result = get(x)
+
+proc tail(x: int): int {.passive, raises.} =
+  if x < 0: raise SyntaxError
+  get(x)
+
+proc viaReturn(x: int): int {.passive, raises.} =
+  if x < 0: raise SyntaxError
+  return get(x)
+
+proc reassign(x: int): int {.passive, raises.} =
+  var y = viaResult(x)  # `y` holds the success tuple
+  y = get(y)            # a passive, non-raising call into it
+  result = y
+
+proc raiseAfter(x: int): int {.passive, raises.} =
+  result = get(x)
+  if result > 30: raise SyntaxError
+
+proc main() {.passive.} =
+  for i in [3, -1]:
+    try:
+      echo viaResult(i)
+      echo tail(i)
+      echo viaReturn(i)
+      echo reassign(i)
+      echo raiseAfter(i)
+    except:
+      echo "caught ", i
+  try:
+    echo raiseAfter(5)
+  except:
+    echo "caught 5"
+
+main()

tests/nimony/cps/tpassiveraises_result.output

@@ -0,0 +1,7 @@
+30
+30
+30
+300
+30
+caught -1
+caught 5