I did not really read/understand the changes, but I'm guessing this could cause the opposite "problem" in Scala.js: that classes (symbols) read from classfiles do have the synthetic parents, but that symbols being compiled do not have the synthetic parents at the time Scala.js runs.

The Scala.js IR does not have the restriction that super calls may only call directly inherited interfaces, so the Scala.js back-end would not add synthetic parents to fix the problem.

I'm not exactly sure whether what I'm saying makes any sense, but maybe it will trigger some thought.

classes (symbols) read from classfiles do have the synthetic parents

Nothing has changed for symbols and frontend types. the additional interfaces only show up in ClassBTypes, which scala.js is probably not using at all.

we have two ways of filling up the details of a ClassBType: either we get the information (parents, flags, etc) from the frontend symbol or from the classfile. to ensure we have all the necessary information in the latter case, some metadata is saved in the classfile in the InlineInfoAttribute, for example what methods have the @inline annotation. The goal is basically to get a consistent ClassBType without requiring the corresponding symbol, because it's not always straightforward to get the correct symbol for a given class internal name (name mangling, synthetic classes, etc).

Nothing has changed for symbols and frontend types. the additional interfaces only show up in ClassBTypes, which scala.js is probably not using at all.

OK then my concern is moot. Scala.js does not use ClassBTypes, indeed. Thanks for the explanation.

here's an observation while working on this PR: there's no good way to generate a valid tree for a static method defined in scala.

for comparison, the selection of a java-defined static method A.f is Select(owner, "f"), owner.symbol is the companion ModuleSymbol for A.

assume we have a static method in a scala class:

• if the owner is an ordinary ClassSymbol c, Select(c.thisType, "m") is not really valid as it's not a selection on the instance. a selection on the companion (if it exists) is also not valid because the static member is in the class, not the companion - for the backend, a member of a companion object would end up in the companion's ModuleClass.
• if the owner is a ModuleClassSymbol we can encode it as a selection on the ModuleSymbol, but that is also inconsistent: the member is static in the module class, and we don't need to load the module to access it.

in any case, the backend will identify the selection of a static method and not generate any bytecode for the Select's qualifier.

