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

AbstractMethodError for setter in trait #10477

Closed
szeiger opened this issue Aug 21, 2017 · 14 comments · Fixed by scala/scala#7627
Closed

AbstractMethodError for setter in trait #10477

szeiger opened this issue Aug 21, 2017 · 14 comments · Fixed by scala/scala#7627

Comments

@szeiger
Copy link

szeiger commented Aug 21, 2017

The following fails on Scala 2.12 (tested with .0 and .3):

abstract class BasicBackend {
  type F
  val f: F
}

class DistributedBackend extends BasicBackend {
  type F = FImpl
  val f: F = new FImpl
  class FImpl
}

trait BasicProfile {
  type Backend <: BasicBackend
  val backend: Backend
  trait SimpleQL {
    val f: backend.F = backend.f
  }
}

trait DistributedProfile extends BasicProfile { _: DistributedDriver =>
  type Backend = DistributedBackend
  val backend: Backend = new DistributedBackend
  val simple: SimpleQL = new SimpleQL {}
}

class DistributedDriver extends DistributedProfile

object Test extends App {
  new DistributedDriver()
}

I was unable to minimize it further. Any small change makes it work. The error is:

java.lang.AbstractMethodError: DistributedProfile$$anon$1.BasicProfile$SimpleQL$_setter_$f_$eq(Ljava/lang/Object;)V
       	at BasicProfile$SimpleQL.$init$(Test.scala:16)
       	at DistributedProfile$$anon$1.<init>(Test.scala:23)
       	at DistributedProfile.$init$(Test.scala:23)
       	at DistributedDriver.<init>(Test.scala:26)
       	at Test$.<init>(Test.scala:29)
       	at Test$.<clinit>(Test.scala)
       	at Test.main(Test.scala)

The minimized test case is based on slick/slick@2.1...mr-git:2.1 which makes Slick 2.1 compile on Scala 2.12.

@sjrd
Copy link
Member

sjrd commented Aug 21, 2017

The same example causes a linking error in Scala.js:

