Mixin generates method signatures that confuse javac

[scabug]

Strictly speaking, this is not a regression, but causes other regressions in the standard library (see #6101, #5976), and may be a blocker.

Scala file:

trait Func1[R] {
  def apply(): R = null.asInstanceOf[R]
}


class Func1ReturnString extends Func1[String]

Java file:

public class Test {
    public static void main(String[] args) {
	Func1ReturnString b = new Func1ReturnString() {};
	b.apply();
    }
}

This crashes javac:

Test.java:7: error: apply() in Func1ReturnString cannot implement apply() in Func1
	Func1ReturnString b = new Func1ReturnString() {};
	                                              ^
  return type Object is not compatible with String
  where R is a type-variable:
    R extends Object declared in interface Func1
1 error

The reason is that mixin does not generate an appropriate signature for apply. Trees at the end of cleanup:

package <empty> {
  abstract trait Func1 extends Object {
    def apply(): Object
  };
  class Func1ReturnString extends Object with Func1 {
    def apply(): Object = Func1$class.apply(Func1ReturnString.this);
    def <init>(): Func1ReturnString = {
      Func1ReturnString.super.<init>();
      Func1$class./*Func1$class*/$init$(Func1ReturnString.this);
      ()
    }
  };
  abstract trait Func1$class extends  {
    def apply($this: Func1): Object = null;
    def /*Func1$class*/$init$($this: Func1): Unit = {
      ()
    }
  }
}

If you look at apply above, the return type is Object instead of String. This happens in general whenever a method signature changes due to a more refined type environment in an inheriting class and the method is not overridden/implemented in the inheriting class.

In some other situations javac compiles the file, but produces bytecode which refers to a non-existing method (signature-wise). See an example in a comment below.

This happens both with Scala 2.9.2 and Scala 2.10.x.

It does not happen with Scala 2.8.2, but we think this is because in Scala 2.8.2 the generic signatures were broken in other ways. Furthermore, it also breaks with 2.8.2 if you change the Java code a bit:

public class Test {
    public static void main(String[] args) {
	Func1ReturnString b = new Func1ReturnString() {};
	String s = b.apply();
    }
}

[scabug]

Imported From: https://issues.scala-lang.org/browse/SI-6103?orig=1
Reporter: @axel22
Affected Versions: 2.9.2, 2.10.0-M5
See #6101

[scabug]

@axel22 said:
If you have a base trait with 2 type parameters:

Scala:

trait Func1[T1, R] {
  def apply(v1: T1): R = null.asInstanceOf[R]
}


class Func1ReturnString[T] extends Func1[T, String]

Java:

public class Test {
    public static void main(String[] args) {
	Func1ReturnString<Integer> b = new Func1ReturnString<Integer>() {};
	b.apply(0);
    }
}

At runtime:

Exception in thread "main" java.lang.NoSuchMethodError: Func1ReturnString.apply(Ljava/lang/Object;)Ljava/lang/String;
	at Test.main(Test.java:7)

[scabug]

@paulp said:
I hope you've looked at #3452, which AFAICS is this.

[scabug]

@paulp said:
Especially: paulp/scala@11c11f0efc

[scabug]

@gkossakowski said:
Paul: It indeed looks related. I guess you are more versatile with #3452 to judge if it's exactly the same issue.

[scabug]

@paulp said:
Well, I'll bet a steak dinner that applying that commit at least changes the behavior in this ticket, and most likely fixes. The patch there is close, I just ran out of heart after days of it.

[scabug]

@paulp said:
For reference, that commit turns the example in comments into a compile time error, which is better than nothing I guess. The difference in the generated scala class is:

--- i/Func1ReturnString.class
+++ w/Func1ReturnString.class
@@ -1,7 +1,7 @@
 
 Compiled from "S_1.scala"
 public class Func1ReturnString<T> implements Func1<T, java.lang.String> {
-  public java.lang.String apply(T);
+  public java.lang.Object apply(java.lang.Object);
     Signature: (Ljava/lang/Object;)Ljava/lang/Object;
     Code:
 ##: aload_1       

That demonstrates the essence of the problem: concrete types cannot be directly substituted for corresponding type parameters in the signatures of mixed in methods, because the underlying concrete implementation will have the erasure of the type parameters, not of the concrete types.

In other words, "Func1" knows nothing of "String", but "Func1ReturnString" thinks it can substitute String for T and everything will work out. It won't.

It's elaborated on at some length in the commit.

[scabug]

@VladUreche said:
Scala Meeting resolution: We'll eliminate AnyRef specialization from Function* and put AnyRef specialization under a flag, since it's not yet ripe for inclusion into trunk.

[scabug]

@paulp said:
I find that resolution somewhat surprising, because this has little to do with AnyRef specialization or the specialization of Function1. It is true that specializing Function1 on AnyRef makes it easier to encounter this pre-existing bug, because Function1 is everywhere. I'm not contesting it, but I hope the bathwater bucket doesn't turn out to have nothing but baby in it.

[scabug]

@lrytz said:
I think Vlad put his comment on the wrong ticket. I'm trying to summarize:

The conclusion of the meeting discussion was that this bug popped up with scalac generating more / better java generic signatures.

What scalac does on the bytecode level is correct, and also the signatures are correct, the problem is rather in javac which is not able to work correctly with these classfiles.

[scabug]

@paulp said:
That sounds painfully optimistic to me. Even if it were true, it wouldn't make any difference: if we don't work with java, it's our problem no matter where the blame might be pinned. But I don't think it's true. Either our signatures/descriptors are wrong/internally-inconsistent or we are missing bridge methods. I was trying to avoid a new batch of bridge methods when I worked on #3452 but maybe they can't be avoided.

[scabug]

@VladUreche said:
Indeed, this ticket doesn't have to do with specialization, sorry! But it's one of the tickets that convinced us to hide the AnyRef specialization behind a flag. :)

[scabug]

@retronym said:
Removing AnyRef specialization of Function1 will be a real pity. Hope it finds its way back.

[scabug]

@VladUreche said (edited on Jul 25, 2012 8:04:16 AM UTC):
@jason: I hope so too. I'll file the patch against 2.10 and revert it in master, so we should be back to normal.

I've had a look at the bugs that concern AnyRef specialization and it's not such a long list: #6103, #6043, #4790, #5976 and #5583. If we make a big push to fix all of them by the next milestone, maybe we can re-enable the AnyRef specialization.

[scabug]

@gkossakowski said:
This bug is related to #6101, see:
https://issues.scala-lang.org/browse/SI-6101?focusedCommentId=58969&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-58969

[scabug]

@gkossakowski said:
Check the comment I mentioned above. We created this ticket because we realized that if we want to fix specialization problem with AnyRef we'd need to duplicate a work that should be done in mixin (where method with wrong signatures are introduced). This ticket just provides a minimal example that triggers a problem in mixin and it just happens that with AnyRef specialization we generate trees of the same shape and stumble upon this issue.

Now, Martin said that the issue with mixin cannot be fixed (because it runs after erasure) so we have two options:

1. Fix specialization by duplicating logic of mixin in specialization for specialized overloads (cray IMO)
2. Give up on AnyRef specialization because it runs into unfixable bug (this one) and see later if we can do something about it

We decided to go with the second option. I think it's too late in 2.10 cycle to do anything more ambitious. We can try to do better than that for 2.11 (2.10.1 is out of question due to binary compatibility concerns).

[scabug]

@retronym said:
Righto. Many of my use cases for Function1 specialization can also be solved by manually defining a SAM interface, and using a macro to convert a function literal into an instance of that.

[scabug]

@paulp said:

Now, Martin said that the issue with mixin cannot be fixed (because it runs after erasure)

"Mixin generates broken code and can't be fixed no matter what" is not a sensible premise from which to analyze this issue.

[scabug]

@VladUreche said:
Changed to Major, as the bug is not fixed, but is addressed by removing AnyRef specialization from the library and not making it available by default.

[scabug]

@retronym said:
I recently opened #8293 with the plan to add bridges to make this work.

The fix we went for for #3452 in 2.11.0, which selectively weakens the generic signatures, is not sufficient to work around this bug.