Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Why isn’t chronosInternalRetFuture gensymmed? #368

Open
SirNickolas opened this issue Mar 9, 2023 · 4 comments
Open

Why isn’t chronosInternalRetFuture gensymmed? #368

SirNickolas opened this issue Mar 9, 2023 · 4 comments

Comments

@SirNickolas
Copy link

# -> iterator nameIter(chronosInternalRetFuture: Future[T]): FutureBase {.closure.} =
# -> {.push warning[resultshadowed]: off.}
# -> var result: T
# -> {.pop.}
# -> <proc_body>
# -> complete(chronosInternalRetFuture, result)
let
internalFutureSym = ident "chronosInternalRetFuture"

If you take the body of a procedure after it is processed by .async, put it into a new procedure, and apply .async to it, chronosInternalRetFuture will refer to the wrong variable. std/asyncmacro does not suffer from this.

P.S. If you are curious, I’m trying to make my asyncIters library work with Chronos.

@cheatfate
Copy link
Collaborator

Because await is template in chronos and it uses chronosInternalRetFuture.

@SirNickolas
Copy link
Author

But you can inject a separate await template for the procedure being transformed so that it will use chronosInternalRetFuture`gensym123456. Since await is already a template, this will not incur additional code bloating.

@arnetheduck
Copy link
Member

Do you have an isolated repro fot this?

@SirNickolas
Copy link
Author

With ease. Here you are:

import std/macros
when not declared assert:
  import std/assertions
when defined useChronos:
  import chronos/asyncloop
else:
  import std/asyncdispatch

macro wrapWithSubproc(body) =
  body.expectKind nnkStmtList
  body[0].expectKind nnkStmtList
  let n = body[0].len - 1
  let ret = body[0][n]
  ret.expectKind nnkReturnStmt
  body[0].del n # Detach from the body.

  result = quote:
    proc inner {.async, genSym.} = `body`
    await inner()
    `ret` # Reattach here.
  echo treeRepr result

proc outer: Future[string] {.async.} =
  wrapWithSubproc:
    return "ok"

doAssert waitFor(outer()) == "ok"
  1. async processes the body of outer and replaces return "ok" with some code. It consists of two parts: completing the future and exiting from the procedure.
  2. This code is passed to wrapWithSubproc. The macro splits it into the aforementioned parts. The return part is left where it is, and the future-completion part is injected into inner.
  3. async processes the body of inner.
  • When you compile with asyncdispatch, you get the following output:

    StmtList
      ProcDef
        Ident "inner`gensym6"
        Empty
        Empty
        FormalParams
          Empty
        Pragma
          Ident "async"
          Ident "genSym"
        Empty
        StmtList
          StmtList
            StmtList
              StmtList
              Call
                Ident "complete"
                Sym "retFuture"
                StrLit "ok"
      Command
        OpenSymChoice 2 "await"
        Call
          Ident "inner`gensym6"
      ReturnStmt
        NilLit
    

    Notice the Sym "retFuture". It refers to a Future[string] variable declared in outer. When run, the code completes successfully.

  • When you compile with chronos, the output is instead:

    StmtList
      ProcDef
        Ident "inner`gensym1"
        Empty
        Empty
        FormalParams
          Empty
        Pragma
          Ident "async"
          Ident "genSym"
        Empty
        StmtList
          StmtList
            StmtList
              Call
                Ident "complete"
                Cast
                  BracketExpr
                    Ident "Future"
                    Ident "string"
                  Ident "chronosInternalRetFuture"
                StrLit "ok"
      Command
        Sym "await"
        Call
          Ident "inner`gensym1"
      ReturnStmt
        NilLit
    

    Here, we see Ident "chronosInternalRetFuture". But at step 3, async will rewrite inner and inject another chronosInternalRetFuture into it, which will shadow the outer’s one. Indeed, when you run it, you get a FutureDefect: An attempt was made to complete a Future more than once.

N.B. Of course, in my library, I do not assume that return-related code has any particular shape. I traverse it and search for my own annotations I put there before.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants