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

Fix classpath within of SBT build #22

Closed
wants to merge 3 commits into from

Conversation

retronym
Copy link

No description provided.

In favour of explicitly ignoring the ones we know contain
SBT build output.

Rationale: we used to have a package named target. Admittedly
we thought better of that (55109d0) so it is now called `meta`,
but let's not get lazy and encode a poor practice into our
gitignore.
I added build configuration to ask SBT to forcible override the Scala
version in the subproject to the declared bootstrap version. I did
this in response to a warning by SBT the declared `scalaVersion`
has been evicted.

However, we actually *want* the newer version. To get rid of the
warning, we need to exclude all `"org.scala-lang" % "*"` when
depending on Scala modules. This cleanly breaks the cycle.

Here's a diff of `*/dependencyClasspath` as a result of this patch:

    https://gist.github.com/retronym/217c76001b1b81798042
cunei pushed a commit to cunei/scala that referenced this pull request May 19, 2015
In Scala 2.8.2, an optimization was added to create a static
cache for Symbol literals (ie, the results of `Symbol.apply("foo"))`.
This saves the map lookup on the second pass through code.

This actually was broken somewhere during the Scala 2.10 series,
after the addition of an overloaded `apply` method to `Symbol`.

The cache synthesis code was made aware of the overload and brought
back to working condition recently, in scala#3149.

However, this has uncovered a latent bug when the Symbol literals are
defined with traits.

One of the enclosed tests failed with:

	  jvm > t8933b-run.log
	java.lang.IllegalAccessError: tried to access field MotherClass.symbol$1 from class MixinWithSymbol$class
	        at MixinWithSymbol$class.symbolFromTrait(A.scala:3)
	        at MotherClass.symbolFromTrait(Test.scala:1)

This commit simply disables the optimization if we are in a trait.
Alternative fixes might be: a) make the static Symbol cache field
public / b) "mixin" the static symbol cache. Neither of these
seem worth the effort and risk for an already fairly situational
optimization.

Here's how the optimization looks in a class:

	% cat sandbox/test.scala; qscalac sandbox/test.scala && echo ":javap C" | qscala;
	class C {
	  'a; 'b
	}
	Welcome to Scala version 2.11.5-20141106-145558-aa558dce6d (Java HotSpot(TM) 64-Bit Server VM, Java 1.8.0_20).
	Type in expressions to have them evaluated.
	Type :help for more information.

	scala> :javap C
	  Size 722 bytes
	  MD5 checksum 6bb00189166917254e8d40499ee7c887
	  Compiled from "test.scala"
	public class C

	{
	  public static {};
	    descriptor: ()V
	    flags: ACC_PUBLIC, ACC_STATIC
	    Code:
	      stack=2, locals=0, args_size=0
	         0: getstatic     adriaanm#16                 // Field scala/Symbol$.MODULE$:Lscala/Symbol$;
	         3: ldc           adriaanm#18                 // String a
	         5: invokevirtual adriaanm#22                 // Method scala/Symbol$.apply:(Ljava/lang/String;)Lscala/Symbol;
	         8: putstatic     adriaanm#26                 // Field symbol$1:Lscala/Symbol;
	        11: getstatic     adriaanm#16                 // Field scala/Symbol$.MODULE$:Lscala/Symbol$;
	        14: ldc           scala#28                 // String b
	        16: invokevirtual adriaanm#22                 // Method scala/Symbol$.apply:(Ljava/lang/String;)Lscala/Symbol;
	        19: putstatic     scala#31                 // Field symbol$2:Lscala/Symbol;
	        22: return

	  public C();
	    descriptor: ()V
	    flags: ACC_PUBLIC
	    Code:
	      stack=1, locals=1, args_size=1
	         0: aload_0
	         1: invokespecial scala#34                 // Method java/lang/Object."<init>":()V
	         4: getstatic     adriaanm#26                 // Field symbol$1:Lscala/Symbol;
	         7: pop
	         8: getstatic     scala#31                 // Field symbol$2:Lscala/Symbol;
	        11: pop
	        12: return
	}

fixup
@adriaanm adriaanm closed this Aug 5, 2015
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`.
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.

2 participants