From 97255e7c4e31614b52f0584a75206fd621198ed4 Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Wed, 28 Aug 2013 15:42:41 +0200 Subject: SI-1909 Move tests from pos to run When we're in the neighbourhood of VerifyErrors, it's better to run the code. This change is leading up to a fix for SI-3832, which regressed with fix for SI-1909. --- test/files/pos/t1909.scala | 8 -------- test/files/pos/t1909b-pos.scala | 6 ------ test/files/run/t1909.check | 3 +++ test/files/run/t1909.scala | 12 ++++++++++++ test/files/run/t1909b.scala | 9 +++++++++ 5 files changed, 24 insertions(+), 14 deletions(-) delete mode 100644 test/files/pos/t1909.scala delete mode 100644 test/files/pos/t1909b-pos.scala create mode 100644 test/files/run/t1909.check create mode 100644 test/files/run/t1909.scala create mode 100644 test/files/run/t1909b.scala (limited to 'test') diff --git a/test/files/pos/t1909.scala b/test/files/pos/t1909.scala deleted file mode 100644 index 01213f62a3..0000000000 --- a/test/files/pos/t1909.scala +++ /dev/null @@ -1,8 +0,0 @@ -// Until #1909 is fixed, if this compiles the bytecode -// will trigger a VerifyError. This liftings and the one -// in 1909b.scala actually happen in two different places -// (uncurry and lambdalifter.) -class Ticket1909 { - def this(value: Int) = this() - def this(p: String) = this(try 0) -} diff --git a/test/files/pos/t1909b-pos.scala b/test/files/pos/t1909b-pos.scala deleted file mode 100644 index b914bee366..0000000000 --- a/test/files/pos/t1909b-pos.scala +++ /dev/null @@ -1,6 +0,0 @@ -class Ticket1909 (x: Int) { - def this() = this({ - def bar() = 5 - bar - }) -} \ No newline at end of file diff --git a/test/files/run/t1909.check b/test/files/run/t1909.check new file mode 100644 index 0000000000..7d25be60fd --- /dev/null +++ b/test/files/run/t1909.check @@ -0,0 +1,3 @@ +t1909.scala:7: warning: A try without a catch or finally is equivalent to putting its body in a block; no exceptions are handled. + def this(p: String) = this(try 0) + ^ diff --git a/test/files/run/t1909.scala b/test/files/run/t1909.scala new file mode 100644 index 0000000000..8ead7bacf2 --- /dev/null +++ b/test/files/run/t1909.scala @@ -0,0 +1,12 @@ +// Until #1909 is fixed, if this compiles the bytecode +// will trigger a VerifyError. This liftings and the one +// in 1909b.scala actually happen in two different places +// (uncurry and lambdalifter.) +class Ticket1909 { + def this(value: Int) = this() + def this(p: String) = this(try 0) +} + +object Test extends App { + new Ticket1909("") +} diff --git a/test/files/run/t1909b.scala b/test/files/run/t1909b.scala new file mode 100644 index 0000000000..89b2af57dc --- /dev/null +++ b/test/files/run/t1909b.scala @@ -0,0 +1,9 @@ +class Ticket1909 (x: Int) { + def this() = this({ + def bar() = 5 + bar + }) +} +object Test extends App { + new Ticket1909() +} -- 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 'test') 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 38a488ea17e4ed42b65df4095c2eac738a63f5c4 Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Wed, 28 Aug 2013 16:15:43 +0200 Subject: SI-7007 Test case shows we disallow premature `this` access Rather than the old behaviour, which compiled successfully but led us into the jaws of a LinkageError. Related to SI-6666. --- test/files/neg/t7007.check | 7 +++++++ test/files/neg/t7007.scala | 14 ++++++++++++++ 2 files changed, 21 insertions(+) create mode 100644 test/files/neg/t7007.check create mode 100644 test/files/neg/t7007.scala (limited to 'test') diff --git a/test/files/neg/t7007.check b/test/files/neg/t7007.check new file mode 100644 index 0000000000..e22ecb9e4e --- /dev/null +++ b/test/files/neg/t7007.check @@ -0,0 +1,7 @@ +t7007.scala:5: error: Implementation restriction: <$anon: A => B> requires premature access to class Crash. + def this(a: Seq[A]) = this(a.collect{ case b: B => b}, a.collect{ case b: B => b}) + ^ +t7007.scala:5: error: Implementation restriction: <$anon: A => B> requires premature access to class Crash. + def this(a: Seq[A]) = this(a.collect{ case b: B => b}, a.collect{ case b: B => b}) + ^ +two errors found diff --git a/test/files/neg/t7007.scala b/test/files/neg/t7007.scala new file mode 100644 index 0000000000..e41dccf550 --- /dev/null +++ b/test/files/neg/t7007.scala @@ -0,0 +1,14 @@ +class A +class B extends A + +class Crash(b1: Seq[B], b2: Seq[B]) { + def this(a: Seq[A]) = this(a.collect{ case b: B => b}, a.collect{ case b: B => b}) +} + +object Main extends App { + + // runtime exception with either constructor + val c1 = new Crash(Seq(new B, new B)) + val c2 = new Crash(Seq(new B), Seq(new B)) + +} -- 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 'test') 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