status-im / nim-chronos

Chronos - An efficient library for asynchronous programming
https://status-im.github.io/nim-chronos/docs/chronos
Apache License 2.0
353 stars 51 forks source link

Fix Future[void] async procedure in generic #328

Closed Menduist closed 1 year ago

Menduist commented 1 year ago
  test "Future void in generic bug":
    proc gen(T: typedesc[int]): T =
      proc testproc(): Future[void] {.async.} = discard
      T(5)
    let test = int.gen()

Currently fails

Upstream has a nicer fix: https://github.com/nim-lang/Nim/pull/17633 which also supports aliasing, but it's more invasive, so not sure if we should also integrate it

Menduist commented 1 year ago

Tried a fix similar to upstream, but it changes the result behavior in Future[void]:

proc x {.async.} =
  echo result # Fails with Error: type mismatch: got <void> [..] but expression 'raiseAssert "You should not reference the `result` variable inside a void async proc"' is of type: void
  result = 12 # Fails with Error: expression 'raiseAssert "You should not reference the `result` variable inside a void async proc"' has no type (or is ambiguous)
  result # Fails AT RUNTIME with `Unhandled exception: You should not reference the `result` variable inside a void async proc`

Unfortunately, I don't see a simple fix to keep the current behavior of keeping the failure at compile time for every case, but maybe someone will have better luck

arnetheduck commented 1 year ago

the idea LGTM but we should stay with newNimNode for the result variable itself so that assignments to it point to the right code - can you rebase?