Skip to content

Defaults #25

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

Closed
wants to merge 28 commits into from
Closed

Defaults #25

wants to merge 28 commits into from

Conversation

adriaanm
Copy link
Owner

No description provided.

retronym and others added 28 commits August 24, 2015 14:03
A DefDef owned by a trait may now have have a method body.
Such a method must be emitted without ACC_ABSTRACT, and
with the code attribute.

Tested by intercepting the compile pipeline and adding the
DEFAULTMETHOD flag and a method body before running
the backend.
And do the same for the specialized variants.

Tested by a Java source file that uses lambda syntax to create
instances of generic and specialized `Function{0,1}`.

Here's how the interfaces look now:

```
% javap -c -classpath /tmp/function 'scala.Function1'
Compiled from "Function1.scala"
public interface scala.Function1<T1, R> {
  public abstract R apply(T1);

  public <A> scala.Function1<A, R> compose(scala.Function1<A, T1>);
    Code:
       0: aload_0
       1: aload_1
       2: invokestatic  #18                 // Method scala/Function1$class.compose:(Lscala/Function1;Lscala/Function1;)Lscala/Function1;
       5: areturn

  public <A> scala.Function1<T1, A> andThen(scala.Function1<R, A>);
    Code:
       0: aload_0
       1: aload_1
       2: invokestatic  #24                 // Method scala/Function1$class.andThen:(Lscala/Function1;Lscala/Function1;)Lscala/Function1;
       5: areturn

  public abstract java.lang.String toString();

  public int apply$mcII$sp(int);
    Code:
       0: aload_0
       1: iload_1
       2: invokestatic  scala#110                // Method scala/Function1$class.apply$mcII$sp:(Lscala/Function1;I)I
       5: ireturn

  public long apply$mcJI$sp(int);
    Code:
       0: aload_0
       1: iload_1
       2: invokestatic  scala#115                // Method scala/Function1$class.apply$mcJI$sp:(Lscala/Function1;I)J
       5: lreturn
    ...
}

% javap -c -classpath /tmp/function 'scala.Function1$mcII$sp'
Compiled from "Function1.scala"
public interface scala.Function1$mcII$sp extends scala.Function1<java.lang.Object, java.lang.Object> {
  public java.lang.Object apply(java.lang.Object);
    Code:
       0: aload_0
       1: aload_1
       2: invokestatic  #16                 // Method scala/runtime/BoxesRunTime.unboxToInt:(Ljava/lang/Object;)I
       5: invokeinterface #19,  2           // InterfaceMethod apply:(I)I
      10: invokestatic  #23                 // Method scala/runtime/BoxesRunTime.boxToInteger:(I)Ljava/lang/Integer;
      13: areturn

  public abstract int apply$mcII$sp(int);

  public int apply(int);
    Code:
       0: aload_0
       1: iload_1
       2: invokeinterface scala#30,  2           // InterfaceMethod apply$mcII$sp:(I)I
       7: ireturn
}
```
Currently, these forward to the impl method in the impl class.
Later (perhaps after bootstrapping dependencies like partest)
we ought do away with the impl classes.
what could possibly go wrong

/*
 * Decompiled with CFR 0_101.
 *
 * Could not load the following classes:
 *  scala.reflect.ScalaSignature
 */
import scala.reflect.ScalaSignature;

@ScalaSignature(bytes="\u0006\u0001\t2q!\u0001\u0002\u0011\u0002\u0007\u0005QAA\u0001U\u0015\u0005\u0019\u0011a\u0002\u001ff[B$\u0018PP\u0002\u0001'\t\u0001a\u0001\u0005\u0002\b\u00155\t\u0001BC\u0001\n\u0003\u0015\u00198-\u00197b\u0013\tY\u0001B\u0001\u0004B]f\u0014VM\u001a\u0005\u0006\u001b\u0001!\tAD\u0001\u0007I%t\u0017\u000e\u001e\u0013\u0015\u0003=\u0001\"a\u0002\t\n\u0005EA!\u0001B+oSRDQa\u0005\u0001\u0005\u0002Q\t\u0011\u0001_\u000b\u0002+A\u0011qAF\u0005\u0003/!\u00111!\u00138u\u0011\u0015I\u0002A\"\u0001\u001b\u0003\u0005IX#A\u000e\u0011\u0005qybBA\u0004\u001e\u0013\tq\u0002\"\u0001\u0004Qe\u0016$WMZ\u0005\u0003A\u0005\u0012aa\u0015;sS:<'B\u0001\u0010\t\u0001")
public interface T {
    default public int x() {
        return 3;
    }

    public String y();

    default public void $init$() {
    }
}
➜  scala git:(defaults) ✗ cat /Users/adriaan/Desktop/trait_accessors.scala
trait T {
  def x: Int = 3
  def y: String
}

object O extends T {
  val y = "2"
  def main(args: Array[String]): Unit = {
    println(x + y)
  }
}%
this restores our ability (which regressed in 65e5278)
to compile e.g. `"".contains("")`; the back end was dying with
"Cannot emit primitive conversion from Ljava/lang/String; to
Ljava/lang/CharSequence;"
when AddInterfaces was removed we regressed on being able to compile
  class C; trait T extends C
why? T will become an interface, and by JVM rules, an interface cannot
extend a class, so we need to replace `extends C` with `extends
Object`. this commit restores the part of AddInterfaces that did that.
TODO: lazy vals in trait methods?

investigate isImplementedStatically
this messes with finding the setter for a getter in a trait,
as somehow the getter is made not-private, but not the setter...

we'll try another approach: link setters & getters symbolically
using the referenced field -- what could possibly go wrong?
no more funny business with impl classes,
rewrite is what was done before in explicitouter,
to make accessors accessible in class that will have the fields
Always emit accessors for traits,
since the fields will disappear in Mixins.
We need them around until Constructors,
so that their RHS can be moved to the $init$ method.

Mixin:
 - changes traits in the following way:
   - drops fields
   - replaces access to fields by usage of trait-field-setter
   - adds abstract field setter
 - mixes the following members into class that extends trait:
    - field
    - accessors (for fields, super, outer, trait-field-setter, ???)
adriaanm pushed 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      scala#34                 // Field $outer:Lp1/DelambdafyInline;
       4: invokevirtual scala#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      scala#34                 // Field $outer:Lp1/DelambdafyInline;
      11: aload_0
      12: invokespecial scala#42                 // Method java/lang/Object."<init>":()V
      15: aload_0
      16: invokespecial scala#45                 // Method p1/T.$init$:()V
      19: return
}

scala> :quit
```
adriaanm pushed 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      scala#34                 // Field $outer:Lp1/DelambdafyInline;
       4: invokevirtual scala#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      scala#34                 // Field $outer:Lp1/DelambdafyInline;
      11: aload_0
      12: invokespecial scala#42                 // Method java/lang/Object."<init>":()V
      15: aload_0
      16: invokespecial scala#45                 // Method p1/T.$init$:()V
      19: return
}

scala> :quit
```

Adriaan is to `git blame` for `reflection-mem-typecheck.scala`.
@adriaanm adriaanm closed this Apr 27, 2016
@adriaanm adriaanm deleted the defaults branch April 27, 2016 20:21
adriaanm pushed a commit that referenced this pull request May 29, 2018
Unlike normal diff, the git diff feature wasn't using the filtered
check file under `--show-diff`.

Now the sample displays just the bad line.

```
!! 1 - neg/t7494-no-options                      [output differs]
% diff /home/apm/projects/snytt/test/files/neg/t7494-no-options-neg.log /home/apm/projects/snytt/test/files/neg/t7494-no-options.check
@@ -1,7 +1,7 @@
 error: Error: ploogin takes no options
     phase name  id  description
     ----------  --  -----------
-        parser   1  parse source into ASTs, perform simple desugaring
+        parser   0  parse source into ASTs, perform simple desugaring
          namer   2  resolve names, attach symbols to named trees
 packageobjects   3  load package objects
          typer   4  the meat and potatoes: type the trees
```
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.

3 participants