Problems with coroutines as by-name arguments

[smithjessk]

Currently, by-name arguments are treated like normal function arguments.

This can be problematic. Consider if one of the by-name arguments were ???. Then, a dummy variable would be created to hold the result of the call to ???. This would throw a NotImplementedException.

I've changed the AST canonicalization to pass by-name arguments directly to the function. This fixes the above problem.

However, this creates other problems. For instance, Future.apply takes a by-name argument and evaluates it in a different context. This will cause a RuntimeException if the argument is a coroutine.

test("nested await as bare expression") {
  val c = async(coroutine { () =>
    await(Future(await(Future("")).isEmpty))
  })
  val result = Await.result(c, 5 seconds)
  assert(result == true)
}

Results in:

- nested await as bare expression *** FAILED ***
[info]   java.lang.RuntimeException: Coroutines can only be invoked directly from within other coroutines. Use `call(<coroutine>(<arg0>, ..., <argN>))` instead if you want to start a new coroutine.
[info]   at scala.sys.package$.error(package.scala:27)

Note: async and await are defined here.

---

I see two solutions to this:

1. Allow by-name coroutines to be invoked directly from any context
2. Do not allow coroutines to be by-name arguments

I can't think of an easy and clean implementation of solution 1. On the other hand, solution 2 may be too restrictive.

What do you think the best way forward is?

[axel22]

There are two things here.

First, a coroutine application (c(x, y, z) where c is a coroutine) should never appear in a by-name parameter - it can only appear in the lexical scope of the coroutine definition, but not in a nested method, lambda declaration or a nested class/trait/object. Effectively, a by-name parameter is a lambda (equivalent to () => expr). We already disallow coroutines in lambdas, and we should use NestedContextValidator on method arguments for which the corresponding parameter is by-name. So, you should make sure that NestedContextValidator gets invoked in this case.

Second, there is still a problem with having by-name parameters evaluated too early because they get canonicalized away. The solution here should be not to canonicalize them, but to leave such expressions at the corresponding parameter positions.

[smithjessk]

I'm working on this in a branch and will open a PR soon.

[axel22]

Sounds good!