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

Cannot call functions defined inside coroutines #20

Open
smithjessk opened this issue Jun 1, 2016 · 16 comments
Open

Cannot call functions defined inside coroutines #20

smithjessk opened this issue Jun 1, 2016 · 16 comments

Comments

@smithjessk
Copy link
Collaborator

smithjessk commented Jun 1, 2016

Demo tests are available here.

A function that is defined inside a coroutine cannot be called inside that same coroutine. The code generator is not able to find the function.
Example:

> console
[info] Starting scala interpreter...
[info] 
Welcome to Scala version 2.11.4 (OpenJDK 64-Bit Server VM, Java 1.8.0_72).
Type in expressions to have them evaluated.
Type :help for more information.

scala> import org.coroutines._
import org.coroutines._

scala> :paste
// Entering paste mode (ctrl-D to finish)

val c1 = coroutine { () =>
  def foo(): String = "bar"
  val bar = "bar"
  1
}

// Exiting paste mode, now interpreting.

c1: org.coroutines.Coroutine._0[Nothing,Int]{def $pop($c: org.coroutines.Coroutine.Frame[Nothing,Int]): Unit} = Coroutine._0@2116030195

scala> :paste
// Entering paste mode (ctrl-D to finish)

val c2 = coroutine { () =>
  def baz(): String = "bah"
  val bah = baz()
  1
}

// Exiting paste mode, now interpreting.

