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.

Sign up for GitHub

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

Make await a template #12085

Merged
merged 5 commits into from
Apr 25, 2020
Merged

Make await a template #12085

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
48c7751
Make await a template
alehander92 Aug 28, 2019
4ec022e
Use typed for await
alehander92 Apr 21, 2020
b48bdc0
Generate await inside async/multisync
alehander92 Apr 22, 2020
da2c09b
Fix errors, more tests
alehander92 Apr 22, 2020
8b016d2
Remove global handler, type error if out of async
alehander92 Apr 22, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
162 changes: 30 additions & 132 deletions lib/pure/asyncmacro.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,17 +11,6 @@

import macros, strutils, asyncfutures

proc skipUntilStmtList(node: NimNode): NimNode {.compileTime.} =
# Skips a nest of StmtList's.
result = node
if node[0].kind == nnkStmtList:
result = skipUntilStmtList(node[0])

proc skipStmtList(node: NimNode): NimNode {.compileTime.} =
result = node
if node[0].kind == nnkStmtList:
result = node[0]

template createCb(retFutureSym, iteratorNameSym,
strName, identName, futureVarCompletions: untyped) =
bind finished
Expand Down Expand Up @@ -58,31 +47,6 @@ template createCb(retFutureSym, iteratorNameSym,
retFutUnown.fail(getCurrentException())
identName()

template useVar(result: var NimNode, futureVarNode: NimNode, valueReceiver,
rootReceiver: untyped, fromNode: NimNode) =
## Params:
## futureVarNode: The NimNode which is a symbol identifying the Future[T]
## variable to yield.
## fromNode: Used for better debug information (to give context).
## valueReceiver: The node which defines an expression that retrieves the
## future's value.
##
## rootReceiver: ??? TODO
# -> yield future<x>
result.add newNimNode(nnkYieldStmt, fromNode).add(futureVarNode)
# -> future<x>.read
valueReceiver = newDotExpr(futureVarNode, newIdentNode("read"))
result.add rootReceiver

template createVar(result: var NimNode, futSymName: string,
asyncProc: NimNode,
valueReceiver, rootReceiver: untyped,
fromNode: NimNode) =
result = newNimNode(nnkStmtList, fromNode)
var futSym = genSym(nskVar, "future")
result.add newVarStmt(futSym, asyncProc) # -> var future<x> = y
useVar(result, futSym, valueReceiver, rootReceiver, fromNode)

proc createFutureVarCompletions(futureVarIdents: seq[NimNode],
fromNode: NimNode): NimNode {.compileTime.} =
result = newNimNode(nnkStmtList, fromNode)
Expand Down Expand Up @@ -128,51 +92,6 @@ proc processBody(node, retFutureSym: NimNode,

result.add newNimNode(nnkReturnStmt, node).add(newNilLit())
return # Don't process the children of this return stmt
of nnkCommand, nnkCall:
if node[0].eqIdent("await"):
case node[1].kind
of nnkIdent, nnkInfix, nnkDotExpr, nnkCall, nnkCommand:
# await x
# await x or y
# await foo(p, x)
# await foo p, x
var futureValue: NimNode
result.createVar("future" & $node[1][0].toStrLit, node[1], futureValue,
futureValue, node)
else:
error("Invalid node kind in 'await', got: " & $node[1].kind)
elif node.len > 1 and node[1].kind == nnkCommand and
node[1][0].eqIdent("await"):
# foo await x
var newCommand = node
result.createVar("future" & $node[0].toStrLit, node[1][1], newCommand[1],
newCommand, node)

of nnkVarSection, nnkLetSection:
case node[0][^1].kind
of nnkCommand:
if node[0][^1][0].eqIdent("await"):
# var x = await y
var newVarSection = node # TODO: Should this use copyNimNode?
result.createVar("future" & node[0][0].strVal, node[0][^1][1],
newVarSection[0][^1], newVarSection, node)
else: discard
of nnkAsgn:
case node[1].kind
of nnkCommand:
if node[1][0].eqIdent("await"):
# x = await y
var newAsgn = node
result.createVar("future" & $node[0].toStrLit, node[1][1], newAsgn[1],
newAsgn, node)
else: discard
of nnkDiscardStmt:
# discard await x
if node[0].kind == nnkCommand and
node[0][0].eqIdent("await"):
var newDiscard = node
result.createVar("futureDiscard_" & $toStrLit(node[0][1]), node[0][1],
newDiscard[0], newDiscard, node)
of RoutineNodes-{nnkTemplateDef}:
# skip all the nested procedure definitions
return
Expand All @@ -182,6 +101,8 @@ proc processBody(node, retFutureSym: NimNode,
result[i] = processBody(result[i], retFutureSym, subTypeIsVoid,
futureVarIdents)

# echo result.repr

proc getName(node: NimNode): string {.compileTime.} =
case node.kind
of nnkPostfix:
Expand Down Expand Up @@ -332,8 +253,24 @@ proc asyncSingleProc(prc: NimNode): NimNode {.compileTime.} =
if returnType.kind == nnkEmpty:
# Add Future[void]
result.params[0] = parseExpr("owned(Future[void])")

# based on the yglukhov's patch to chronos: https://github.com/status-im/nim-chronos/pull/47
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would remove this comment, at this point this really doesn't have many parallels. I doubt it'll be useful for those exploring this code.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well, I still got the "define await as a template" overall approach from there, but I am not sure what's the best way to document that stuff

# however here the overloads are placed inside each expanded async
var awaitDefinition = quote:
template await(f: typed): untyped =
static:
error "await expects Future[T], got " & $typeof(f)

template await[T](f: Future[T]): auto =
var internalTmpFuture: FutureBase = f
yield internalTmpFuture
(cast[type(f)](internalTmpFuture)).read()

if procBody.kind != nnkEmpty:
result.body = outerProcBody
result.body = quote:
`awaitDefinition`
`outerProcBody`

#echo(treeRepr(result))
#if prcName == "recvLineInto":
# echo(toStrLit(result))
Expand All @@ -350,50 +287,6 @@ macro async*(prc: untyped): untyped =
when defined(nimDumpAsync):
echo repr result


# Multisync
proc emptyNoop[T](x: T): T =
# The ``await``s are replaced by a call to this for simplicity.
when T isnot void:
return x

proc stripAwait(node: NimNode): NimNode =
## Strips out all ``await`` commands from a procedure body, replaces them
## with ``emptyNoop`` for simplicity.
result = node

let emptyNoopSym = bindSym("emptyNoop")

case node.kind
of nnkCommand, nnkCall:
if node[0].eqIdent("await"):
node[0] = emptyNoopSym
elif node.len > 1 and node[1].kind == nnkCommand and node[1][0].eqIdent("await"):
# foo await x
node[1][0] = emptyNoopSym
of nnkVarSection, nnkLetSection:
case node[0][^1].kind
of nnkCommand:
if node[0][^1][0].eqIdent("await"):
# var x = await y
node[0][^1][0] = emptyNoopSym
else: discard
of nnkAsgn:
case node[1].kind
of nnkCommand:
if node[1][0].eqIdent("await"):
# x = await y
node[1][0] = emptyNoopSym
else: discard
of nnkDiscardStmt:
# discard await x
if node[0].kind == nnkCommand and node[0][0].eqIdent("await"):
node[0][0] = emptyNoopSym
else: discard

for i in 0 ..< result.len:
result[i] = stripAwait(result[i])

proc splitParamType(paramType: NimNode, async: bool): NimNode =
result = paramType
if paramType.kind == nnkInfix and paramType[0].strVal in ["|", "or"]:
Expand Down Expand Up @@ -426,8 +319,12 @@ proc splitProc(prc: NimNode): (NimNode, NimNode) =
for i in 1 ..< result[0][3].len:
# Sync proc (0) -> FormalParams (3) -> IdentDefs, the parameter (i) ->
# parameter type (1).
result[0][3][i][1] = splitParamType(result[0][3][i][1], async = false)
result[0][6] = stripAwait(result[0][6])
result[0][3][i][1] = splitParamType(result[0][3][i][1], async=false)
var multisyncAwait = quote:
template await(value: typed): untyped =
value

result[0][^1] = nnkStmtList.newTree(multisyncAwait, result[0][^1])
Comment on lines +322 to +327
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I love it :D

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

credit to @zah's idea #11911 (comment) i think i finally remembered this is also possible


result[1] = prc.copyNimTree()
if result[1][3][0].kind == nnkBracketExpr:
Expand All @@ -447,8 +344,9 @@ macro multisync*(prc: untyped): untyped =
result = newStmtList()
result.add(asyncSingleProc(asyncPrc))
result.add(sync)
# echo result.repr

proc await*[T](x: T) =
## The 'await' keyword is also defined here for technical
## reasons. (Generic symbol lookup prepass.)
{.error: "Await only available within .async".}
# overload for await as a fallback handler, based on the yglukhov's patch to chronos: https://github.com/status-im/nim-chronos/pull/47
# template await*(f: typed): untyped =
# static:
# error "await only available within {.async.}"
Comment on lines +349 to +352
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You still need this, no?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh, you removed this because you didn't want dirty. Is dirty really that bad? :)

Copy link
Contributor Author

@alehander92 alehander92 Apr 23, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no, but it can be a bit surprising: we need to somehow hint here in the error "if you use a template, you might want to use {.dirty.}" because otherwise many users would be just confused by why just a template/macro can't work

on the other hand just a type mismatch/undefined error is very natural, await is not found and that's it

9 changes: 9 additions & 0 deletions tests/async/tasync_misc.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,4 +53,13 @@ block: # nkCheckedFieldExpr

waitFor foo()

block: # 12743

template templ = await sleepAsync 0

proc prc {.async.} = templ

waitFor prc()


echo "ok"
15 changes: 15 additions & 0 deletions tests/async/tasync_noasync.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
discard """
errormsg: "undeclared identifier: 'await'"
cmd: "nim c $file"
file: "tasync_noasync.nim"
"""
import async

proc a {.async.} =
discard

await a()

# if we overload a fallback handler to get
# await only available within {.async.}
# we would need `{.dirty.}` templates for await
11 changes: 11 additions & 0 deletions tests/async/tasync_nofuture.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
discard """
errormsg: "await expects Future[T], got int"
cmd: "nim c $file"
file: "asyncmacro.nim"
"""
import async

proc a {.async.} =
await 0

waitFor a()
6 changes: 4 additions & 2 deletions tests/async/tasync_traceback.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ Async traceback:
asyncmacro\.nim\(\d+?\)\s+?a
asyncmacro\.nim\(\d+?\)\s+?aNimAsyncContinue
## Resumes an async procedure
tasync_traceback\.nim\(\d+?\)\s+?aIter
asyncmacro\.nim\(\d+?\)\s+?aIter
asyncfutures\.nim\(\d+?\)\s+?read
\]#
Exception message: b failure
Expand Down Expand Up @@ -110,13 +110,15 @@ Async traceback:
## Executes pending callbacks
asyncmacro\.nim\(\d+?\)\s+?fooNimAsyncContinue
## Resumes an async procedure
tasync_traceback\.nim\(\d+?\)\s+?fooIter
asyncmacro\.nim\(\d+?\)\s+?fooIter
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This actually reduces the amount of signal we get from the async tracebacks. I cannot recall whether this particular line provided useful information, but if it does then that could be a blocker for these changes. Can you run this test and show us what the tracebacks are before and after your changes?

Copy link
Contributor Author

@alehander92 alehander92 Apr 16, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIRC the change is similar to what is seen in the diff, basically we need to add the linedir of tasync_traceback to the result of template somehow maybe?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I want to see what line the traceback points to now.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

asyncmacro.nim(261)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

which seems to be (cast[type(f)](internalTmpFuture)).read()

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That should be okay, I'm assuming that tasync_traceback in the old traceback doesn't point to anything interesting though.

asyncfutures\.nim\(\d+?\)\s+?read
\]#
Exception message: bar failure
Exception type:
"""

# TODO: is asyncmacro good enough location for fooIter traceback/debugging? just put the callsite info for all?

let resLines = splitLines(result.strip)
let expLines = splitLines(expected.strip)

Expand Down