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

SI-4043 WIP fix eta expansion. #16

Closed
wants to merge 3 commits into from
Closed

SI-4043 WIP fix eta expansion. #16

wants to merge 3 commits into from

Conversation

retronym
Copy link
Owner

No description provided.

retronym added 3 commits July 30, 2015 12:05
In the enclosed test case, which defines:

   trait PA[H1] {
     type Apply[A <: H1] = Any
   }

The type selection `PA[Int]#Apply` was being eta expanded
to `[A <: H1]Any`.

After this commit, it is expanded to `[A' <: Int]Any`. This avoids
a spurious kind conformance error in the test.
Refactors duplication between these methods. I've left the
old code with assertions that the old and new results are
equivalent, I'll remove these crutches in the subsequent commit.
Part II, remove the old implementation.
@retronym retronym changed the title SI-4303 WIP fix eta expansion. SI-4043 WIP fix eta expansion. Jul 30, 2015
@retronym
Copy link
Owner Author

Fails during boostrapping:

quick.lib:
    [mkdir] Created dir: /home/jenkins/workspace/scala-2.11.x-validate-publish-core@2/build/quick/classes/library
    [javac] Compiling 32 source files to /home/jenkins/workspace/scala-2.11.x-validate-publish-core@2/build/quick/classes/library
[quick.library] Compiling 579 files to /home/jenkins/workspace/scala-2.11.x-validate-publish-core@2/build/quick/classes/library
[quick.library] /home/jenkins/workspace/scala-2.11.x-validate-publish-core@2/src/library/scala/collection/parallel/mutable/ParArray.scala:616: error: constructor cannot be instantiated to expected type;
[quick.library]  found   : ParArray.this.ScanNode[U(in class ScanNode)]
[quick.library]  required: ParArray.this.ScanTree[U(in class ScanToArray)]
[quick.library]       case ScanNode(left, right) =>
[quick.library]            ^
[quick.library] /home/jenkins/workspace/scala-2.11.x-validate-publish-core@2/src/library/scala/collection/parallel/mutable/ParArray.scala:619: error: constructor cannot be instantiated to expected type;
[quick.library]  found   : ParArray.this.ScanLeaf[U(in class ScanLeaf)]
[quick.library]  required: ParArray.this.ScanTree[U(in class ScanToArray)]
[quick.library]       case ScanLeaf(_, _, from, len, Some(prev), _) =>
[quick.library]            ^
[quick.library] /home/jenkins/workspace/scala-2.11.x-validate-publish-core@2/src/library/scala/collection/parallel/mutable/ParArray.scala:620: error: value acc is not a member of Any
[quick.library]         scanLeaf(array, targetarr, from, len, prev.acc)
[quick.library]                                                    ^
[quick.library] /home/jenkins/workspace/scala-2.11.x-validate-publish-

@retronym
Copy link
Owner Author

The problem minimizes to:

trait Base[T] {
  case class ScanNode1[U >: T]()
}
class Sub[T] extends Base[T] {
  class X[V >: T] {
    (null: ScanNode1[V]) match {
      case ScanNode1() =>
    }
  }
}

sandbox/test.scala:7: error: constructor cannot be instantiated to expected type;
 found   : Sub.this.ScanNode1[U]
 required: Sub.this.ScanNode1[V]

retronym added a commit that referenced this pull request Mar 30, 2016
Rather than in implementation of the abstract method in the
expanded anonymous class.

This leads to more more efficient use of the constant pool,
code shapes more amenable to SAM inlining, and is compatible
with the old behaviour of `-Xexperimental` in Scala 2.11,
which ScalaJS now relies upon.

Manual test:

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

package p1; trait T { val x = 0; def apply(): Any }; class DelambdafyInline { def t: T = (() => "") }

// Exiting paste mode, now interpreting.

scala> :javap -c p1.DelambdafyInline
Compiled from "<pastie>"
public class p1.DelambdafyInline {
  public p1.T t();
    Code:
       0: new           #10                 // class p1/DelambdafyInline$$anonfun$t$1
       3: dup
       4: aload_0
       5: invokespecial #16                 // Method p1/DelambdafyInline$$anonfun$t$1."<init>":(Lp1/DelambdafyInline;)V
       8: areturn

  public final java.lang.Object p1$DelambdafyInline$$$anonfun$1();
    Code:
       0: ldc           #22                 // String
       2: areturn

  public p1.DelambdafyInline();
    Code:
       0: aload_0
       1: invokespecial #25                 // Method java/lang/Object."<init>":()V
       4: return
}

scala> :javap -c p1.DelambdafyInline$$anonfun$t$1
Compiled from "<pastie>"
public final class p1.DelambdafyInline$$anonfun$t$1 implements p1.T,scala.Serializable {
  public static final long serialVersionUID;

