Skip to content

AbstractMethodError with anonymous class #7597

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
bishabosha opened this issue Nov 21, 2019 · 10 comments
Closed

AbstractMethodError with anonymous class #7597

bishabosha opened this issue Nov 21, 2019 · 10 comments

Comments

@bishabosha
Copy link
Member

bishabosha commented Nov 21, 2019

minimized code

def foo[S <: String]: String => Int = new { def apply(s: S): Int = 0 }
val test = foo("")

runtime error

java.lang.AbstractMethodError: Method conversions/Conversions$package$$anon$1.apply(Ljava/lang/Object;)Ljava/lang/Object; is abstract, took 0.002 sec
     at conversions.Conversions$package$$anon$1.apply(Conversions.scala)
     at conversions.Conversions$package$.<init>(Conversions.scala:4)
     at conversions.Conversions$package$.<clinit>(Conversions.scala)
     ...

expectation

Some type error that apply is not implemented

@ashwinbhaskar
Copy link
Contributor

The error is not with anonymous class here. I get the same error error even with the below code (which is what the FrontEnd phase of the dotty compiler generates even for the code mentioned in the issue)

def foo[A <: String]: Function1[String, Int] = new Function1[String, Int] with
    def apply(a: A): Int = 1

The problem is the dotty compiler compiles this code but it should have thrown an error saying that that apply(String): Int is not implemented.

The Scalac compiler throws the expected error

error: object creation impossible, since method apply in trait Function1 of type (v1: String)Int is not defined

@ashwinbhaskar
Copy link
Contributor

Documenting my observations so far

  • The error should be generated in RefChecks.scala which happens to be a miniphase in MegaPhase.
  • This is where the object creation impossible... error is thrown.
  • But for the error to be thrown, clazz.thisType.abstractTermMembers cannot be empty. Code is here
  • In our case clazz.thisType.abstractTermMembers turns out to be empty and hence our error goes unnoticed.

@ashwinbhaskar
Copy link
Contributor

Documenting further observations when the running the program

def foo[A <: String]: Function1[String, Int] = new Function1[String, Int] with
    def apply(a: A): Int = 1

Printing the value of pre.nonPrivateMember(name) for Symbol class $anon at this line.

Actual output

nonPrivateMemberName = method ==
nonPrivateMemberName = method !=
nonPrivateMemberName = method equals
nonPrivateMemberName = method hashCode
nonPrivateMemberName = method toString
nonPrivateMemberName = method ##
nonPrivateMemberName = method getClass
nonPrivateMemberName = method isInstanceOf
nonPrivateMemberName = method asInstanceOf
nonPrivateMemberName = method $isInstanceOf$
nonPrivateMemberName = method $asInstanceOf$
nonPrivateMemberName = method <init>
nonPrivateMemberName = method eq
nonPrivateMemberName = method ne
nonPrivateMemberName = method synchronized
nonPrivateMemberName = method clone
nonPrivateMemberName = method finalize
nonPrivateMemberName = method notify
nonPrivateMemberName = method notifyAll
nonPrivateMemberName = method wait <and> method wait <and> method wait
nonPrivateMemberName = method wait <and> method wait <and> method wait
nonPrivateMemberName = method wait <and> method wait <and> method wait
nonPrivateMemberName = val <none>
nonPrivateMemberName = val <none>
nonPrivateMemberName = method <init>
nonPrivateMemberName = method apply
nonPrivateMemberName = method compose
nonPrivateMemberName = method andThen
nonPrivateMemberName = method toString
nonPrivateMemberName = method apply
nonPrivateMemberName = method <init>
nonPrivateMemberName = method apply

Expected Output

nonPrivateMemberName = method ==
nonPrivateMemberName = method !=
nonPrivateMemberName = method equals
nonPrivateMemberName = method hashCode
nonPrivateMemberName = method toString
nonPrivateMemberName = method ##
nonPrivateMemberName = method getClass
nonPrivateMemberName = method isInstanceOf
nonPrivateMemberName = method asInstanceOf
nonPrivateMemberName = method $isInstanceOf$
nonPrivateMemberName = method $asInstanceOf$
nonPrivateMemberName = method <init>
nonPrivateMemberName = method eq
nonPrivateMemberName = method ne
nonPrivateMemberName = method synchronized
nonPrivateMemberName = method clone
nonPrivateMemberName = method finalize
nonPrivateMemberName = method notify
nonPrivateMemberName = method notifyAll
nonPrivateMemberName = method wait <and> method wait <and> method wait
nonPrivateMemberName = method wait <and> method wait <and> method wait
nonPrivateMemberName = method wait <and> method wait <and> method wait
nonPrivateMemberName = val <none>
nonPrivateMemberName = val <none>
nonPrivateMemberName = method <init>
nonPrivateMemberName = method apply
nonPrivateMemberName = method compose
nonPrivateMemberName = method andThen
nonPrivateMemberName = method toString
nonPrivateMemberName = method apply <and> <SingleDenotation of type MethodType(List(v1), List(TypeRef(TermRef(ThisType(TypeRef(NoPrefix,module class scala)),module Predef),type String)), TypeRef(TermRef(ThisType(TypeRef(NoPrefix,module class <root>)),module scala),class Int))>
nonPrivateMemberName = method <init>
nonPrivateMemberName = method apply <and> <SingleDenotation of type MethodType(List(v1), List(TypeRef(TermRef(ThisType(TypeRef(NoPrefix,module class scala)),module Predef),type String)), TypeRef(TermRef(ThisType(TypeRef(NoPrefix,module class <root>)),module scala),class Int))>
nonPrivateMemberName = method apply <and> <SingleDenotation of type MethodType(List(v1), List(TypeRef(TermRef(ThisType(TypeRef(NoPrefix,module class scala)),module Predef),type String)), TypeRef(TermRef(ThisType(TypeRef(NoPrefix,module class <root>)),module scala),class Int))>

@Blaisorblade
Copy link
Contributor

I'm not sure we can throw the same error as Scalac, in the current architecture: abstractTermMembers is computed using the signatures (which is supposed to match what happens after erasure), and the signature of def apply(s: S): Int = 0 is the one for apply(s: String): Int, correctly. I'm less sure about the erasure of Function1[String, Int].apply.

On Gitter, I proposed to fix this by using subtype checking, instead of going by signatures. If member M overrides member O, sometimes Dotty checks that the type of M is a subtype of the type of O (before erasure)*; and here, (s: S): Int is not a subtype of (s: String): Int. Moreover, Dotty's subtype checker already has code for subtype checking of MethodType and PolyType:
https://github.com/lampepfl/dotty/blob/0.21.0-RC1/compiler/src/dotty/tools/dotc/core/TypeComparer.scala#L650-L668

@Blaisorblade
Copy link
Contributor

Blaisorblade commented Dec 22, 2019

Notwithstanding all I said, I wonder if this code should be rejected (tho it's rejected in Scala 2):

scala> object Bar {
     |   def foo[A >: String]: Function1[String, Int] = new Function1[String, Int] {
     |     def apply(a: A): Int = 1
     |   }
     | }
2 |  def foo[A >: String]: Function1[String, Int] = new Function1[String, Int] {
  |                                                 ^
  |object creation impossible, since def apply(v1: String): Int is not defined
  |(Note that T1 does not match A)

I wonder because this can be fixed via manual bridging, tho the resulting code is a bit silly

object Bar {
  def foo[A >: String]: Function1[String, Int] = new Function1[String, Int] {
    def apply(a: A): Int = 1
    def apply(x: String): Int = apply(x: A)
  }
}

or via

scala> object Bar {
  def foo[A >: String]: Function1[String, Int] = new Function1[A, Int] {
    def apply(a: A): Int = 1
  }
}

and this makes me start to wonder if erasing abstract types in negative positions should use the lower bound instead... tho that sounds extremely invasive (and I'd guess wrong somewhere else?).

@ashwinbhaskar
Copy link
Contributor

@Blaisorblade should this the expected error?

@Blaisorblade
Copy link
Contributor

@ashwinbhaskar yes, that's what I'd expect from tuning step 1.8.

ashwinbhaskar added a commit to ashwinbhaskar/dotty that referenced this issue Dec 27, 2019
ashwinbhaskar added a commit to ashwinbhaskar/dotty that referenced this issue Jan 1, 2020
@ashwinbhaskar
Copy link
Contributor

I'm not sure we can throw the same error as Scalac, in the current architecture: abstractTermMembers is computed using the signatures (which is supposed to match what happens after erasure), and the signature of def apply(s: S): Int = 0 is the one for apply(s: String): Int, correctly. I'm less sure about the erasure of Function1[String, Int].apply.

On Gitter, I proposed to fix this by using subtype checking, instead of going by signatures. If member M overrides member O, sometimes Dotty checks that the type of M is a subtype of the type of O (before erasure)*; and here, (s: S): Int is not a subtype of (s: String): Int. Moreover, Dotty's subtype checker already has code for subtype checking of MethodType and PolyType:
https://github.com/lampepfl/dotty/blob/0.21.0-RC1/compiler/src/dotty/tools/dotc/core/TypeComparer.scala#L650-L668

@Blaisorblade
For the step 1.8 check to happen, the method checkOverride needs to be called. checkOverride is called here. But that is on the mercy of opc.hasNext.

Now coming to OverridingPairs class, on initialising the Cursor class inside OverridingPairs, the next method is called once. next method being recursive can terminate only here. For the execution to reach there, matches(overriding, overridden) must return true; which does not happen as this check is before erasure.
So we basically end up not calling the checkOverride method at all.

I tried to change this line to if (overriding.owner != overridden.owner && !matches(overriding, overridden)) (Added ! before matches). This fixes the problem in our example but fails in other cases with errors similar to

error overriding method eq in class Object of type (x$0: Object): Boolean;
    |        value eq of type Eq[Lst[Int]] cannot override final member method eq in class Object

@Blaisorblade
Copy link
Contributor

Blaisorblade commented Jan 1, 2020

Hm, this is tricky. One must design together OverridingPairs.matches, abstractTermMembers, and bridge generation in https://github.com/lampepfl/dotty/blob/d45fea099a35cfb50faad6c806fac8ff95fe60fb/compiler/src/dotty/tools/dotc/transform/Bridges.scala#L47.

I'm now less sure about the code calling abstractTermMembers; but it still seems that, if that should be changed, the problem cannot be due to the erasure behavior you were questioning on Gitter.

One idea is to try changing OverridingPairs.matches to check for override after erasure, hence compare signatures, like abstractTermMembers. Then, it would check that methods that override each other after erasure are also overriding each other before. However, after looking at bridge generation, that seems questionable — bridge generation depends on the current behavior of OverridingPairs.matches.

@Blaisorblade
Copy link
Contributor

Going back to #7597 (comment), abstractTermMembers could even be correct, but maybe RefChecks should use something more careful. But I'd rather not suggest what, as I think I'm out of my depth.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants