Compilation errors with `-d:chronosHandleException`

[gmega]

As part of our V4 migration strategy we've been investigating using the V3 "compatibility" mode (i.e., compiling with -d:chronosHandleException) with Chronos, but I'm running into a compilation issue (nim 1.6.20, Linux x86_64) . The following example:

import chronos

proc myProc() {.async.} = 
  echo "hello"

will fail with:

Error: expression 'cbproc(transp, remote)' is of type 'Future[system.void]' and has to be used (or discarded); start of expression here: /.../chronos/transports/datagram.nim(635, 5)

Looking at the offending code, I see:

proc wrap(transp: DatagramTransport, remote: TransportAddress) {.async: (raises: []).} =
  try:
    cbproc(transp, remote)
  except CatchableError as exc:
    raiseAssert "Unexpected exception from stream server cbproc: " & exc.msg

where cbproc is of type:

UnsafeDatagramCallback* = proc(transp: DatagramTransport,
                           remote: TransportAddress): Future[void] {.async.}

I suppose the intent here is to make this into a safe DatagramCallback:

DatagramCallback* = proc(transp: DatagramTransport,
                           remote: TransportAddress): Future[void] {.
                           async: (raises: []).}

at which point this seems to be missing a call to await?

As a side note, this code as it is now is pretty strange even when it compiles - the Future returned by cbproc is never set as a return value in the expansion of wrap (produced with -d:chronosDumpAsync); i.e., it doesn't seem to be calling setResult as other procs do:

proc wrap ... =
  iterator wrap_1895826090(chronosInternalRetFuture: FutureBase): FutureBase {.
      closure, gcsafe, raises: [].} =
    template result(): auto {.used.} = ...

    var closureSucceeded_1895826089 = true
    # no setResult do:
    try:
      try:
        cbproc(transp, remote)
      except CatchableError as exc:
        raiseAssert "Unexpected exception from stream server cbproc: " & exc.msg
    except Defect as exc:
      closureSucceeded_1895826089 = false
      raise exc
    finally:
      if closureSucceeded_1895826089:
        complete(cast[Future[void]](chronosInternalRetFuture))

I tried to figure out the implication of this by doing some testing on iterators and seeing what happens to those "fall through" values, and it looks like they're simply lost; e.g.:

proc proc1(): iterator = 
  iterator: int {.closure.} = 2

proc proc2(): iterator = 
  iterator: int {.closure.} = return 2

echo proc1()() # prints 0
echo proc2()() # prints 2

so not really sure what happens to the Future returned from cbproc after the call (guess it just runs as if we had done discard cbproc(...)?).

I'm opening a PR adding the calls to await as that seems to fix it, doesn't break anything, and seems to reflect the actual intent, but considering the above I'm not sure that's the only issue.

[dryajov]

at which point this seems to be missing a call to await?

Yes, proc aProc() {.async.} = echo "hello" transforms the signature into proc aProc(): Future[void] = ..., so all async annotations need to be either awaited or discarded.

As a side note, this code as it is now is pretty strange even when it compiles as the Future won't ever be set as a return value as the expansion of wrap does not call setResult:

I suspect that that is not the full transpiled code, must be missing something.