[error] Referring to non-existent method helloworld.DistributedProfile$$anon$1.helloworld$BasicProfile$SimpleQL$$undsetter$und$f$und$eq(java.lang.Object)scala.Unit
[error]   called from helloworld.BasicProfile$SimpleQL.$$init$()scala.Unit
[error]   called from helloworld.DistributedProfile$$anon$1.<init>(helloworld.DistributedDriver)
[error]   called from helloworld.DistributedProfile.$$init$()scala.Unit
[error]   called from helloworld.DistributedDriver.<init>()
[error]   called from helloworld.Test$.main([java.lang.String)scala.Unit
[error]   called from core module module initializers
[error] involving instantiated classes:
[error]   helloworld.Test$

@nafg
Copy link

nafg commented Aug 22, 2017

Is there any workaround? I'm using slick-pg which seems to require this pattern.

@nafg
Copy link

nafg commented Aug 22, 2017

I reproduced my issue which I think is the same on 2.12.1, FWIW

@retronym
Copy link
Member

Perhaps related to #10308.

We seem to be missing a bridge.

@retronym
Copy link
Member

The bridge method is omitted because the trait setter in the subclass (anonymous above, SimpleQLImpl in my version) isn't deemed to override the one in SimpleQL.

This seems to be because isSameType is being called on prefixes involving different backend Symbols.

trait BasicProfile {
    <method> <deferred> <stable> <accessor> <triedcooking> def backend#7860(): BasicProfile#6740.this.Backend#7859;
    trait SimpleQL{
      <method> <deferred> <mutable> <accessor> <sub_synth> protected[this] def BasicProfile$SimpleQL$_setter_$f_$eq#8744(x$1#8745: BasicProfile#6740.this.backend#7860.F#6905): scala#23.this.Unit#1816;
...
trait DistributedProfile { self: DistributedDriver =>
   <method> <stable> <accessor> <sub_synth> def backend#7877():
   class SimpleQlImpl {
      <method> override <mutable> <accessor> protected[this] def BasicProfile$SimpleQL$_setter_$f_$eq#8750(x$1#8754: DistributedProfile#6767.this.backend#7877.FImpl#7854): scala#23.this.Unit#1816 = ...

The self type is involved, but I haven't connected all the dots.

Making prefix unification a little looser for ThisType-s over self-type-bearing classes
"fixes" the problem, but this might just be a bandaid. https://github.com/retronym/scala/tree/ticket/10477

@retronym
Copy link
Member

Related scala/scala-dev#219 scala/scala-dev#268

  // scala/scala-dev#219, scala/scala-dev#268
  // Cast to avoid spurious mismatch in paths containing trait vals that have
  // not been rebound to accessors in the subclass we're in now.
  // For example, for a lazy val mixed into a class, the lazy var's info
  // will not refer to symbols created during our info transformer,
  // so if its type depends on a val that is now implemented after the info transformer,
  // we'll get a mismatch when assigning `rhs` to `lazyVarOf(getter)`.
  // TODO: could we rebind more aggressively? consider overriding in type equality?
  def castHack(tree: Tree, pt: Type) = gen.mkAsInstanceOf(tree, pt)

@retronym
Copy link
Member

@adriaanm WDYT? My patch seems to be taking a step down "consider overriding in type equality" path. Maybe we can limit it to cases where we have the SYNTHESIZE_IMPL_IN_SUBCLASS / MIXED_IN flags on the pair of symbols? (We don't yet add the latter flag to the mixed in accessor, a todo comment in fields suggests we should, though.)

@retronym
Copy link
Member

retronym commented Aug 22, 2017

Here's a workaround that you might be able to use in the meantime.

--- test/files/run/t10477.scala	2017-08-22 14:46:28.000000000 +1000
+++ sandbox/test.scala	2017-08-22 14:44:33.000000000 +1000
@@ -23,8 +23,11 @@
   val simple: SimpleQL = new SimpleQL {}
 }
 
-class DistributedDriver extends DistributedProfile
+trait DistributedDriver extends DistributedProfile {
+  override val backend: DistributedBackend
+}
+class DistributedDriver1 extends DistributedDriver
 
 object Test extends App {
-  new DistributedDriver()
+  new DistributedDriver1()
 }

@adriaanm
Copy link
Contributor

adriaanm commented Aug 22, 2017

I think it's also worth looking at the rebinding angle:

  /** Rebind symbol `sym` to an overriding member in type `pre`. */
  private def rebind(pre: Type, sym: Symbol): Symbol = {
    if (!sym.isOverridableMember || sym.owner == pre.typeSymbol) sym
    else pre.nonPrivateMember(sym.name).suchThat { sym =>
      // scala/bug#7928 `isModuleNotMethod` is here to avoid crashing with spuriously "overloaded" module accessor and module symbols.
      //         These appear after the fields phase eliminates ModuleDefs that implement an interface.
      //         Here, we exclude the module symbol, which allows us to bind to the accessor.
      // scala/bug#8054 We must only do this after fields, otherwise we exclude the module symbol which does not yet have an accessor!
      val isModuleWithAccessor = phase.assignsFields && sym.isModuleNotMethod
      sym.isType || (!isModuleWithAccessor && sym.isStable && !sym.hasVolatileType)
    } orElse sym
  }

@adriaanm
Copy link
Contributor

I've gone with @retronym's suggestion. I'm not super happy with the whole name-based unification that's going on in subtyping, but since we already have it, and you could argue it was an oversight there was no case for ThisType (which will widen to a refined type), let's do this!

timorantalaiho added a commit to Opetushallitus/hakurekisteri that referenced this issue Jan 28, 2019
timorantalaiho added a commit to Opetushallitus/hakurekisteri that referenced this issue Jan 31, 2019
@nafg
Copy link

nafg commented Feb 24, 2019

No fix in 2.12.x?

@SethTisue
Copy link
Member

No fix in 2.12.x?

It seems likely that a pull request with a backport would be accepted.

timorantalaiho added a commit to Opetushallitus/hakurekisteri that referenced this issue Feb 27, 2019
timorantalaiho added a commit to Opetushallitus/hakurekisteri that referenced this issue Feb 27, 2019
timorantalaiho added a commit to Opetushallitus/hakurekisteri that referenced this issue Mar 1, 2019
timorantalaiho added a commit to Opetushallitus/hakurekisteri that referenced this issue Mar 29, 2019
timorantalaiho added a commit to Opetushallitus/hakurekisteri that referenced this issue Apr 2, 2019
timorantalaiho added a commit to Opetushallitus/hakurekisteri that referenced this issue Apr 5, 2019
timorantalaiho added a commit to Opetushallitus/hakurekisteri that referenced this issue Apr 8, 2019
msiukola pushed a commit to Opetushallitus/hakurekisteri that referenced this issue Apr 9, 2019
timorantalaiho added a commit to Opetushallitus/hakurekisteri that referenced this issue Apr 9, 2019
timorantalaiho added a commit to Opetushallitus/hakurekisteri that referenced this issue Apr 11, 2019
tatu1 pushed a commit to Opetushallitus/hakurekisteri that referenced this issue Apr 12, 2019
@nafg
Copy link

nafg commented Jul 16, 2020

This doesn't seem to work, even in 2.13.3. For example, I can't find a way to shorten this:

trait SlickProfile extends slick.jdbc.PostgresProfile with slick.additions.AdditionsProfile {
  object myApi extends API with AdditionsApi
  override val api = myApi
}
object SlickProfile extends SlickProfile

Ideally it would be at least

object SlickProfile extends slick.jdbc.PostgresProfile with slick.additions.AdditionsProfile {
  object myApi extends API with AdditionsApi
  override val api = myApi
}

or even

object SlickProfile extends slick.jdbc.PostgresProfile with slick.additions.AdditionsProfile {
  override object api extends API with AdditionsApi
}

@SethTisue
Copy link
Member

@nafg it would increase the likelihood of someone choosing to poke at it if you 1) (preferably) supplied a version of the code that doesn't require external dependencies, or 2) (failing that) give us the coordinates to the required dependencies in a form that can easily be used in sbt or ammonite

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

Successfully merging a pull request may close this issue.

6 participants