  public int x();
    Code:
       0: aload_0
       1: getfield      #25                 // Field x:I
       4: ireturn

  public void p1$T$_setter_$x_$eq(int);
    Code:
       0: aload_0
       1: iload_1
       2: putfield      #25                 // Field x:I
       5: return

  public final java.lang.Object apply();
    Code:
       0: aload_0
       1: getfield      #34                 // Field $outer:Lp1/DelambdafyInline;
       4: invokevirtual #37                 // Method p1/DelambdafyInline.p1$DelambdafyInline$$$anonfun$1:()Ljava/lang/Object;
       7: areturn

  public p1.DelambdafyInline$$anonfun$t$1(p1.DelambdafyInline);
    Code:
       0: aload_1
       1: ifnonnull     6
       4: aconst_null
       5: athrow
       6: aload_0
       7: aload_1
       8: putfield      #34                 // Field $outer:Lp1/DelambdafyInline;
      11: aload_0
      12: invokespecial #42                 // Method java/lang/Object."<init>":()V
      15: aload_0
      16: invokespecial #45                 // Method p1/T.$init$:()V
      19: return
}

scala> :quit
```
retronym added a commit that referenced this pull request Mar 30, 2016
Rather than in implementation of the abstract method in the
expanded anonymous class.

This leads to more more efficient use of the constant pool,
code shapes more amenable to SAM inlining, and is compatible
with the old behaviour of `-Xexperimental` in Scala 2.11,
which ScalaJS now relies upon.

Manual test:

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

package p1; trait T { val x = 0; def apply(): Any }; class DelambdafyInline { def t: T = (() => "") }

// Exiting paste mode, now interpreting.

scala> :javap -c p1.DelambdafyInline
Compiled from "<pastie>"
public class p1.DelambdafyInline {
  public p1.T t();
    Code:
       0: new           #10                 // class p1/DelambdafyInline$$anonfun$t$1
       3: dup
       4: aload_0
       5: invokespecial #16                 // Method p1/DelambdafyInline$$anonfun$t$1."<init>":(Lp1/DelambdafyInline;)V
       8: areturn

  public final java.lang.Object p1$DelambdafyInline$$$anonfun$1();
    Code:
       0: ldc           #22                 // String
       2: areturn

  public p1.DelambdafyInline();
    Code:
       0: aload_0
       1: invokespecial #25                 // Method java/lang/Object."<init>":()V
       4: return
}

scala> :javap -c p1.DelambdafyInline$$anonfun$t$1
Compiled from "<pastie>"
public final class p1.DelambdafyInline$$anonfun$t$1 implements p1.T,scala.Serializable {
  public static final long serialVersionUID;

  public int x();
    Code:
       0: aload_0
       1: getfield      #25                 // Field x:I
       4: ireturn

  public void p1$T$_setter_$x_$eq(int);
    Code:
       0: aload_0
       1: iload_1
       2: putfield      #25                 // Field x:I
       5: return

  public final java.lang.Object apply();
    Code:
       0: aload_0
       1: getfield      #34                 // Field $outer:Lp1/DelambdafyInline;
       4: invokevirtual #37                 // Method p1/DelambdafyInline.p1$DelambdafyInline$$$anonfun$1:()Ljava/lang/Object;
       7: areturn

  public p1.DelambdafyInline$$anonfun$t$1(p1.DelambdafyInline);
    Code:
       0: aload_1
       1: ifnonnull     6
       4: aconst_null
       5: athrow
       6: aload_0
       7: aload_1
       8: putfield      #34                 // Field $outer:Lp1/DelambdafyInline;
      11: aload_0
      12: invokespecial #42                 // Method java/lang/Object."<init>":()V
      15: aload_0
      16: invokespecial #45                 // Method p1/T.$init$:()V
      19: return
}

scala> :quit
```
retronym added a commit that referenced this pull request Apr 14, 2016
Rather than in implementation of the abstract method in the
expanded anonymous class.

This leads to more more efficient use of the constant pool,
code shapes more amenable to SAM inlining, and is compatible
with the old behaviour of `-Xexperimental` in Scala 2.11,
which ScalaJS now relies upon.

Manual test:

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

package p1; trait T { val x = 0; def apply(): Any }; class DelambdafyInline { def t: T = (() => "") }

// Exiting paste mode, now interpreting.

scala> :javap -c p1.DelambdafyInline
Compiled from "<pastie>"
public class p1.DelambdafyInline {
  public p1.T t();
    Code:
       0: new           #10                 // class p1/DelambdafyInline$$anonfun$t$1
       3: dup
       4: aload_0
       5: invokespecial #16                 // Method p1/DelambdafyInline$$anonfun$t$1."<init>":(Lp1/DelambdafyInline;)V
       8: areturn

