From c2dc34640c9fd06abda91266ca21160bb423bf41 Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Wed, 28 Aug 2013 15:46:04 +0200 Subject: SI-3832 Extract tracking of under-construction classes to a mixin In order to reduce the noise in OuterPathTransformer. --- .../scala/tools/nsc/transform/ExplicitOuter.scala | 17 ++--------------- src/reflect/scala/reflect/internal/Trees.scala | 19 +++++++++++++++++++ 2 files changed, 21 insertions(+), 15 deletions(-) (limited to 'src') diff --git a/src/compiler/scala/tools/nsc/transform/ExplicitOuter.scala b/src/compiler/scala/tools/nsc/transform/ExplicitOuter.scala index d6a6e027cb..8af95afdec 100644 --- a/src/compiler/scala/tools/nsc/transform/ExplicitOuter.scala +++ b/src/compiler/scala/tools/nsc/transform/ExplicitOuter.scala @@ -216,7 +216,7 @@ abstract class ExplicitOuter extends InfoTransform * values for outer parameters of constructors. * The class provides methods for referencing via outer. */ - abstract class OuterPathTransformer(unit: CompilationUnit) extends TypingTransformer(unit) { + abstract class OuterPathTransformer(unit: CompilationUnit) extends TypingTransformer(unit) with UnderConstructionTransformer { /** The directly enclosing outer parameter, if we are in a constructor */ protected var outerParam: Symbol = NoSymbol @@ -276,16 +276,6 @@ abstract class ExplicitOuter extends InfoTransform } - /** The stack of class symbols in which a call to this() or to the super - * constructor, or early definition is active - */ - protected def isUnderConstruction(clazz: Symbol) = selfOrSuperCalls contains clazz - protected val selfOrSuperCalls = mutable.Stack[Symbol]() - @inline protected def inSelfOrSuperCall[A](sym: Symbol)(a: => A) = { - selfOrSuperCalls push sym - try a finally selfOrSuperCalls.pop() - } - override def transform(tree: Tree): Tree = { val savedOuterParam = outerParam try { @@ -299,10 +289,7 @@ abstract class ExplicitOuter extends InfoTransform } case _ => } - if ((treeInfo isSelfOrSuperConstrCall tree) || (treeInfo isEarlyDef tree)) - inSelfOrSuperCall(currentOwner.owner)(super.transform(tree)) - else - super.transform(tree) + super.transform(tree) } finally outerParam = savedOuterParam } diff --git a/src/reflect/scala/reflect/internal/Trees.scala b/src/reflect/scala/reflect/internal/Trees.scala index 84818a6f42..e784d704d6 100644 --- a/src/reflect/scala/reflect/internal/Trees.scala +++ b/src/reflect/scala/reflect/internal/Trees.scala @@ -1617,6 +1617,25 @@ trait Trees extends api.Trees { self: SymbolTable => } } + /** Tracks the classes currently under construction during a transform */ + trait UnderConstructionTransformer extends Transformer { + import collection.mutable + + protected def isUnderConstruction(clazz: Symbol) = selfOrSuperCalls contains clazz + + /** The stack of class symbols in which a call to this() or to the super + * constructor, or early definition is active */ + private val selfOrSuperCalls = mutable.Stack[Symbol]() + + abstract override def transform(tree: Tree) = { + if ((treeInfo isSelfOrSuperConstrCall tree) || (treeInfo isEarlyDef tree)) { + selfOrSuperCalls push currentOwner.owner + try super.transform(tree) + finally selfOrSuperCalls.pop() + } else super.transform(tree) + } + } + def duplicateAndKeepPositions(tree: Tree) = new Duplicator(focusPositions = false) transform tree // ------ copiers ------------------------------------------- -- cgit v1.2.3 From f04257b8414745d8e13bee213e551ef01c839602 Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Wed, 28 Aug 2013 15:51:49 +0200 Subject: SI-3832 Don't lift methods in aux constructor trailing stats as STATIC SI-1909 modified LambdaLift to lift in auxiliary constructors methods as STATIC so they could be called before the self-constructor was called. That allowed for: class Foo (x: Int) { def this() = this( { def bar() = 5 bar }) } However, if the method is in a statement that trails the self constructor call, this is unnecessary and in fact incorrect as it robs the lifted method of `this`. This commit uses the machinery established in SI-6666 to limit the STATIC-ness of lifted methods to those used in arguments for self-constructor calls. This is used exclusively; the `isAuxillaryConstructor` check wasn't the right way to solve this, as was seen by the regression it caused in SI-3832. A new test case shows that we can statically lift methods in super-constructor calls, rather than just self-constructor calls. We also have to avoid statically lifting objects in these positions. For now, I just emit a dev warning that a VerifyError is in your future. With some more thought we could escalate that to a implementation restriction and emit an error. --- src/compiler/scala/tools/nsc/transform/LambdaLift.scala | 11 +++++++++-- test/files/neg/t1909-object.check | 4 ++++ test/files/neg/t1909-object.flags | 1 + test/files/neg/t1909-object.scala | 12 ++++++++++++ test/files/run/t1909c.scala | 9 +++++++++ test/files/run/t3832.scala | 17 +++++++++++++++++ 6 files changed, 52 insertions(+), 2 deletions(-) create mode 100644 test/files/neg/t1909-object.check create mode 100644 test/files/neg/t1909-object.flags create mode 100644 test/files/neg/t1909-object.scala create mode 100644 test/files/run/t1909c.scala create mode 100644 test/files/run/t3832.scala (limited to 'src') diff --git a/src/compiler/scala/tools/nsc/transform/LambdaLift.scala b/src/compiler/scala/tools/nsc/transform/LambdaLift.scala index 515fa66cfa..acef2a50d8 100644 --- a/src/compiler/scala/tools/nsc/transform/LambdaLift.scala +++ b/src/compiler/scala/tools/nsc/transform/LambdaLift.scala @@ -436,8 +436,15 @@ abstract class LambdaLift extends InfoTransform { private def liftDef(tree: Tree): Tree = { val sym = tree.symbol val oldOwner = sym.owner - if (sym.owner.isAuxiliaryConstructor && sym.isMethod) // # bug 1909 - sym setFlag STATIC + if (sym.isMethod && isUnderConstruction(sym.owner.owner)) { // # bug 1909 + if (sym.isModule) { // Yes, it can be a module and a method, see comments on `isModuleNotMethod`! + // TODO promote to an implementation restriction if we can reason that this *always* leads to VerifyError. + // See neg/t1909-object.scala + def msg = s"SI-1909 Unable to STATICally lift $sym, which is defined in the self- or super-constructor call of ${sym.owner.owner}. A VerifyError is likely." + devWarning(tree.pos, msg) + } else sym setFlag STATIC + } + sym.owner = sym.owner.enclClass if (sym.isClass) sym.owner = sym.owner.toInterface if (sym.isMethod) sym setFlag LIFTED diff --git a/test/files/neg/t1909-object.check b/test/files/neg/t1909-object.check new file mode 100644 index 0000000000..401c1f7ebf --- /dev/null +++ b/test/files/neg/t1909-object.check @@ -0,0 +1,4 @@ +t1909-object.scala:4: error: !!! SI-1909 Unable to STATICally lift object InnerTrouble$1, which is defined in the self- or super-constructor call of class Kaboom. A VerifyError is likely. + object InnerTrouble + ^ +one error found diff --git a/test/files/neg/t1909-object.flags b/test/files/neg/t1909-object.flags new file mode 100644 index 0000000000..eb8b40661b --- /dev/null +++ b/test/files/neg/t1909-object.flags @@ -0,0 +1 @@ +-Xdev -Xfatal-warnings \ No newline at end of file diff --git a/test/files/neg/t1909-object.scala b/test/files/neg/t1909-object.scala new file mode 100644 index 0000000000..d6011ba4a5 --- /dev/null +++ b/test/files/neg/t1909-object.scala @@ -0,0 +1,12 @@ +class Kaboom(a: Any) { + def this() = { + this({ + object InnerTrouble + InnerTrouble + }) + } +} + +object Test extends App { + new Kaboom() +} \ No newline at end of file diff --git a/test/files/run/t1909c.scala b/test/files/run/t1909c.scala new file mode 100644 index 0000000000..87c0eb08b5 --- /dev/null +++ b/test/files/run/t1909c.scala @@ -0,0 +1,9 @@ +class Base(a: Any) + +// java.lang.VerifyError: (class: Sub, method: signature: ()V) Expecting to find object/array on stack +// at Test$.(t1909c.scala) +class Sub() extends Base({ def bippy = 5; bippy }) + +object Test extends App { + new Sub() +} diff --git a/test/files/run/t3832.scala b/test/files/run/t3832.scala new file mode 100644 index 0000000000..ac44358bc7 --- /dev/null +++ b/test/files/run/t3832.scala @@ -0,0 +1,17 @@ +class t3832 { + def this(un: Int) = { + this() + def bippy = this + () + } + def this(un: Boolean) = { + this() + def boppy = () => this + () + } +} + +object Test extends App { + new t3832(0) + new t3832(true) +} -- cgit v1.2.3 From 27d73ee7a92d8dd10d4d0598a29d3a3657053995 Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Wed, 28 Aug 2013 17:32:40 +0200 Subject: SI-7223 More finesse in setting INCONSTRUCTOR It's a clunky flag used to determine very early on whether we're in the self-call, super-call or early-init section. In SI-6666 / fd6125428, the check was improved to consider nesting. But, that caused this regression, as Function's haven't been translated to classes yet, so our check for enclosing non-term owners failed wrongly flagged definitins body of a anonymous function as INCONSTRUCTOR. With this patch, we correctly flag: class C extends D { // INCONSTRUCTOR () => { !INCONSTRUCTOR } // INCONSTRUCTOR } --- src/compiler/scala/tools/nsc/typechecker/Namers.scala | 3 ++- test/files/run/t7223.check | 1 + test/files/run/t7223.scala | 11 +++++++++++ 3 files changed, 14 insertions(+), 1 deletion(-) create mode 100644 test/files/run/t7223.check create mode 100644 test/files/run/t7223.scala (limited to 'src') diff --git a/src/compiler/scala/tools/nsc/typechecker/Namers.scala b/src/compiler/scala/tools/nsc/typechecker/Namers.scala index 454f913412..9ac0b0835a 100644 --- a/src/compiler/scala/tools/nsc/typechecker/Namers.scala +++ b/src/compiler/scala/tools/nsc/typechecker/Namers.scala @@ -135,7 +135,8 @@ trait Namers extends MethodSynthesis { setPrivateWithin(tree, sym, tree.mods) def inConstructorFlag: Long = { - val termOwnedContexts: List[Context] = context.enclosingContextChain.takeWhile(_.owner.isTerm) + val termOwnedContexts: List[Context] = + context.enclosingContextChain.takeWhile(c => c.owner.isTerm && !c.owner.isAnonymousFunction) val constructorNonSuffix = termOwnedContexts exists (c => c.owner.isConstructor && !c.inConstructorSuffix) val earlyInit = termOwnedContexts exists (_.owner.isEarlyInitialized) if (constructorNonSuffix || earlyInit) INCONSTRUCTOR else 0L diff --git a/test/files/run/t7223.check b/test/files/run/t7223.check new file mode 100644 index 0000000000..573541ac97 --- /dev/null +++ b/test/files/run/t7223.check @@ -0,0 +1 @@ +0 diff --git a/test/files/run/t7223.scala b/test/files/run/t7223.scala new file mode 100644 index 0000000000..a707e957df --- /dev/null +++ b/test/files/run/t7223.scala @@ -0,0 +1,11 @@ +class D(val a: () => Int => () => Any) { + a()(0)() +} + +object Crash extends D(() => { + (x: Int) => {() => { new { println(x.toString) } }} +}) + +object Test extends App { + Crash +} -- cgit v1.2.3