<console>:10: error: exception during macro expansion:
scala.reflect.macros.TypecheckException: not found: value baz
    at scala.reflect.macros.contexts.Typers$$anonfun$typecheck$2$$anonfun$apply$1.apply(Typers.scala:34)
    at scala.reflect.macros.contexts.Typers$$anonfun$typecheck$2$$anonfun$apply$1.apply(Typers.scala:28)
    at scala.reflect.macros.contexts.Typers$$anonfun$3.apply(Typers.scala:24)
    at scala.reflect.macros.contexts.Typers$$anonfun$3.apply(Typers.scala:24)
    at scala.reflect.macros.contexts.Typers$$anonfun$withContext$1$1.apply(Typers.scala:25)
    at scala.reflect.macros.contexts.Typers$$anonfun$withContext$1$1.apply(Typers.scala:25)
    at scala.reflect.macros.contexts.Typers$$anonfun$1.apply(Typers.scala:23)
    at scala.reflect.macros.contexts.Typers$$anonfun$1.apply(Typers.scala:23)
    at scala.reflect.macros.contexts.Typers$class.withContext$1(Typers.scala:25)
    at scala.reflect.macros.contexts.Typers$$anonfun$typecheck$2.apply(Typers.scala:28)
    at scala.reflect.macros.contexts.Typers$$anonfun$typecheck$2.apply(Typers.scala:28)
    at scala.reflect.internal.Trees$class.wrappingIntoTerm(Trees.scala:1691)
    at scala.reflect.internal.SymbolTable.wrappingIntoTerm(SymbolTable.scala:16)
    at scala.reflect.macros.contexts.Typers$class.withWrapping$1(Typers.scala:26)
    at scala.reflect.macros.contexts.Typers$class.typecheck(Typers.scala:28)
    at scala.reflect.macros.contexts.Context.typecheck(Context.scala:6)
    at scala.reflect.macros.contexts.Context.typecheck(Context.scala:6)
    at org.coroutines.ThreeAddressFormTransformation$class.transformToThreeAddressForm(ThreeAddressFormTransformation.scala:453)
    at org.coroutines.Synthesizer.transformToThreeAddressForm(Synthesizer.scala:15)
    at org.coroutines.Synthesizer.synthesize(Synthesizer.scala:310)
    at org.coroutines.Coroutine$.synthesize(Coroutine.scala:198)

       val c2 = coroutine { () =>
                          ^
@smithjessk
Copy link
Collaborator Author

smithjessk commented Jun 1, 2016

The second test results in the following expanded code:

(() => {
  val x$macro$1723 = foo();
  val bar = x$macro$1723;
  1
})

The function definition is not copied.

@axel22
Copy link
Member

axel22 commented Jun 1, 2016

I didn't dive into it, but from what you say, it's quite likely that this is because ASTCanonicalization drops the definition. It should be easy to fix.

@smithjessk
Copy link
Collaborator Author

smithjessk commented Jun 6, 2016

It did drop the definition. I've updated my branch to copy the definition.
The above test, and a few others that used to fail, now pass.

However, this test fails compilation. The test passes, however, if def get becomes def get(). Not sure why this is, but I'm working on it now.

@axel22
Copy link
Member

axel22 commented Jun 7, 2016

Sounds good!

@smithjessk
Copy link
Collaborator Author

smithjessk commented Jun 7, 2016

The test at the bottom of this comment doesn't compile because the compiler can't find the value get when trying to evaluate foo(get).

However, if the function is defined either outside the scope of the coroutine or with parentheses (e.g. def get(), then the test passes. In the latter case, new variables are synthesized to hold the applications of get. In the former case, however, no such variables are created so I know that the problem is not the lack of creation of these dummy variables.

Do you see any obvious problems?

// Test
test("defining a function with no parentheses inside of a coroutine") {
  def foo(a0: Int)(b0: Int) = s"a0 = $a0, b0 = $b0"
  val c = async(coroutine {() =>
    var i = 0
    def get = { i += 1; i }
    foo(get)(await(Future(get)))
  })
  assert(Await.result(c, 5 seconds) == "a0 = 1, b0 = 2")
}

// Transformed source found at the end of `canonicalizeTree`
(() => {
  var i = 0;
  def get = {
    var x$macro$60 = null.asInstanceOf[Int];
    val x$macro$61 = i.$plus(1);
    i = x$macro$61;
    x$macro$60 = i;
    x$macro$60
  };
  val x$macro$62 = AsyncAwaitTest.this.await[Int];
  val x$macro$63 = scala.concurrent.Future;
  val x$macro$64 = scala.concurrent.ExecutionContext;
  val x$macro$65 = x$macro$64.Implicits;
  val x$macro$66 = x$macro$65.global;
  val x$macro$67 = x$macro$63.apply[Int](get)(x$macro$66);
  val x$macro$68 = Predef.this.$eq$colon$eq;
  val x$macro$69 = x$macro$68.tpEquals[((scala.concurrent.Future[Int], org.coroutines.AsyncAwaitTest.Cell[Int]), Int)];
  val x$macro$70 = x$macro$62.apply[(scala.concurrent.Future[Int], org.coroutines.AsyncAwaitTest.Cell[Int]), Int](x$macro$67)(x$macro$69);
  val x$macro$71 = foo(get)(x$macro$70);
  x$macro$71
})

@axel22
Copy link
Member

axel22 commented Jun 9, 2016

This sounds like another canonicalization case missed.

On June 7, 2016 9:53:47 PM GMT+02:00, Jess Smith notifications@github.com wrote:

The test at the bottom of this comment doesn't compile because the
compiler can't find the value get when trying to evaluate foo(get).

However, if the function is defined either with parentheses or outside
the scope of the coroutine, then the test passes. In the former case,
new variables are synthesized to hold the
applications
of get. In the latter case, however, no such variables are created so
I know that the problem is not the lack of creation of these dummy
variables.

Do you see any obvious problems?

// Test
test("defining a function with no parentheses inside of a coroutine") {
 def foo(a0: Int)(b0: Int) = s"a0 = $a0, b0 = $b0"
 val c = async(coroutine {() =>
   var i = 0
   def get = { i += 1; i }
   foo(get)(await(Future(get)))
 })
 assert(Await.result(c, 5 seconds) == "a0 = 1, b0 = 2")
}

// Transformed source found at the end of `canonicalizeTree`
(() => {
 var i = 0;
 def get = {
   var x$macro$60 = null.asInstanceOf[Int];
   val x$macro$61 = i.$plus(1);
   i = x$macro$61;
   x$macro$60 = i;
   x$macro$60
 };
 val x$macro$62 = AsyncAwaitTest.this.await[Int];
 val x$macro$63 = scala.concurrent.Future;
 val x$macro$64 = scala.concurrent.ExecutionContext;
 val x$macro$65 = x$macro$64.Implicits;
 val x$macro$66 = x$macro$65.global;
 val x$macro$67 = x$macro$63.apply[Int](get)(x$macro$66);
 val x$macro$68 = Predef.this.$eq$colon$eq;
val x$macro$69 = x$macro$68.tpEquals[((scala.concurrent.Future[Int],
org.coroutines.AsyncAwaitTest.Cell[Int]), Int)];
val x$macro$70 = x$macro$62.apply[(scala.concurrent.Future[Int],
org.coroutines.AsyncAwaitTest.Cell[Int]), Int](x$macro$67)(x$macro$69);
 val x$macro$71 = foo(get)(x$macro$70);
 x$macro$71
})

You are receiving this because you commented.
Reply to this email directly or view it on GitHub:
#20 (comment)

Sent from my Android device with K-9 Mail. Please excuse my brevity.

@smithjessk
Copy link
Collaborator Author

smithjessk commented Jun 9, 2016

The test passes if the calls to get are assigned to variables. For instance, this code compiles and runs as expected:

test("await in non-primary param section 1") {
  def foo(a0: Int)(b0: Int) = s"a0 = $a0, b0 = $b0"
  val c = async(coroutine {() =>
    var i = 0
    def get = { i += 1; i }
    val call1 = get
    val call2 = get
    foo(call1)(await(Future(call2)))
  })
  assert(Await.result(c, 5 seconds) == "a0 = 1, b0 = 2")
}

One way that the canonicalization fails is by not assigning the result of get to a val as is done in, e.g. the line

val x$macro$859 = get();

This happens because the function calls are to get and not to get().

I couldn't think of an easily-implementable case that would catch function calls that do not have parentheses after them. The current case for function application requires that paramss.length >= 0. This rule catches functions with an empty argument list, but not functions with no argument list.

However, I'm still not sure why the compiler complains about not being able to find get. In the previous comment's code, get is clearly defined in scope. Thus, as I understand Scala, lines like

val x$macro$67 = x$macro$63.apply[Int](get)(x$macro$66);

should not fail compilation because of the missing symbol get.

I tested this in the REPL and confirmed my suspicions. Compilation seems to only fail when it is run through Coroutines.

> test:console
[info] Starting scala interpreter...
[info] 
Welcome to Scala version 2.11.4 (OpenJDK 64-Bit Server VM, Java 1.8.0_72).
Type in expressions to have them evaluated.
Type :help for more information.

scala> :paste
// Entering paste mode (ctrl-D to finish)

import org.coroutines._
import scala.annotation.unchecked.uncheckedVariance
import scala.concurrent._
import scala.concurrent.ExecutionContext.Implicits.global

def foo(a0: Int)(b0: Int) = s"a0 = $a0, b0 = $b0"

def anonFunc() = {
  var i = 0;
  def get = {
    var x$macro$857 = null.asInstanceOf[Int];
    val x$macro$858 = i.$plus(1);
    i = x$macro$858;
    x$macro$857 = i;
    x$macro$857
  };
  val x$macro$859 = AsyncAwaitTest.await[Int];
  val x$macro$860 = scala.concurrent.Future;
  val x$macro$861 = scala.concurrent.ExecutionContext;
  val x$macro$862 = x$macro$861.Implicits;
  val x$macro$863 = x$macro$862.global;
  val x$macro$864 = x$macro$860.apply[Int](get)(x$macro$863);
  val x$macro$865 = Predef.$eq$colon$eq;
  val x$macro$866 = x$macro$865.tpEquals[((scala.concurrent.Future[Int], org.coroutines.AsyncAwaitTest.Cell[Int]), Int)];
  val x$macro$867 = x$macro$859.apply[(scala.concurrent.Future[Int], org.coroutines.AsyncAwaitTest.Cell[Int]), Int](x$macro$864)(x$macro$866);
  val x$macro$868 = foo(get)(x$macro$867);
  x$macro$868
}


// Exiting paste mode, now interpreting.

import org.coroutines._
import scala.annotation.unchecked.uncheckedVariance
import scala.concurrent._
import scala.concurrent.ExecutionContext.Implicits.global
foo: (a0: Int)(b0: Int)String
anonFunc: ()String

scala> 

@axel22
Copy link
Member

axel22 commented Jun 10, 2016

If I remember correctly, there is a special encoding for the no args methods (called nullary methods), and you probably need to use the Tree api, instead of quasiquotes. I don't exactly remember what the encoding for nullary methods was - can you google for it?

About get, the reason for what you see is probably that get already has a Symbol attached, which refers to the get definition from the original tree, but which disappeared in the output tree. So the compiler does not even consider the second get definition.

On June 9, 2016 5:34:57 PM GMT+02:00, Jess Smith notifications@github.com wrote:

One way that the canonicalization fails is by not assigning the result
of get to a val as is done in, e.g. the line

val x$macro$859 = get();

This happens because the function calls are to get and not to
get().

I couldn't think of an easily-implementable case that would catch
function calls that do not have parentheses after them. The current
case

for function application requires that paramss.length >= 0. This rule
catches functions with an empty argument list, but not functions with
no argument list.

However, I'm still not sure why the compiler complains about not
being able to find get. In the previous comment's code, get is
clearly defined in scope. Thus, as I understand Scala, lines like

val x$macro$67 = x$macro$63.apply[Int](get)(x$macro$66);

should not fail compilation because of the missing symbol get.

I tested this in the REPL and confirmed my suspicions. Compilation
seems to only fail when it is run through Coroutines.

> test:console
[info] Starting scala interpreter...
[info] 
Welcome to Scala version 2.11.4 (OpenJDK 64-Bit Server VM, Java
1.8.0_72).
Type in expressions to have them evaluated.
Type :help for more information.

scala> :paste
// Entering paste mode (ctrl-D to finish)

import org.coroutines._
import scala.annotation.unchecked.uncheckedVariance
import scala.concurrent._
import scala.concurrent.ExecutionContext.Implicits.global

def foo(a0: Int)(b0: Int) = s"a0 = $a0, b0 = $b0"

def anonFunc() = {
 var i = 0;
 def get = {
   var x$macro$857 = null.asInstanceOf[Int];
   val x$macro$858 = i.$plus(1);
   i = x$macro$858;
   x$macro$857 = i;
   x$macro$857
 };
 val x$macro$859 = AsyncAwaitTest.await[Int];
 val x$macro$860 = scala.concurrent.Future;
 val x$macro$861 = scala.concurrent.ExecutionContext;
 val x$macro$862 = x$macro$861.Implicits;
 val x$macro$863 = x$macro$862.global;
 val x$macro$864 = x$macro$860.apply[Int](get)(x$macro$863);
 val x$macro$865 = Predef.$eq$colon$eq;
val x$macro$866 = x$macro$865.tpEquals[((scala.concurrent.Future[Int],
org.coroutines.AsyncAwaitTest.Cell[Int]), Int)];
val x$macro$867 = x$macro$859.apply[(scala.concurrent.Future[Int],
org.coroutines.AsyncAwaitTest.Cell[Int]),
Int](x$macro$864)(x$macro$866);
 val x$macro$868 = foo(get)(x$macro$867);
 x$macro$868
}


// Exiting paste mode, now interpreting.

import org.coroutines._
import scala.annotation.unchecked.uncheckedVariance
import scala.concurrent._
import scala.concurrent.ExecutionContext.Implicits.global
foo: (a0: Int)(b0: Int)String
anonFunc: ()String

scala> 

You are receiving this because you commented.
Reply to this email directly or view it on GitHub:
#20 (comment)

Sent from my Android device with K-9 Mail. Please excuse my brevity.

@axel22
Copy link
Member

axel22 commented Jun 10, 2016

@smithjessk
Copy link
Collaborator Author

How would I be able to tell if the Symbol disappeared? I can only see the pre- and post-canonicalization ASTs, and both contain a definition for get.

@axel22
Copy link
Member

axel22 commented Jun 23, 2016

You can print the symbols of different trees to compare them.

On June 22, 2016 9:54:40 PM GMT+02:00, Jess Smith notifications@github.com wrote:

How would I be able to tell if the Symbol disappeared? I can only see
the pre- and post-canonicalization ASTs, and both contain a definition
for get.


You are receiving this because you commented.
Reply to this email directly or view it on GitHub:
#20 (comment)

Sent from my Android device with K-9 Mail. Please excuse my brevity.

@smithjessk
Copy link
Collaborator Author

Maybe I'm misunderstanding your suggestion.

Inside canonicalizeTree, println(rawlambda.symbol) and println(c.typecheck(untypedtaflambda).symbol) both display value $anonfun.
println(untypedtaflambda) prints <none>.

@axel22
Copy link
Member

axel22 commented Jun 26, 2016

The last println shows <none> because that tree is not yet typechecked (it's not assigned any types or symbols).

Try printing the symbol's position to compare:

http://www.scala-lang.org/api/2.12.0-M4/scala-reflect/scala/reflect/api/Symbols$Symbol.html#pos:Symbols.this.Position

Alternatively, you could print its full name to see which get method it is referring to:

http://www.scala-lang.org/api/2.12.0-M4/scala-reflect/scala/reflect/api/Symbols$Symbol.html#fullName:String

Finally, you could cast the Symbol object to scala.reflect.internal.Symbol and print its unique identifier to compare two symbols:

https://github.com/scala/scala/blob/2.12.x/src/reflect/scala/reflect/internal/Symbols.scala#L219

The symbols that you want to print are those corresponding to the get tree, so you probably want to add a println to the default case:

or otherwise one of the cases that handle this get.

Note that in this test case, you want to ensure that canonicalization stops when entering the scope of def get -- we don't allow yields inside nested methods, so there is no need to canonicalize them. You need to apply the NestedContextValidator as is done in the other get cases. You should double-check that the existing method case is really catching this def get.

Also, you have to understand that AST canonicalization is essentially a recursive method that takes a tree (a complex expression) as an input, and returns two values:

  1. a list of local variable (or method) declarations that evaluate subparts of the original tree
  2. a simple expression that is equivalent to the original tree, but may reference variables in the list from 1.

For example, if you have:

(1 * 2) * 3

you should get back the following list of declarations:

val x$fresh$0 = 1 * 2
val x$fresh$1 = x$fresh$0 * 3

and a simple expression that is equivalent to the original tree when the above declarations are in scope:

x$fresh$1

That said, it could be the case that the current def case is not returning the right thing.

@smithjessk
Copy link
Collaborator Author

smithjessk commented Jul 5, 2016

I printed the symbol for the first instance of get inside foo(get)(await(Future(get))). get's symbol is <none> both when it is defined using def get and when it is defined using def get(). In addition, symbol.pos is always NoPosition.

The existing method case is definitely catching the def get.

In db52258, I stopped canonicalization of nested method definitions. Does this case look good to you?

@axel22
Copy link
Member

axel22 commented Jul 9, 2016

That is because you're printing info of the duplicated, untyped tree. You
should get the original tree first with the ByTreeTyper.

About the nested method canonicalization you disabled - that looks right.

On Tue, Jul 5, 2016 at 9:41 PM, Jess Smith notifications@github.com wrote:

I printed the symbol for the first instance of get inside
foo(get)(await(Future(get))). get's symbol is both when it is
defined using def get and when it is defined using def get(). In
addition, symbol.pos is always NoPosition.

The existing method case is definitely catching the def get.

In db52258
db52258,
I stopped canonicalization of nested method definitions. Does this


You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#20 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/AAJQ3tGZRiEb_RAwQCa4F0mWIrfIZRqXks5qSrN5gaJpZM4Ir9K2
.

@smithjessk
Copy link
Collaborator Author

I got the symbols via ByTreeTyper.treeMapping.

For both def get and def get(), the symbol was method get, the position was
async-await-tests.scala,line-779,offset=20972, and the fullName was org.coroutines.AsyncAwaitTest.get.

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

2 participants