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

Serializing val that refers to another val captures enclosing class #8033

Closed
travisbrown opened this issue Jan 19, 2020 · 3 comments
Closed

Comments

@travisbrown
Copy link
Contributor

minimized code

trait Okay extends Serializable {
  def okay: Okay
}

class Foo {
  def okay1: Okay = new Okay() {
    val okay: Okay = this
  }
  def okay2: Okay = new Okay {
    val okay: Okay = okay1
  }
}

object Test {
  def main(args: Array[String]): Unit = {
    val foo = new Foo
    println(roundTrip(foo.okay1))
    println(roundTrip(foo.okay2))
  }

  def roundTrip[A](a: A): A = {
    import java.io._

    val aos = new ByteArrayOutputStream()
    val oos = new ObjectOutputStream(aos)
    oos.writeObject(a)
    oos.close()
    val ais = new ByteArrayInputStream(aos.toByteArray())
    val ois: ObjectInputStream = new ObjectInputStream(ais)
    val newA = ois.readObject()
    ois.close()
    newA.asInstanceOf[A]
  }
}
Compiles but fails at runtime
Foo$$anon$1@10234dd6
Exception in thread "main" java.io.NotSerializableException: Foo
	at java.io.ObjectOutputStream.writeObject0(ObjectOutputStream.java:1184)
	at java.io.ObjectOutputStream.defaultWriteFields(ObjectOutputStream.java:1548)
	at java.io.ObjectOutputStream.writeSerialData(ObjectOutputStream.java:1509)
	at java.io.ObjectOutputStream.writeOrdinaryObject(ObjectOutputStream.java:1432)
	at java.io.ObjectOutputStream.writeObject0(ObjectOutputStream.java:1178)
	at java.io.ObjectOutputStream.writeObject(ObjectOutputStream.java:348)
	at Test$.roundTrip(Test.scala:26)
	at Test$.main(Test.scala:18)
	at Test.main(Test.scala)

expectation

This is minimisation from a failing test in Cats, where this arrangement has never been a problem in Scala 2, and the code above runs fine on Scala 2:

Foo$$anon$1@fc3b81d0
Foo$$anon$2@16d6d0a3

Note that if you change val okay in okay2 to a def, you see the same error on Scala 2 and Dotty.

@smarter
Copy link
Member

smarter commented Jan 19, 2020

scalac generates an anonymous class that decompiles to this:

public final class Foo$$anon$2
implements Okay {
    private final Okay okay;

    @Override
    public Okay okay() {
        return this.okay;
    }

    public Foo$$anon$2(Foo $outer) {
        this.okay = $outer.okay1();
    }
}

Whereas we generate this:

private static final class Foo$$anon$2
implements Okay {
    private final Okay okay;
    private final Foo $outer;

    public Foo$$anon$2(Foo $outer) {
        if ($outer == null) {
            throw new NullPointerException();
        }
        this.$outer = $outer;
        this.okay = this.Foo$_$$anon$$$outer().okay1();
    }

    @Override
    public Okay okay() {
        return this.okay;
    }

    private Foo $outer() {
        return this.$outer;
    }

    public final Foo Foo$_$$anon$$$outer() {
        return this.$outer();
    }
}

I guess https://github.com/lampepfl/dotty/blob/master/compiler/src/dotty/tools/dotc/transform/ExplicitOuter.scala needs to learn to not generate outer accessors when the outer class is only referenced in the constructor ?

@travisbrown
Copy link
Contributor Author

@smarter It's not hard for us to work around this, but it wasn't especially easy to characterise at first, and seems likely to cause confusion for other people as well.

@smarter
Copy link
Member

smarter commented Jan 19, 2020

I guess https://github.com/lampepfl/dotty/blob/master/compiler/src/dotty/tools/dotc/transform/ExplicitOuter.scala needs to learn to not generate outer accessors when the outer class is only referenced in the constructor ?

Actually, scalac also generates that outer accessor and field in ExplicitOuter, but optimizes it away in its Constructors phase. Our Constructors phase should be doing similar optimizations but apparently they're not working as well.

@odersky odersky self-assigned this Jan 22, 2020
odersky added a commit to dotty-staging/dotty that referenced this issue Feb 8, 2020
odersky added a commit that referenced this issue Feb 17, 2020
Fix #8033: Eliminate unnecessary outer accessors
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