Defdef vparamss and symbol have inconsistent types

[errikos]

Me and @sjrd stumbled upon this bug while trying to fix scala-js/scala-js#3953. It seems that Defdef.vparamss and the respective Defdef's symbol have inconsistent types.

Steps for reproduction:

Clone the master branch scala-js/scala-js@cc04be5 of Scala.js locally and replace examples/helloworld/src/main/scala/helloworld/HelloWorld.scala contents with the following:

import scala.language.higherKinds

sealed class StreamT[M[_]](val step: M[Step[StreamT[M]]])

sealed abstract class Step[S]

object Main {
  implicit class AnyOps[A](actual: A) {
    def mustMatch(f: PartialFunction[A, Boolean]): Unit = f.applyOrElse(???, ???)
  }

  type Id[A] = A

  def main(args: Array[String]): Unit = {
    (??? : StreamT[Id]).step mustMatch {
      case _ => true
    }
  }
}

Then, open an SBT shell and execute:

helloworld2_12/fastOptJS

Please note that the 2_12 suffix indicates the Scala version to use, so you can also use 2.11 or 2.13.

You should get the following error:

[error] file:/Users/ergys/dev/scala-js/examples/helloworld/src/main/scala/helloworld/HelloWorld.scala(27:40:MethodDef): The signature of helloworld.Main$$anonfun$main$1.applyOrElse(helloworld.Step,scala.Function1)java.lang.Object, which is (List(AnyType, ClassType(ClassName<scala.Function1>)),AnyType), does not match its name (should be (List(ClassType(ClassName<helloworld.Step>), ClassType(ClassName<scala.Function1>)),AnyType)).
[error] There were 1 IR checking errors.
[error] (helloworld2_12 / Compile / fastOptJS) There were 1 IR checking errors.

Here is the IR that is produced for the lambda inside the main method:

class helloworld.Main$$anonfun$main$1{anonfun$main$1} extends scala.runtime.AbstractPartialFunction implements scala.Serializable {
  def applyOrElse;Lhelloworld.Step;Lscala.Function1;Ljava.lang.Object(x1: any, default: scala.Function1): any = {
    val x1$2{x1}: helloworld.Step = x1.asInstanceOf[helloworld.Step];
    true
  }
  def isDefinedAt;Lhelloworld.Step;Z(x1: helloworld.Step): boolean = {
    val x1$2{x1}: helloworld.Step = x1;
    true
  }
  def isDefinedAt;Ljava.lang.Object;Z(x: any): boolean = {
    this.isDefinedAt;Lhelloworld.Step;Z(x.asInstanceOf[helloworld.Step])
  }
  def applyOrElse;Ljava.lang.Object;Lscala.Function1;Ljava.lang.Object(x: any, default: scala.Function1): any = {
    this.applyOrElse;Lhelloworld.Step;Lscala.Function1;Ljava.lang.Object(x.asInstanceOf[helloworld.Step], default)
  }
  constructor def <init>;V() {
    this.scala.runtime.AbstractPartialFunction::<init>;V()
  }
}

It seems that the applyOrElse method mangled name has the correct types (Step, Function1), but the actual parameter types are wrong (Any, Function1). The parameter types are extracted from Defdef.vparamss.

The JVM bytecode also seems to have the correct type, which you can consult in SBT:

set scalacOptions in helloworld.v2_12 += "-Xprint:mixin"
helloworld2_12/console

and then:

:javap helloworld.Main$$anonfun$main$1

gives:

public final <A1 extends helloworld.Step<helloworld.StreamT<java.lang.Object>>, B1 extends java.lang.Object> B1 applyOrElse(A1, scala.Function1<A1, B1>);
    descriptor: (Lhelloworld/Step;Lscala/Function1;)Ljava/lang/Object;
    flags: ACC_PUBLIC, ACC_FINAL
    Code:
...

As @gzm0 pointed out in scala-js/scala-js#3953 (comment):

So it seems the problem is that we take the param symbols from different locations:

• for the param list, we take them from vparamss in the DefDef tree.
• for the name, we take them from the tpe of the method symbol

This is (for now), the only difference I can see between the ways we retrieve the types :-/

Both genParam and paramOrResultTypeRef seem to be calling convert at the end.

[dwijnand]

A self-contained (i.e. not a repo clone) reproduction would be lovely, if possible.

[sjrd]

You can see the problem independently of Scala.js, using the reproduction above in a single file Test.scala, and doing:

$ scalac -Xprint:mixin -Vprint-types src/main/scala/test/Test.scala

which shows:

[[syntax trees at end of                     mixin]] // Test.scala
package <empty>{type} {
  sealed class StreamT extends Object {
    <paramaccessor> private[this] val step: Object = _;
    <stable> <accessor> <paramaccessor> def step(): Object = StreamT.this{StreamT}.step{Object};
    def <init>(step: Object): StreamT = {
      StreamT.this{StreamT}.step{Object} = step{Object}{Unit};
      StreamT.super{Object}.<init>{()Object}(){Object};
      (){Unit}
    }{Unit}
  };
  sealed abstract class Step extends Object {
    def <init>(): Step = {
      Step.super{Object}.<init>{()Object}(){Object};
      (){Unit}
    }{Unit}
  };
  object Main extends Object {
    implicit <synthetic> def AnyOps(actual: Object): Main$AnyOps = new Main$AnyOps{Main$AnyOps}{(actual: Object)Main$AnyOps}(actual{Object}){Main$AnyOps};
    def main(args: Array[String]): Unit = Main.this{Main.type}.AnyOps{(actual: Object)Main$AnyOps}((scala.Predef.???{()Nothing}(){Nothing}: StreamT){StreamT}.step{()Object}(){Object}){Main$AnyOps}.mustMatch{(f: PartialFunction)Unit}(({
      new <$anon: Function1>{anonfun$main$1}{()anonfun$main$1}(){anonfun$main$1}
    }{anonfun$main$1}: PartialFunction){PartialFunction}){Unit};
    def <init>(): Main.type = {
      Main.super{Object}.<init>{()Object}(){Object};
      (){Unit}
    }{Unit}
  };
  implicit class Main$AnyOps extends Object {
    def mustMatch(f: PartialFunction): Unit = {
      f.applyOrElse{(x: Object, default: Function1)Object}(scala.Predef.???{()Nothing}(){Nothing}, scala.Predef.???{()Nothing}(){Nothing}){Object};
      (){Unit}
    }{Unit};
    def <init>(actual: Object): Main$AnyOps = {
      Main$AnyOps.super{Object}.<init>{()Object}(){Object};
      (){Unit}
    }{Unit}
  };
  @SerialVersionUID(value = 0) final <synthetic> class anonfun$main$1 extends scala.runtime.AbstractPartialFunction with java.io.Serializable {
    final override def applyOrElse(x1: Object, default: Function1): Object = {
      case <synthetic> val x1: Step = (((x1.$asInstanceOf{[T0]()T0}[Step]{()Step}(){Step}: Step){Step}: Step){Step}: Step){Step};
      case4(){
        matchEnd3{(x: Object)Object}(scala.Boolean.box{(x: Boolean)Boolean}(true{Boolean(true)}){Object}){Object}
      }{Object};
      matchEnd3(x: Object){
        x{Object}
      }{Object}
    }{Object};
    final def isDefinedAt(x1: Step): Boolean = {
      case <synthetic> val x1: Step = (((x1{Step}: Step){Step}: Step){Step}: Step){Step};
      case4(){
        matchEnd3{(x: Boolean)Boolean}(true{Boolean(true)}){Boolean}
      }{Boolean};
      matchEnd3(x: Boolean){
        x{Boolean}
      }{Boolean}
    }{Boolean};
    final <bridge> <artifact> def isDefinedAt(x: Object): Boolean = anonfun$main$1.this{anonfun$main$1}.isDefinedAt{(x1: Step)Boolean}(x.$asInstanceOf{[T0]()T0}[Step]{()Step}(){Step}){Boolean};
    final override <bridge> <artifact> def applyOrElse(x: Object, default: Function1): Object = anonfun$main$1.this{anonfun$main$1}.applyOrElse{(x1: Step, default: Function1)Object}(x.$asInstanceOf{[T0]()T0}[Step]{()Step}(){Step}, default{Function1}){Object};
    def <init>(): <$anon: Function1> = {
      anonfun$main$1.super{runtime.AbstractPartialFunction}.<init>{()scala.runtime.AbstractPartialFunction}(){scala.runtime.AbstractPartialFunction};
      (){Unit}
    }{Unit}
  }
}