  public final java.lang.Object p1$DelambdafyInline$$$anonfun$1();
    Code:
       0: ldc           #22                 // String
       2: areturn

  public p1.DelambdafyInline();
    Code:
       0: aload_0
       1: invokespecial #25                 // Method java/lang/Object."<init>":()V
       4: return
}

scala> :javap -c p1.DelambdafyInline$$anonfun$t$1
Compiled from "<pastie>"
public final class p1.DelambdafyInline$$anonfun$t$1 implements p1.T,scala.Serializable {
  public static final long serialVersionUID;

  public int x();
    Code:
       0: aload_0
       1: getfield      #25                 // Field x:I
       4: ireturn

  public void p1$T$_setter_$x_$eq(int);
    Code:
       0: aload_0
       1: iload_1
       2: putfield      #25                 // Field x:I
       5: return

  public final java.lang.Object apply();
    Code:
       0: aload_0
       1: getfield      #34                 // Field $outer:Lp1/DelambdafyInline;
       4: invokevirtual #37                 // Method p1/DelambdafyInline.p1$DelambdafyInline$$$anonfun$1:()Ljava/lang/Object;
       7: areturn

  public p1.DelambdafyInline$$anonfun$t$1(p1.DelambdafyInline);
    Code:
       0: aload_1
       1: ifnonnull     6
       4: aconst_null
       5: athrow
       6: aload_0
       7: aload_1
       8: putfield      #34                 // Field $outer:Lp1/DelambdafyInline;
      11: aload_0
      12: invokespecial #42                 // Method java/lang/Object."<init>":()V
      15: aload_0
      16: invokespecial #45                 // Method p1/T.$init$:()V
      19: return
}

scala> :quit
```

Adriaan is to `git blame` for `reflection-mem-typecheck.scala`.
@retronym retronym closed this Jun 2, 2016
retronym added a commit that referenced this pull request Aug 19, 2016
  - Avoid boxing the {Object, Int, ...}Ref itself by storing it in
    a val, not a var
  - Avoid box/unbox of primitive lazy expressions due which are added
    to "adapt" it to the erased type of the fictional syncronized
    method, by using a `return`. We have to add a dummy throw after
    the synchronized block, but this is elimnated by the always-on
    DCE in the code generator.

```
⚡  qscalac -Xprint:mixin $(f "class C { def foo = { lazy val x = 42; x }}"); javap -private -c -cp . C
[[syntax trees at end of                     mixin]] // a.scala
package <empty> {
  class C extends Object {
    def foo(): Int = {
      lazy <artifact> val x$lzy: scala.runtime.LazyInt = new scala.runtime.LazyInt();
      C.this.x$1(x$lzy)
    };
    final private[this] def x$1(x$lzy$1: scala.runtime.LazyInt): Int = {
      x$lzy$1.synchronized({
        if (x$lzy$1.initialized().unary_!())
          {
            x$lzy$1.initialized_=(true);
            x$lzy$1.value_=(42)
          };
        return x$lzy$1.value()
      });
      throw null
    };
    def <init>(): C = {
      C.super.<init>();
      ()
    }
  }
}

Compiled from "a.scala"
public class C {
  public int foo();
    Code:
       0: new           #12                 // class scala/runtime/LazyInt
       3: dup
       4: invokespecial #16                 // Method scala/runtime/LazyInt."<init>":()V
       7: astore_1
       8: aload_1
       9: invokestatic  #20                 // Method x$1:(Lscala/runtime/LazyInt;)I
      12: ireturn

  private static final int x$1(scala.runtime.LazyInt);
    Code:
       0: aload_0
       1: dup
       2: astore_1
       3: monitorenter
       4: aload_0
       5: invokevirtual #31                 // Method scala/runtime/LazyInt.initialized:()Z
       8: ifne          22
      11: aload_0
      12: iconst_1
      13: invokevirtual #35                 // Method scala/runtime/LazyInt.initialized_$eq:(Z)V
      16: aload_0
      17: bipush        42
      19: invokevirtual #39                 // Method scala/runtime/LazyInt.value_$eq:(I)V
      22: aload_0
      23: invokevirtual #42                 // Method scala/runtime/LazyInt.value:()I
      26: istore_2
      27: goto          33
      30: aload_1
      31: monitorexit
      32: athrow
      33: aload_1
      34: monitorexit
      35: iload_2
      36: ireturn
    Exception table:
       from    to  target type
           4    30    30   Class java/lang/Throwable

  public C();
    Code:
       0: aload_0
       1: invokespecial #43                 // Method java/lang/Object."<init>":()V
       4: return
}
```
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

Successfully merging this pull request may close these issues.

1 participant