these are not blockers, just a little weird. we should keep them in mind while adding more static methods (https://issues.scala-lang.org/browse/SI-9390)

there's no good way to generate a valid tree for a static method defined in scala.

I remember noticing the same problem once before when working in Delambdafy. I think I tried Select(EmptyTree, "m"), which I thought was more honest than the version useing This, but that broke something down the line, so I backed out of the change. We could try that again, perhaps things work now that GenASM is gone.

Another possibility would be be to use a null constant for the qualifier.

if the owner is a ModuleClassSymbol we can encode it as a selection on the ModuleSymbol, but that is also inconsistent: the member is static in the module class, and we don't need to load the module to access it.

FYI, currently the Scala.js backend expects this. It does not do anything special to identify static methods. It compiles them as calls to methods on the companion object, just as the tree says.

That's what it's supposed to do, because Java statics end up being re implemented in Scala as methods on the companion object.

@sjrd Here's a case where Scala 2.11 currently emits a static method within a class. This is done as a special case in lambdalift to avoid a verify error. Might be interesting to look at this from the ScalaJS perspective.

scala> class C(a: Any); class D extends C({ def foo = 1; foo})
defined class C
defined class D

scala> :javap -private -c D
Compiled from "<console>"
public class D extends C {
  private static final int foo$1();
    Code:
       0: iconst_1
       1: ireturn

  public D();
    Code:
       0: aload_0
       1: invokestatic  #11                 // Method foo$1:()I
       4: invokestatic  #17                 // Method scala/runtime/BoxesRunTime.boxToInteger:(I)Ljava/lang/Integer;
       7: invokespecial #20                 // Method C."<init>":(Ljava/lang/Object;)V
      10: return
}

@retronym That's ... interesting ^^ I never saw that pop up. It turns out it works fine because the method is also emitted as non-static, and that is not a problem for the Scala.js IR:

> helloworld/scalajsp helloworld/D.sjsir
class Lhelloworld_D extends Lhelloworld_C {
  def foo$1__p2__I(): int = {
    1
  }
  def init___() {
    this.Lhelloworld_C::init___O(this.foo$1__p2__I())
  }
}

We may need a related change in erasure related to the casts inserted to implement refinements (is it just refinements that require this treatment?).

public class Test {
    private static class A {
        int i;
    }
    static class B extends A {
    };

class D {
  def test(b: Other with Test.B) {
    b.i = b.i + 1;
  }
}

class Other

// scalac-2.11.8 -Xprint:posterasure  sandbox/test.scala
[[syntax trees at end of               posterasure]] // test.scala
package <empty> {
  class D extends Object {
    def test(b: Other): Unit = b.$asInstanceOf[Test.A]().i = b.$asInstanceOf[Test.A]().i.+(1)
  };
}

Here, we have cast the qualifier to the owner of the field.

I expect this would be pretty hard to fix, though, and it is something of a corner case so we might just have to leave it.

Good observation..! I managed to turn this into a test:

$ cat B.java
package p1;

interface A {
  int i();
}

public interface B extends A { }

$ cat Test.scala
package p2 {
  class K
}

object Test {
  def t(x: p2.K with p1.B) = x.i()
  def main(args: Array[String]): Unit = {
    println(t(new p2.K with p1.B { def i() = 1 }))
  }
}

$ rm -rf p1 p2 *.class
$ javac B.java -d .
$ qsc Test.scala
$ qs Test
java.lang.IllegalAccessError: tried to access class p1.A from class Test$
    at Test$.t(Test.scala:6)

Echoing the earlier discussion about [I.clone vs Object.clone, because it's hidden now after a force-push:

Jason:

Seems like OpenJDK has a special case in method resolution to allow this. Probably best not to rely on it.
http://hg.openjdk.java.net/jdk8/jdk8/hotspot/file/tip/src/share/vm/interpreter/linkResolver.cpp#l439

The special case basically considers Object.clone as public if the dynamic receiver is an array.

Lukas:

Checked again the JVM spec

Access Control: protected method R, declared in C, is accessible > to a class or interface D if

• D is a subclass of C, and
• R must contain a symbolic reference to a class T, such that T is either a subclass of D, a superclass of D, or D itself

"symbolic reference to class T" means the receiver class in the invocation instruction.

But there's more: "This discussion omits a related restriction [...] is checked as part of the verification process (§4.10.1.8); it is not > part of link-time access control."

4.10.1.8. Type Checking for protected Members encodes that a > protected method C.m can be accessed in a subclass D only if the receiver object's type conforms to D. This corresponds to the Java spec > 6.6.2.1.

Assume we have

class C { override def clone(): Object = "hi" }

We cannot emit def f(c: C) = c.clone() as Object.clone(). he JVM would perform the following checks:

• Method Resolution (5.4.3.3) finds Object.clone
• Access Control (5.4.4) is checked at the end of resolution > and succeeds (every class and interface is a subclass of Object, so Object.clone can always passes)
• Verification (4.10.1.8) will fail the > passesProtectedCheck

I tested this with a compiler hack, we get java.lang.VerifyError indeed.

So the reason Object.clone works for arrays is really the special cases you point to above. Given that javac also emits [I.clone we > better stick to that.

Finally, to make the discussion complete, here's the example with traits:

trait T { override def clone(): Object = "hu" }
trait U extends T
class K extends U
object T1 { def f1(u: U) = u.clone() }

We have to emit u.clone as T.clone. Using U.clone gives NoSuchMethodError: U.clone. Looking at 5.4.3.4. Interface Method Resolution I actually don't see why this is the case, it might be a bug in the JVM.

Lukas:

To stress that point ("it might be a bug in the JVM"), I can get javac to produce bytecode that crashes with a NoSuchMethodError:

public class A {
  public interface I1 {
    default Object clone() { return "hi"; }
  }

  public interface I2 extends I1 { }

  public static class K implements I2 {
    public Object clone() { return "Ka"; }
  }

  public static void f(I2 i) { System.out.println(i.clone()); }

  public static void main(String[] args) {
    f(new K());
  }
}

$ javac A.java
$ java A
Exception in thread "main" java.lang.NoSuchMethodError: A$I2.clone()Ljava/lang/Object;
  at A.f(A.java:12)
  at A.main(A.java:15)

@retronym I updated the comments here and here and filed https://issues.scala-lang.org/browse/SI-9761, so this PR should be ready now.

LGTM