In that print-out you can see that the first (non-bridge) definition of applyOrElse is:

    final override def applyOrElse(x1: Object, default: Function1): Object = {

but it is called from the bridge as

    final override <bridge> <artifact> def applyOrElse(x: Object, default: Function1): Object =
      anonfun$main$1.this{anonfun$main$1}.applyOrElse{(x1: Step, default: Function1)Object}(x.$asInstanceOf{[T0]()T0}[Step]{()Step}(){Step}, default{Function1}){Object};

which shows that the symbol of the non-bridge applyOrElse has type (x1: Step, default: Function1)Object even though the vparamss of the DefDef for that non-bridge applyOrElse say that x1 has type Object (instead of Step).

More fundamentally, this means that the vparamss have different symbols that the sym.info.paramss of the DefDef!

[dwijnand]

Out of interest, it seems this isn't a new regression. I checked up to 2.10.7, because I couldn't get scalac 2.9.3 to run.

[SethTisue]

comment in a (closed-source) compiler plugin of mine:

      // note that we use dd.vparamss
      // not dd.symbol.paramLists; the latter symbols aren't the same
      // symbols that we'll see in the body.  (the existence of these
      // two different sets of symbols for parameters is a common
      // scalac hacker gotcha, says Adriaan)

certainly sounds related...?

[sjrd]

Yup, definitely sounds related. Nice catch!

[dwijnand]

Looks like @lrytz knew this fact too in #3140 (comment) ?

[sjrd]

So ... does that mean that it's valid/expected that the symbols not be the same?

Regardless, surely you would consider it a bug that they don't have the same types (tpe), though, right?

[Jasper-M]

I would say the type in the DefDef is wrong... If the bridge method has exactly the same signature as the regular method why is there even a bridge method? And looks like it's effectively emitted as (Step, Function1)Object in the bytecode.

[sjrd]

It's obviously the tpe of the vparamss that are wrong, yes. The tpe of the method itself is correct, and corresponds to what's in the bytecode.

[hrhino]

I think this might be the same cause as #11817.

[lrytz]

So ... does that mean that it's valid/expected that the symbols not be the same?

I would say so. A type is just a type, it stands on its own, it can be cloned and still has the same meaning (unlike Symbols). For example

scala> :power

scala> class C[T] { def f(x: T): Int = 0 }
scala> class D extends C[String]

scala> val f = typeOf[C[_]].member(TermName("f"))
f: $r.intp.global.Symbol = method f

scala> f.tpe.asSeenFrom(typeOf[D], f.owner)
res12: $r.intp.global.Type = (x: String)Int

This is, in a way, still the type of f, but obviously cannot use the parameter symbol of f's DefDef.

A MethodType happens to use Symbols to represent its paramters, which is useful for representing (Name+Type) and is also useful for representing dependent types (scalac used De Bruijn indices before the named & default parameters refactoring).

Regardless, surely you would consider it a bug that they don't have the same types (tpe), though, right?

I didn't really manage to follow through the example here, maybe you can clarify...?

[sjrd]

Regardless, surely you would consider it a bug that they don't have the same types (tpe), though, right?

I didn't really manage to follow through the example here, maybe you can clarify...?

The problem is that, given the dd @ DefDef(_, _, _, vparamss, _, _) representing the non-bridge def applyOrElse (the first one), the following happens:

val sym = dd.symbol
println(sym.tpe.params.map(_.tpe))
// List(Step, Function1)
println(vparamss.flatten.map(_.symbol.tpe))
// List(Object, Function1)

So the tpe of the vparamss are not consistent with the tpes of the method symbol's parameters!

This can already be seen right after erasure, so erasure is clearly to blame here.

This is extremely bad, because the full name of a method (its descriptor) is derived from the method symbol's parameters (it has to, since the descriptor has to be computed at call site as well), but the type of the formal parameters are derived from vparamss (they have to, because the body of the method will refer to those symbols, so their types).

On the JVM, I think what allows it to survive is that:

• The types of formal parameters are never explicitly stored in the bytecode. The JVM infers them from the method descriptor, which is Step.
• In the body, references to formal parameters don't store the type either (I guess?). They receive their type from the JVM, so Step.
• They are used by the JVM backend as if they were Object, but that's OK because they are only read, and Step <: Object.

So, it's chance, basically.

In the Scala.js IR, the type of formal parameters and of references to local vars are all stored, and checked for consistency. This triggers the error in Scala.js.

[lrytz]

I see now, thanks. That looks like a bug, here's a minimization (I couldn't find one without involving PartialFunction):

object Main {
  type Id[A] = A
  def a: PartialFunction[Id[String], Boolean] = { case _ => true }
  def b: PartialFunction[String, Boolean] = { case _ => true }
}

With scalac Test.scala -Vprint:mixin -Vprint-types

object Main extends Object {
  def a(): PartialFunction = ({
    new <$anon: Function1>{anonfun$a$1}{()anonfun$a$1}(){anonfun$a$1}
  }{anonfun$a$1}: PartialFunction){PartialFunction};

  def b(): PartialFunction = ({
    new <$anon: Function1>{anonfun$b$1}{()anonfun$b$1}(){anonfun$b$1}
  }{anonfun$b$1}: PartialFunction){PartialFunction};
}

final <synthetic> class anonfun$a$1 {
  final override def applyOrElse(x1: Object, default: Function1): Object = ...
  final <bridge> <artifact> def isDefinedAt(x: Object): Boolean = ...
}

final <synthetic> class anonfun$b$1 {
  final override def applyOrElse(x1: String, default: Function1): Object = ...
  final <bridge> <artifact> def isDefinedAt(x: Object): Boolean = ...
}

[SethTisue]

@sjrd are you able to work around this? is this a "I thought you might want to know" type of ticket, or a "I really need this fixed" type ticket?

[sjrd]

@errikos is working on a workaround, but it is very very ugly.

Usually we don't even have a choice. We have to work around everything scalac throws at us, because we need to support old versions of Scala.

However, here it is triggered a) rarely (PartialFunction with a higher-kinded type alias) and b) for non-JS-specific code. That means that we could declare that this kind of code will only work with a recent enough Scala that does not exhibit the internal bug.

So, the bottom line is: if it can be fixed in scalac, I could avoid introducing a several-dozen-LoC-long hack in Scala.js to work around a scalac bug in an obscure corner case.

[joroKr21]

Yes, erasure is the problem 👍

It's natural to assume that the arguments of a type wouldn't matter for erasure.
But in this case they do.