bug
bug copied to clipboard
scala
correct outer check for match on final inner class
=== What steps will reproduce the problem (please be specific and use wikiformatting)? ===
The following code crashes with a NoSuchMethodError:
final class Outer {
sealed trait Inner
final case class Inner1(foo: Int) extends Inner
val inner: Outer#Inner = Inner1(0)
def bar = inner match {
case Inner1(i) => i
}
}
object Crash {
def main(args: Array[String]) {
println((new Outer).bar)
}
}
The crash does not happen when one of these changes is applied:
-
Inneris asealed abstract classinstead of asealed trait - the type of
val inneris not a type projection but justInner - the class
Inner1is not marked final
The NoSuchMethodError at runtime:
java.lang.NoSuchMethodError: Outer$$Inner1.Outer$$Inner1$$$$$$outer()LOuter;
at Outer.bar(finalcase.scala:9)
at Crash$$.main(finalcase.scala:16)
at Crash.main(finalcase.scala)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
at java.lang.reflect.Method.invoke(Method.java:597)
at scala.tools.nsc.util.ScalaClassLoader$$$$anonfun$$run$$1.apply(ScalaClassLoader.scala:81)
at scala.tools.nsc.util.ScalaClassLoader$$class.asContext(ScalaClassLoader.scala:24)
at scala.tools.nsc.util.ScalaClassLoader$$URLClassLoader.asContext(ScalaClassLoader.scala:91)
at scala.tools.nsc.util.ScalaClassLoader$$class.run(ScalaClassLoader.scala:81)
at scala.tools.nsc.util.ScalaClassLoader$$URLClassLoader.run(ScalaClassLoader.scala:104)
at scala.tools.nsc.ObjectRunner$$.run(ObjectRunner.scala:33)
at scala.tools.nsc.ObjectRunner$$.runAndCatch(ObjectRunner.scala:40)
at scala.tools.nsc.MainGenericRunner$$.runTarget$$1(MainGenericRunner.scala:61)
at scala.tools.nsc.MainGenericRunner$$.process(MainGenericRunner.scala:85)
at scala.tools.nsc.MainGenericRunner$$.main(MainGenericRunner.scala:33)
at scala.tools.nsc.MainGenericRunner.main(MainGenericRunner.scala)
=== What versions of the following are you using? ===
- Scala: 2.9.0.RC1
- Java: 1.6.0_24
- Operating system: OS X 10.6.7
Imported From: https://issues.scala-lang.org/browse/SI-4440?orig=1 Reporter: Moritz Uhlig (muhlig)
@magarciaEPFL said: [[syntax trees at end of cleanup]]// Scala source: bt4.scala
package <empty> {
final class Outer extends java.lang.Object with ScalaObject {
@volatile protected var bitmap$$0: Int = 0;
<synthetic> lazy private[this] var Inner1$$module: object Outer$$Inner1 = _;
<synthetic> <stable> <accessor> lazy def Inner1(): object Outer$$Inner1 = {
if (Outer.this.bitmap$$0.&(1).==(0))
{
Outer.this.synchronized({
if (Outer.this.bitmap$$0.&(1).==(0))
{
Outer.this.Inner1$$module = new object Outer$$Inner1(Outer.this);
Outer.this.bitmap$$0 = Outer.this.bitmap$$0.|(1);
()
};
scala.runtime.BoxedUnit.UNIT
});
()
};
Outer.this.Inner1$$module
};
private[this] val inner: Outer$$Inner = _;
<stable> <accessor> def inner(): Outer$$Inner = Outer.this.inner;
def bar(): Int = {
<synthetic> val temp5: Outer$$Inner = Outer.this.inner();
if (temp5.$$isInstanceOf[Outer$$Inner1]().&&(temp5.$$asInstanceOf[Outer$$Inner1]().Outer$$Inner1$$$$$$outer().eq(Outer.this)))
{
temp5.$$asInstanceOf[Outer$$Inner1]().foo()
}
else
throw new MatchError(temp5)
};
def this(): Outer = {
Outer.super.this();
Outer.this.inner = new Outer$$Inner1(Outer.this, 0);
()
}
};
final object Crash extends java.lang.Object with ScalaObject {
def main(args: Array[java.lang.String]): Unit = scala.this.Predef.println(scala.Int.box(new Outer().bar()));
def this(): object Crash = {
Crash.super.this();
()
}
};
sealed abstract trait Outer$$Inner extends java.lang.Object;
final case class Outer$$Inner1 extends java.lang.Object with Outer$$Inner with ScalaObject with Product with Serializable {
def productIterator(): Iterator = scala.Product$$class.productIterator(Outer$$Inner1.this);
@deprecated("use productIterator instead") def productElements(): Iterator = scala.Product$$class.productElements(Outer$$Inner1.this);
<caseaccessor> <paramaccessor> private[this] val foo: Int = _;
<stable> <caseaccessor> <accessor> <paramaccessor> def foo(): Int = Outer$$Inner1.this.foo;
<synthetic> def copy(foo: Int = foo): Outer$$Inner1 = new Outer$$Inner1(Outer$$Inner1.this.$$outer, foo);
<synthetic> def copy$$default$$1(): Int = Outer$$Inner1.this.foo();
override def hashCode(): Int = ScalaRunTime.this._hashCode(Outer$$Inner1.this);
override def toString(): java.lang.String = ScalaRunTime.this._toString(Outer$$Inner1.this);
override def equals(x$$1: java.lang.Object): Boolean = Outer$$Inner1.this.eq(x$$1).||({
{
<synthetic> val temp1: java.lang.Object = x$$1;
if (temp1.$$isInstanceOf[Outer$$Inner1]())
{
<synthetic> val temp2: Outer$$Inner1 = temp1.$$asInstanceOf[Outer$$Inner1]();
<synthetic> val temp3: Int = temp2.foo();
val foo$$1: Int = temp3;
if (Outer$$Inner1.this.gd1$$1(foo$$1))
{
x$$1.$$asInstanceOf[Outer$$Inner1]().canEqual(Outer$$Inner1.this)
}
else
{
false
}
}
else
{
false
}
}
});
override def productPrefix(): java.lang.String = "Inner1";
override def productArity(): Int = 1;
override def productElement(x$$1: Int): java.lang.Object = {
<synthetic> val temp4: Int = x$$1;
if (temp4.==(0))
{
scala.Int.box(Outer$$Inner1.this.foo())
}
else
{
throw new java.lang.IndexOutOfBoundsException(scala.Int.box(x$$1).toString())
}
};
override def canEqual(x$$1: java.lang.Object): Boolean = x$$1.$$isInstanceOf[Outer$$Inner1]();
<synthetic> <paramaccessor> private[this] val $$outer: Outer = _;
final <synthetic> private[this] def gd1$$1(x$$1: Int): Boolean = x$$1.==(Outer$$Inner1.this.foo());
def this($$outer: Outer, foo: Int): Outer$$Inner1 = {
Outer$$Inner1.this.foo = foo;
if ($$outer.eq(null))
throw new java.lang.NullPointerException()
else
Outer$$Inner1.this.$$outer = $$outer;
Outer$$Inner1.super.this();
scala.Product$$class./*Product$$class*/$$init$$(Outer$$Inner1.this);
()
}
};
final <synthetic> object Outer$$Inner1 extends scala.runtime.AbstractFunction1 with ScalaObject with Serializable {
final override def toString(): java.lang.String = "Inner1";
case <synthetic> def unapply(x$$0: Outer$$Inner1): Option = if (x$$0.==(null))
scala.this.None
else
new Some(scala.Int.box(x$$0.foo()));
case <synthetic> def apply(foo: Int): Outer$$Inner1 = new Outer$$Inner1(Outer$$Inner1.this.$$outer, foo);
<synthetic> <paramaccessor> private[this] val $$outer: Outer = _;
case <synthetic> <bridge> def apply(v1: java.lang.Object): java.lang.Object = Outer$$Inner1.this.apply(scala.Int.unbox(v1));
def this($$outer: Outer): object Outer$$Inner1 = {
if ($$outer.eq(null))
throw new java.lang.NullPointerException()
else
Outer$$Inner1.this.$$outer = $$outer;
Outer$$Inner1.super.this();
()
}
}
}
@lexn82 said: I believe this is related, but does not require a sealed trait or a case class:
object Test {
def main(args: Array[String]) {
(new Clazz).matcher()
}
}
final class Clazz {
private final class SubClazz {
def foo() = 1
}
private val array = new Array[AnyRef](2)
(0 until 2).foreach(i => array(i) = new SubClazz)
def matcher() {
var i = 0; while (i < array.size) { val c = array(i)
c match {
case s: SubClazz => println(s)
case _ => println("no match")
}
}
}
}
@soc said (edited on Apr 20, 2012 5:55:53 PM UTC): Another snippet with probably the same problem, although tha stacktrace seems to be a bit different:
trait ContextContainer {
def Contexts: Contexts
abstract class Contexts extends swing.Publisher {
trait Context extends swing.Publisher {
Contexts.this.listenTo(this)
def fireChanged() {publish(ContextChanged(this))}
}
reactions += {case c @ ContextChanged(_) => publish(c)}
}
final case class ContextChanged(c: Contexts#Context) extends swing.event.Event
}
object ContextsTest extends App with ContextContainer {
object Contexts extends Contexts {
case class NamedContext(name: String) extends Context
val C1 = NamedContext("C1")
}
Contexts.C1.fireChanged() //java.lang.NoSuchMethodError: ContextContainer$ContextChanged.ContextContainer$ContextChanged$$$outer()LContextContainer;
}
java.lang.NoSuchMethodError: ContextContainer$ContextChanged.$line37$$read$ContextContainer$ContextChanged$$$outer()LContextContainer;
at ContextContainer$Contexts$$anonfun$1.isDefinedAt(<console>:18)
at ContextContainer$Contexts$$anonfun$1.isDefinedAt(<console>:18)
at scala.swing.Reactions$Impl$$anonfun$apply$1.apply(Reactions.scala:25)
at scala.swing.Reactions$Impl$$anonfun$apply$1.apply(Reactions.scala:25)
at scala.collection.LinearSeqOptimized$class.foreach(LinearSeqOptimized.scala:59)
at scala.collection.immutable.List.foreach(List.scala:77)
at scala.collection.generic.TraversableForwarder$class.foreach(TraversableForwarder.scala:31)
at scala.collection.mutable.ListBuffer.foreach(ListBuffer.scala:45)
at scala.swing.Reactions$Impl.apply(Reactions.scala:25)
at scala.swing.Reactions$Impl.apply(Reactions.scala:19)
at scala.Function1$class.apply$mcVL$sp(Function1.scala:39)
at scala.swing.Reactions.apply$mcVL$sp(Reactions.scala:46)
at scala.swing.Publisher$$anonfun$publish$1.apply(Publisher.scala:47)
at scala.swing.Publisher$$anonfun$publish$1.apply(Publisher.scala:47)
at scala.collection.Iterator$class.foreach(Iterator.scala:697)
at scala.swing.SingleRefCollection$$anon$4.foreach(Publisher.scala:108)
at scala.collection.IterableLike$class.foreach(IterableLike.scala:73)
at scala.swing.RefSet.foreach(Publisher.scala:165)
at scala.swing.Publisher$class.publish(Publisher.scala:47)
at ContextsTest$Contexts$NamedContext.publish(<console>:26)
at ContextContainer$Contexts$Context$class.fireChanged(<console>:16)
at ContextsTest$Contexts$NamedContext.fireChanged(<console>:26)
at ContextsTest$delayedInit$body.apply(<console>:30)
at scala.Function0$class.apply$mcV$sp(Function0.scala:40)
at scala.runtime.AbstractFunction0.apply$mcV$sp(AbstractFunction0.scala:12)
at scala.App$$anonfun$main$1.apply(App.scala:61)
at scala.App$$anonfun$main$1.apply(App.scala:61)
at scala.collection.LinearSeqOptimized$class.foreach(LinearSeqOptimized.scala:59)
at scala.collection.immutable.List.foreach(List.scala:77)
at scala.collection.generic.TraversableForwarder$class.foreach(TraversableForwarder.scala:31)
at scala.App$class.main(App.scala:61)
at ContextsTest$.main(<console>:24)
at .<init>(<console>:35)
at .<clinit>(<console>)
at .<init>(<console>:7)
at .<clinit>(<console>)
at $print(<console>)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke(Method.java:616)
at scala.tools.nsc.interpreter.IMain$ReadEvalPrint.call(IMain.scala:775)
at scala.tools.nsc.interpreter.IMain$Request$$anonfun$16.apply(IMain.scala:1039)
at scala.tools.nsc.interpreter.Line.scala$tools$nsc$interpreter$Line$$runAndSetState(Line.scala:41)
at scala.tools.nsc.interpreter.Line$$anonfun$2.apply$mcV$sp(Line.scala:47)
at scala.tools.nsc.io.package$$anon$2.run(package.scala:22)
at java.lang.Thread.run(Thread.java:679)
@retronym said (edited on Jun 22, 2012 6:35:35 AM UTC): A pinch of minimization and a sprinkle of analysis:
class Outer {
final class Inner {
// Main$$anon$Outer$Inner1$$$outer removed in the constructors phase.
// because Inner1 is effectively final. (Remove `final` and things work.)
// See:
// $outer method elimination: https://github.com/scala/scala/blob/3631f1d/src/compiler/scala/tools/nsc/transform/Constructors.scala#L226
// outer test in pattern matcher: https://github.com/scala/scala/blob/8284486/src/compiler/scala/tools/nsc/typechecker/PatternMatching.scala#L937
// Supposed elimintion of the outer test: https://github.com/scala/scala/blob/8284486/src/compiler/scala/tools/nsc/transform/ExplicitOuter.scala#L525
}
def bar2 = (new Inner: Any) match {
case i: Inner =>
}
}
println((new Outer).bar2)
// java.lang.NoSuchMethodError: Main$$anon$1$Outer$Inner1.Main$$anon$Outer$Inner1$$$outer()LMain$$anon$1$Outer;
// <synthetic> <stable> def Main$$anon$Outer$Inner1$$$outer(): Outer = Inner1.this.$outer
@adriaanm said (edited on Jul 19, 2012 10:17:39 AM UTC): we used to drop outer accessors from nested final classes if the outer-accessor wasn't accessed anywhere however, for the sake of separate compilation and the outer checks inserted by the pattern matcher, we must assume they might be accessed anywhere
Why single out final classes in the first place?
@adriaanm said: workaround in https://github.com/scala/scala/pull/976
(#951 was rejected because a proper fix would increase memory footprint)
@adriaanm said: In 2.11 we'll do the right thing: the final modifier is not a sufficient condition for omitting the outer pointer. If we can come up with some kind of optimization (hard to do if you can't assume a closed world), we'll optimize memory usage there.
In the mean time, if you don't want an outer pointer, don't nest your class.
@retronym said (edited on Mar 2, 2013 8:11:25 AM UTC): Perhaps a class annotation {{@noouter}}?
@Blaisorblade said: I and Miguel Garcia discussed this bug again on a pull request. Since the current situation was not obvious, here's a link to the post where I explain the current problem:
https://github.com/scala/scala/pull/2634#issuecomment-19173210 Code and results are at https://gist.github.com/Blaisorblade/5745026.
I might post here an updated summary in my copious free time, but until then, I hope that link helps.
@Blaisorblade said: The optimization should still be valid when any of the following conditions holds:
- the Inner class is only visible within the compilation unit, say because Inner (or Outer) is private (but not private[x]), and the compilation unit doesn't use @outer
- the inner class is anonymous (so it can't be used in a pattern match - right?)
- the inner class is annotated with @noouter (Jason Zaugg's proposal).
Here's also a link to the email conversation, for completeness (not sure it contains anything more): https://groups.google.com/d/msg/scala-internals/jQiRINTvhZQ/GrxAJjccdmUJ
@dcsobral said: So, 2.11 is out and the compiler isn't doing "the right thing", so the bug persists. I assume no annotations came forth either?
Malcolm Greaves (malcolmgreaves) said (edited on Nov 24, 2015 12:13:45 AM UTC): Any update on this bug? I just ran into a problem that's related to this one (I think). In my code, if I do the following, I do not get any errors:
trait BinaryTree {
trait Payload
sealed trait Node
case class Parent(left: Option[Node], right: Option[Node]) extends Node
case class Leaf(data: Payload) extends Node
}
However, if I add final to the case classes, then I get the following compiler error:
The outer reference in this type test cannot be checked at run time.
final case class Leaf(d: Payload) extends Node
(Note: I get the same error message for both Leaf and Parent.)
My current solution is to use sealed instead of final. Unlike final, using sealed actually works! Specifically, the following compiles:
trait BinaryTree {
trait Payload
sealed trait Node
sealed case class Parent(left: Option[Node], right: Option[Node]) extends Node
sealed case class Leaf(data: Payload) extends Node
}
Any thoughts on this? I'd like to express the fact that this ADT definition cannot be changed. Including the definitions for Leaf and Parent. Since I'm controlling the definition of the ADT here, I think that using sealed suffices. I'd welcome any other ideas though!
Also, I hope that this can jump-start this stale ticket.
@DarkDimius said: This bug is non-existent in Dotty. See also discussion in https://github.com/lampepfl/dotty/issues/2156 for discusion of outer-checking and pattern matching
Here's another instance of this bug along with another workaround: https://github.com/scalameta/scalameta/issues/900#issuecomment-304483357.
It seems something like this bug can also be hit when you have aliases then try to pattern match on the val alias to the type:
zoolander-util/com/stripe/zoolander/util/scalding/Chunky.scala:33: warning: The outer reference in this type test cannot be checked at run time.
case (TypedPipe.Fork(t), rec) => rec(t).flatMap(_.forceToDiskExecution)
^
zoolander-util/com/stripe/zoolander/util/scalding/Chunky.scala:34: warning: The outer reference in this type test cannot be checked at run time.
case (TypedPipe.ForceToDisk(t), rec) => rec(t).flatMap(_.forceToDiskExecution)
For example:
https://github.com/twitter/scalding/blob/181dc646f7f6ad1ed9d61b012b3bbbdaa2dbaf5f/scalding-core/src/main/scala/com/twitter/scalding/typed/TypedPipe.scala
in TypedPipe above we have a number of final case classes inside the object. We reexport com.twitter.scalding.typed.TypedPipe as com.twitter.scalding.TypedPipe with the following in the package object:
https://github.com/twitter/scalding/blob/181dc646f7f6ad1ed9d61b012b3bbbdaa2dbaf5f/scalding-core/src/main/scala/com/twitter/package.scala#L26
val TypedPipe = com.twitter.scalding.typed.TypedPipe
type TypedPipe[+T] = com.twitter.scalding.typed.TypedPipe[T]
using the un-aliased types seems to side-step the the issue.
@johnynek This is how we work around the problem in Scalameta. Annoying but effective: https://github.com/scalameta/scalameta/blob/v3.2.0/langmeta/langmeta/shared/src/main/scala/org/langmeta/inputs/Api.scala.
Here's the simplest reproducer I could find:
scala> :setting -unchecked
scala> :paste
// Entering paste mode (ctrl-D to finish)
abstract class A {
sealed abstract class B
final case class C(value: Int) extends B
}
// Exiting paste mode, now interpreting.
<pastie>:14: warning: The outer reference in this type test cannot be checked at run time.
final case class C(value: Int) extends B
^
defined class A
Changing the final case class C to sealed case class C eliminates the warning.
Note that in my case I actually eliminated the warning by moving the class declarations to the companion object - an oversight on my part.
Adriaan also gave some examples in scala-internals in 2013: https://groups.google.com/d/msg/scala-internals/vw8Kek4zlZ8/EMipRxRVQi0J
The warning refers to the synthesized equals method, which cannot test the outer pointer for Inner (as we don't an outer accessor for final nested classes for reasons unknown to me).
scala> class Outer { | final case class Inner(n: Int) | } warning: there were 1 unchecked warning(s); re-run with -unchecked for details defined class Outer scala> val o1 = new Outer o1: Outer = Outer@52c54b3b scala> val o2 = new Outer o2: Outer = Outer@7cef7efe scala> (new o1.Inner(1)) == (new o2.Inner(1)) res0: Boolean = trueWhen an outer accessor is available, you'll get the correct result:
scala> class Outer { case class Inner(n: Int) // look ma, no final! } defined class Outer scala> val o1 = new Outer o1: Outer = Outer@2a29a451 scala> val o2 = new Outer o2: Outer = Outer@6cb8daa7 scala> (new o1.Inner(1)) == (new o2.Inner(1)) res1: Boolean = false
It seems there are a few issues here:
- There seems to be a trade-off between the optimization to elide the outer reference vs. sound pattern matching. It seems right now the outer reference is removed if it is possible to do so for other reasons even if it breaks pattern matching. It seems one way to control it is using the
finalmodifier. Should that really be the API? (Also, is the optimization a good idea in the first place? Guessing about whether an outer reference is included or not seems quite brittle for serialization. If you need serialization, be explicit and provide explicit data structures for that matter. Or was removing extra outer references the main purpose, that would seems more useful.) - The warning message is really useless. It doesn't explain or point to the above trade-off. There are probably multiple ways you could resolve the situation from the view of the user:
- The inner class doesn't need to be an inner class and shouldn't have been path dependent in the first place => the user could move the class out to somewhere else
- You have a pattern match but you don't care for the outer match because the value you get in will always have the right outer reference => either improve the compiler to prove that or point the user to add
@uncheckedto the pattern match - You really need the outer check => remove
final(or whatever else the API is) to make sure the outer reference is kept (but point out the tradeoff)
- The warning message is shown at the definition of the class not where it is used in a pattern match (see the code snippet @dwijnand posted right above). This should be fixed to remove spurious warnings. If you don't pattern match you shouldn't get a warning in the first place. I guess the reason might be the generated
unapplyforcase classes? In that case, the warning should be propagated to use-sites of theunapplymethod. - Update: and there is https://github.com/scala/bug/issues/6813
+1 on all your points. We should not use final as the "API" for omitting outer pointers. There should perhaps be some way to control unintended capture. Spores solve this for closures, but there are more ways to subtly capture the environment, as illustrated here. I would be happy to have a comprehensive solution in this space that's better integrated into the core. Note there are also opportunities in lambdalift to be more aggressive in pulling closures to outer context to avoid capturing more (as is already done in Dotty).
spores use macros (which I thought are kind of on the chopping block), and have not been published for 2.12: https://search.maven.org/#search%7Cga%7C1%7Cch.epfl.scala.spores
Is there any chance to get a supported version of spores into scala? It would be great for GC sensitive apps (controlling what you close over) as well as serialization cases (spark, scalding, etc...)
@johnynek FWIW for spores 2.12, see https://github.com/scalacenter/spores/issues/21 (that's currently the master repo), but I'll leave your other questions to others.
@dwijnand did this progress in https://github.com/scala/bug/issues/12398 ?
No. The original issue was a duplicate of this, but I repurposed it for the specific case where the outer check isn't required (with Singleton):
@dwijnand changed the title ~Type prefix outer reference is not checked in type tests~ Path-dependent type on singleton value emits spurious outer test warnings 4 days ago
This case is unfixable in 2.13, because we can't change the binary format of nested final inner classes mid-2.13.
meanwhile over in Scala 3, https://github.com/lampepfl/dotty/issues/13096
