-
Notifications
You must be signed in to change notification settings - Fork 21
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
lambdalift crashes when Block in constructor-argument position requires outer() access #6666
Comments
Imported From: https://issues.scala-lang.org/browse/SI-6666?orig=1 |
@magarciaEPFL said (edited on Nov 14, 2012 3:34:17 PM UTC):
Sidenote: plans for MethodHandles at http://lamp.epfl.ch/~magarcia/ScalaCompilerCornerReloaded/2012Q4/2012-11-04-FasterClosures.pdf |
@magarciaEPFL said:
|
@retronym said: scala> class IP extends Foo( {
| val dummy = {
| def poorMansMH1 = println("!")
| object X {
| poorMansMH1
| }
| X
| }
|
| dummy
| }
| )
defined class IP
scala> new IP
java.lang.VerifyError: (class: IP, method: <init> signature: ()V) Expecting to find object/array on stack |
@retronym said (edited on Dec 9, 2012 9:26:01 PM UTC): The difference in behaviour between the original case and the case in which the object is nested in def inConstructorFlag: Long =
if (owner.isConstructor && !context.inConstructorSuffix || owner.isEarlyInitialized) INCONSTRUCTOR
else 0l class ClassSymbol {
override def isClassLocalToConstructor = this hasFlag INCONSTRUCTOR
} Perhaps not relevant to this problem, but for completeness, I'll note that for functions translated into anonymous classes, this flag is set in if (dd.symbol.isClassConstructor) {
atOwner(sym) {
val rhs1 = (rhs: @unchecked) match {
case Block(stats, expr) =>
def transformInConstructor(stat: Tree) =
withInConstructorFlag(INCONSTRUCTOR) { transform(stat) }
val presupers = treeInfo.preSuperFields(stats) map transformInConstructor
val rest = stats drop presupers.length
val supercalls = rest take 1 map transformInConstructor
val others = rest drop 1 map transform
treeCopy.Block(rhs, presupers ::: supercalls ::: others, transform(expr))
}
treeCopy.DefDef(
dd, mods, name, transformTypeDefs(tparams),
transformValDefss(vparamssNoRhs), transform(tpt), rhs1)
}
} else {
super.transform(treeCopy.DefDef(dd, mods, name, tparams, vparamssNoRhs, tpt, rhs))
}
...
case Template(_, _, _) =>
withInConstructorFlag(0) { super.transform(tree) }
...
val anonClass = fun.symbol.owner newAnonymousFunctionClass(fun.pos, inConstructorFlag) addAnnotation serialVersionUIDAnnotation The accessor for this flag: // class C extends D( { class E { ... } ... } ). Here, E is a class local to a constructor
def isClassLocalToConstructor = false is only used in /** The class that is logically an outer class of given `clazz`.
* This is the enclosing class, except for classes defined locally to constructors,
* where it is the outer class of the enclosing class.
*/
final def outerClass: Symbol =
if (owner.isClass) owner
else if (isClassLocalToConstructor) owner.enclClass.outerClass
else owner.outerClass Which is in turn only used in So, after all that, a question: should class C extends D( { val x = { class E } }) If so, what about? (I'd say no.) class C extends D( { object O { class E } }) And a trickier one: class C extends D( { () => { class E } })
class C extends D( assert(true, /*by-name*/{ class E } ) |
@paulp said: implementation restriction: you can't declare classes in the midst of a supercall
scalac editorializes: come on, don't tell me this is so important you want us
picking through debris for INCONSTRUCTOR flag clues. |
@retronym said (edited on Dec 9, 2012 10:27:36 PM UTC): class C(a: Any)
object F {
def byname(a: => Any) = println(a)
def hof(a: () => Any) = println(a())
}
// java.lang.NullPointerException
// at O1$$anonfun$$init$$1.apply(<console>:11)
//
// The thunk's apply method accesses the MODULE$
// field before it is set.
//
// 0: getstatic #23; //Field O1$.MODULE$:LO1$;
// 3: invokevirtual #26; //Method O1$.O1$$x$1:()Ljava/lang/String;
object O1 extends C({
def x = "".toString
F.byname(x)
})
// java.lang.NullPointerException
// at O2$$anonfun$$init$$1.apply(<console>:11)
object O2 extends C({
lazy val x = "".toString
F.byname(x)
})
// java.lang.NullPointerException
// at O3$$anonfun$$init$$1.apply(<console>:11)
object O3 extends C({
def x = "".toString
F.hof(() => x)
})
// java.lang.VerifyError: (class: O5$, method: <init> signature: ()V) Expecting to find object/array on stack
object O4 extends C({
object Nested
Nested
})
// Okay, the nested classes don't get an outer pointer passed,
// just an extra param for `x: String`.
object O6 extends C({
val x = "".toString
F.byname(x); F.hof(() => x); (new { val xx = x }.xx)
})
// no-symbol does not have an owner
// ...
// at scala.tools.nsc.transform.ExplicitOuter$OuterPathTransformer.outerSelect(ExplicitOuter.scala:229)
class C1 extends C({
def x = "".toString
F.byname(x)
})
class C2 extends C({
lazy val x = "".toString
F.byname(x)
})
class C3 extends C({
def x = "".toString
F.hof(() => x)
})
class C4 extends C({
def x = "".toString
object Nested { def xx = x}
Nested.xx
})
// java.lang.VerifyError: (class: C5, method: <init> signature: ()V) Expecting to find object/array on stack
class C5 extends C({
def x = "".toString
val y = {
object Nested { def xx = x}
Nested.xx
}
})
// okay, for same reason as O6
class C6 extends C({
val x = "".toString
F.byname(x); F.hof(() => x); (new { val xx = x }.xx)
})
|
@paulp said: |
@adriaanm said: Jason, as you're on top of this, assigning to you. |
@retronym said (edited on Dec 10, 2012 7:13:15 AM UTC): I might need a little help to find the best place to put the check. It has to be post-uncurry (to see expanded by-name thunks). explicitouter? lambdalift? |
@magarciaEPFL said: The experimental optimizer can live without this bug being fixed, in fact it currently sidesteps the problematic case via:
|
@retronym said: import scala.collection.immutable.TreeMap
import scala.math.Ordering
class Test[K](param:TreeMap[K,Int]){
def this() = this({
implicit object TreeOrd extends Ordering[K](){
def compare(a:K, b:K) = {
-1
}
}
new TreeMap[K, Int]()
})
} |
@retronym said: |
@retronym said: retronym/scala@scala:2.10.x...retronym:ticket/6666-wip Along the way I've had to reopen #6259 (which was flagged, correctly, by the new checks.) #6997 is not yet prevented. |
@JamesIry said: |
@JamesIry said: |
@retronym said: |
As discussed in https://groups.google.com/d/topic/scala-internals/Ief8xGFd6fU/discussion , currently lambdalift crashes on:
The motivation for this ticket is an alternative lowering for closures that is amenable to MethodHandles (both the real thing and so-called "Poor Man's Method Handles" , https://groups.google.com/d/topic/scala-internals/f7TektSb26s/discussion , ie the general rule is:
Thus the following (longer) example is representative of a use case around "Poor Man's Method Handles"
The text was updated successfully, but these errors were encountered: