Fix nested implicit functions

[b-studios]

Currently nested implicit function types (#1775) are not successfully eta-expanded. That is the following is not valid:

def foo: implicit A => implicit B => Int = {
  42
}

While this PR drafts a solution, for now it should only serve for discussion purposes.

@smarter @odersky Is there a reason why automatic eta expansion of nested implicit function types is not implemented?

[b-studios]

Here are some examples on how the implemented expansion behaves currently (for some arbitrary types A and B:

def foo: implicit A => implicit B => Int = {
  implicitly[A]
  implicitly[B]
  42
}
//=====>
def foo: implicit A => implicit B => Int = { implicit evidence$2 => implicit evidence$1 =>
  implicitly[A](evidence$2)
  implicitly[B](evidence$1)
  42
}

def foo: implicit A => implicit B => Int = { implicit a: A =>
  implicitly[A]
  implicitly[B]
  42
}

//=====>

def foo: implicit A => implicit B => Int = { implicit a: A => implicit evidence$1 =>
  implicitly[A](a)
  implicitly[B](evidence$1)
  42
}

def foo: implicit A => implicit B => Int = { implicit a: A => implicit b: B =>
  implicitly[A]
  implicitly[B]
  42
}

//=====>

def foo: implicit A => implicit B => Int = { implicit a: A => implicit b: B =>
  implicitly[A](a)
  implicitly[B](b)
  42
}

[b-studios]

The newly added tests show some failure during typechecking which I haven't observed running dotc. I guess the postconditions are only checked if explicitly enabled which is the case during testing?

The relevant line 1572:

[error] Test dotc.tests.pos_all failed: java.lang.AssertionError: assertion failed: 
  bad owner; method $anonfun has owner method foo, expected was method foo$direct

[smarter]

@b-studios Yes, see https://github.com/lampepfl/dotty/blob/master/compiler/test/dotc/tests.scala#L30 and https://github.com/lampepfl/dotty/blob/master/compiler/test/dotc/tests.scala#L65 The stacktrace for your errors shows that it happens in TreeChecker which is run by -Ycheck

[b-studios]

@smarter Thanks, looks like the bug hides in the ShortcutImplicit phase, which I barely touched, yet. When disabling the phase the tests run through.

[b-studios]

@smarter I must say, I am a bit confused how ShortcutImplicits should proceed with nested implicit function types. Let's assume

type IF = implicit Us => implicit Vs => (implicit ...) => R

Would a method definition m

def m(xs: Ts): IF = implicit (ys: Us) => implicit (zs: Vs) =>  (implicit ...) => E

be transformed into the following two methods

def m(xs: Ts): IF = implicit (ys: Us) => implicit (zs: Vs) =>  (implicit ...) => m$direct(xs)(ys, zs, ...)
def m$direct(xs: Ts)(implicit ys: Us, zs: Vs, ...): R = E

or into the following?

def m(xs: Ts): IF = implicit (ys: Us) => implicit (zs: Vs) =>  (implicit ...) => m$direct(xs)(ys)(zs)(...)
def m$direct(xs: Ts)(implicit ys: Us)(implicit zs: Vs, ...): R = E

Probably the second makes more sense, given that if m already has implicit parameters currently ShortcutImplicits will add multiple implicit argument-sections anyway.

[Blaisorblade]

@b-studios:

Probably the second option makes more sense

I guess it's a matter of taste, but I agree the second makes more sense. Among other reasons, it keeps the same type for m$direct (with multiple argument lists rather than one). At runtime, m$direct is invoked in one go anyway.

Two small nitpicks.
Nitpick one: at least according to ShortcutImplicits's docs in comments (haven't double-checked), m$direct takes all arguments as explicit arguments, not implicit ones. And I guess you plan to keep what happens now.

Nitpick two: your example with the second should call m$direct(xs)(ys)(zs)(...) not m$direct(xs)(ys, zs, ...).

def m(xs: Ts): IF = implicit (ys: Us) => implicit (zs: Vs) =>  (implicit ...) => m$direct(xs)(ys)(zs)(...)
def m$direct(xs: Ts)(implicit ys: Us)(implicit zs: Vs, ...): R = E

[b-studios]

@Blaisorblade
RE Nitpick 1: You are right, this is how it is documented. However, printing after the shortcut-phase, for the following example

object shortcut {
  trait A
  def m(n: Int): implicit A => Int = n
}

yields

class shortcut$() extends Object() {
  <trait> interface trait A() extends Object {}
  def m(n: Int): implicit shortcut.A => Int = {
    def $anonfun(implicit evidence$1: shortcut.A): Int =
        shortcut.m$direct(n)(evidence$1)
      closure($anonfun)
    }
  def m$direct(n: Int)(implicit x$0: shortcut.A): Int = n
}

RE Nitpick 2: Thanks, I now got that fixed in place!

[Blaisorblade]

@b-studios Thanks for the answer and fixes!

RE Nitpick 1: You are right, this is how it is documented.

It'd be great if somebody fixed those docs then — wanna get one more patch under your belt? 😉

I pointed out I hadn't double-checked because I had my suspects: I looked at the code and couldn't see where implicit was dropped — the code seems to take the type for m$direct from the generated apply. I assume I don't get all details though 🙂

[odersky]

Thanks for brining up the problem and proposing a fix. Your fix went into the right direction. I was able to simplify it considerably in the end. The new version is in #2235. Review comments very welcome!