From 90314b36d121755c52c00a57088623eba734a123 Mon Sep 17 00:00:00 2001 From: Adriaan Moors Date: Sat, 10 Sep 2016 11:22:39 +0200 Subject: SI-9918 object in trait mixed into package object --- test/files/pos/t9918/package.scala | 1 + test/files/pos/t9918/t9918.scala | 3 +++ 2 files changed, 4 insertions(+) create mode 100644 test/files/pos/t9918/package.scala create mode 100644 test/files/pos/t9918/t9918.scala (limited to 'test') diff --git a/test/files/pos/t9918/package.scala b/test/files/pos/t9918/package.scala new file mode 100644 index 0000000000..9bd8ac9a69 --- /dev/null +++ b/test/files/pos/t9918/package.scala @@ -0,0 +1 @@ +package object pkg extends T diff --git a/test/files/pos/t9918/t9918.scala b/test/files/pos/t9918/t9918.scala new file mode 100644 index 0000000000..ec9a146579 --- /dev/null +++ b/test/files/pos/t9918/t9918.scala @@ -0,0 +1,3 @@ +package pkg + +trait T { object O } -- cgit v1.2.3 From a919fd7fa1f3c39dc396e7758240354e6fb0e79b Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Mon, 12 Sep 2016 14:49:07 +1000 Subject: Avoid omitting constant typed vals in constructors Fix for regression in 2.12.0-RC1 compiling shapeless tests. They were given the same treatment as vals that are members of classes on the definition side without the requisite transformation of references to the val to fold the constant into references. This commit limits the transform to members of classes. Co-Authored-By: Miles Sabin --- src/compiler/scala/tools/nsc/transform/Fields.scala | 2 +- test/files/pos/shapeless-regression.scala | 16 ++++++++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) create mode 100644 test/files/pos/shapeless-regression.scala (limited to 'test') diff --git a/src/compiler/scala/tools/nsc/transform/Fields.scala b/src/compiler/scala/tools/nsc/transform/Fields.scala index a383b65192..0c7bc742d9 100644 --- a/src/compiler/scala/tools/nsc/transform/Fields.scala +++ b/src/compiler/scala/tools/nsc/transform/Fields.scala @@ -694,7 +694,7 @@ abstract class Fields extends InfoTransform with ast.TreeDSL with TypingTransfor // drop the val for (a) constant (pure & not-stored) and (b) not-stored (but still effectful) fields case ValDef(mods, _, _, rhs) if (rhs ne EmptyTree) && !excludedAccessorOrFieldByFlags(statSym) - && fieldMemoizationIn(statSym, currOwner).constantTyped => + && currOwner.isClass && fieldMemoizationIn(statSym, currOwner).constantTyped => EmptyThicket case ModuleDef(_, _, impl) => diff --git a/test/files/pos/shapeless-regression.scala b/test/files/pos/shapeless-regression.scala new file mode 100644 index 0000000000..f3a1ed1ba0 --- /dev/null +++ b/test/files/pos/shapeless-regression.scala @@ -0,0 +1,16 @@ +class W[T <: AnyRef](val t: T) { + val v: T {} = t +} + +object W { + def apply[T <: AnyRef](t: T) = new W[t.type](t) +} + +object RightAssoc { + def ra_:[T](t: T): Unit = () +} + +object Boom { + W("fooo").v ra_: RightAssoc +} + -- cgit v1.2.3 From a5bb6e00f051bf93fe7df4a02583eba478fa5ca1 Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Tue, 13 Sep 2016 22:45:10 +1000 Subject: SD-225 Use a "lzycompute" method for module initialization The monitors and module instantation were inliuned into the module accessor method in b2e0911. However, this seems to have had a detrimental impact on performance. This might be because the module accessors are now above the "always inline" HotSpot threshold of 35 bytes, or perhaps because they contain monitor-entry/exit and exception handlers. This commit returns to the the 2.11.8 appraoch of factoring the the second check of the doublecheck locking into a method. I've done this by declaring a nested method within the accessor; this will be lifted out to the class level by lambdalift. This represents a slight deviation from the implementation strategy used for lazy accessors, which create a symbol for the slowpath method in the info transform and generate the corresponding DefDef as a class member. I don't believe this deviation is particular worrisome, though. I have bootstrapped the compiler through this commit and found that the drastic regression in compiling the shapeless test suite is solved. --- src/compiler/scala/tools/nsc/transform/Fields.scala | 6 ++++-- test/files/run/delambdafy_t6028.check | 5 +++-- test/files/run/t6028.check | 5 +++-- .../scala/tools/nsc/backend/jvm/opt/InlineWarningTest.scala | 4 +++- .../scala/tools/nsc/backend/jvm/opt/ScalaInlineInfoTest.scala | 9 +++++++-- 5 files changed, 20 insertions(+), 9 deletions(-) (limited to 'test') diff --git a/src/compiler/scala/tools/nsc/transform/Fields.scala b/src/compiler/scala/tools/nsc/transform/Fields.scala index a383b65192..45741eb391 100644 --- a/src/compiler/scala/tools/nsc/transform/Fields.scala +++ b/src/compiler/scala/tools/nsc/transform/Fields.scala @@ -22,7 +22,7 @@ import symtab.Flags._ * in the first (closest in the subclassing lattice) subclass (not a trait) of a trait. * * For lazy vals and modules, we emit accessors that using double-checked locking (DCL) to balance thread safety - * and performance. A lazy val gets a compute method for the DCL's slow path, for a module it's all done in the accessor. + * and performance. For both lazy vals and modules, the a compute method contains the DCL's slow path. * * Local lazy vals do not receive bitmaps, but use a Lazy*Holder that has the volatile init bit and the computed value. * See `mkLazyLocalDef`. @@ -236,7 +236,9 @@ abstract class Fields extends InfoTransform with ast.TreeDSL with TypingTransfor * * TODO: optimize using local variable? */ - Block(If(needsInit, gen.mkSynchronized(monitorHolder)(If(needsInit, init, EmptyTree)), EmptyTree) :: Nil, moduleVarRef) + val computeName = nme.newLazyValSlowComputeName(module.name) + val computeMethod = DefDef(NoMods, computeName, Nil, ListOfNil, TypeTree(UnitTpe), gen.mkSynchronized(monitorHolder)(If(needsInit, init, EmptyTree))) + Block(computeMethod :: If(needsInit, Apply(Ident(computeName), Nil), EmptyTree) :: Nil, moduleVarRef) } // NoSymbol for lazy accessor sym with unit result type diff --git a/test/files/run/delambdafy_t6028.check b/test/files/run/delambdafy_t6028.check index eaba70ee1a..6a15b3b003 100644 --- a/test/files/run/delambdafy_t6028.check +++ b/test/files/run/delambdafy_t6028.check @@ -42,10 +42,11 @@ package { def $outer(): T = MethodLocalObject$2.this.$outer; def $outer(): T = MethodLocalObject$2.this.$outer }; + final private[this] def MethodLocalObject$lzycompute$1(barParam$1: String, MethodLocalObject$module$1: runtime.VolatileObjectRef): Unit = T.this.synchronized[Unit](if (MethodLocalObject$module$1.elem.$asInstanceOf[T#MethodLocalObject$2.type]().eq(null)) + MethodLocalObject$module$1.elem = new T#MethodLocalObject$2.type(T.this, barParam$1)); final private[this] def MethodLocalObject$1(barParam$1: String, MethodLocalObject$module$1: runtime.VolatileObjectRef): T#MethodLocalObject$2.type = { if (MethodLocalObject$module$1.elem.$asInstanceOf[T#MethodLocalObject$2.type]().eq(null)) - T.this.synchronized[Unit](if (MethodLocalObject$module$1.elem.$asInstanceOf[T#MethodLocalObject$2.type]().eq(null)) - MethodLocalObject$module$1.elem = new T#MethodLocalObject$2.type(T.this, barParam$1)); + T.this.MethodLocalObject$lzycompute$1(barParam$1, MethodLocalObject$module$1); MethodLocalObject$module$1.elem.$asInstanceOf[T#MethodLocalObject$2.type]() }; final private[this] def $anonfun$tryy$1(tryyParam$1: String, tryyLocal$1: runtime.ObjectRef): Unit = try { diff --git a/test/files/run/t6028.check b/test/files/run/t6028.check index d6cc452bbf..80f8698ecf 100644 --- a/test/files/run/t6028.check +++ b/test/files/run/t6028.check @@ -54,10 +54,11 @@ package { def $outer(): T = MethodLocalObject$2.this.$outer; def $outer(): T = MethodLocalObject$2.this.$outer }; + final private[this] def MethodLocalObject$lzycompute$1(barParam$1: Int, MethodLocalObject$module$1: runtime.VolatileObjectRef): Unit = T.this.synchronized[Unit](if (MethodLocalObject$module$1.elem.$asInstanceOf[T#MethodLocalObject$2.type]().eq(null)) + MethodLocalObject$module$1.elem = new T#MethodLocalObject$2.type(T.this, barParam$1)); final private[this] def MethodLocalObject$1(barParam$1: Int, MethodLocalObject$module$1: runtime.VolatileObjectRef): T#MethodLocalObject$2.type = { if (MethodLocalObject$module$1.elem.$asInstanceOf[T#MethodLocalObject$2.type]().eq(null)) - T.this.synchronized[Unit](if (MethodLocalObject$module$1.elem.$asInstanceOf[T#MethodLocalObject$2.type]().eq(null)) - MethodLocalObject$module$1.elem = new T#MethodLocalObject$2.type(T.this, barParam$1)); + T.this.MethodLocalObject$lzycompute$1(barParam$1, MethodLocalObject$module$1); MethodLocalObject$module$1.elem.$asInstanceOf[T#MethodLocalObject$2.type]() }; @SerialVersionUID(value = 0) final class $anonfun$tryy$1 extends scala.runtime.AbstractFunction0$mcV$sp with Serializable { diff --git a/test/junit/scala/tools/nsc/backend/jvm/opt/InlineWarningTest.scala b/test/junit/scala/tools/nsc/backend/jvm/opt/InlineWarningTest.scala index 85b44d9fa0..95b47f7d04 100644 --- a/test/junit/scala/tools/nsc/backend/jvm/opt/InlineWarningTest.scala +++ b/test/junit/scala/tools/nsc/backend/jvm/opt/InlineWarningTest.scala @@ -198,7 +198,9 @@ class InlineWarningTest extends BytecodeTesting { |Note that class A is defined in a Java source (mixed compilation), no bytecode is available.""".stripMargin ) var c = 0 - compileClasses(sCode, javaCode = List((jCode, "A.java")), allowMessage = i => { c += 1; warns.exists(i.msg.contains)}) + compileClasses(sCode, javaCode = List((jCode, "A.java")), allowMessage = i => { c += 1; + warns.exists(i.msg.contains) + }) assert(c == 2) } } diff --git a/test/junit/scala/tools/nsc/backend/jvm/opt/ScalaInlineInfoTest.scala b/test/junit/scala/tools/nsc/backend/jvm/opt/ScalaInlineInfoTest.scala index eae5385147..8861577366 100644 --- a/test/junit/scala/tools/nsc/backend/jvm/opt/ScalaInlineInfoTest.scala +++ b/test/junit/scala/tools/nsc/backend/jvm/opt/ScalaInlineInfoTest.scala @@ -107,7 +107,9 @@ class ScalaInlineInfoTest extends BytecodeTesting { ("x5$(LT;)I", MethodInlineInfo(true ,false,false)), ("L$1(Lscala/runtime/VolatileObjectRef;)LT$L$2$;", MethodInlineInfo(true, false,false)), ("nest$1()I", MethodInlineInfo(true, false,false)), - ("$init$(LT;)V", MethodInlineInfo(true,false,false))), + ("$init$(LT;)V", MethodInlineInfo(true,false,false)), + ("L$lzycompute$1(Lscala/runtime/VolatileObjectRef;)V", MethodInlineInfo(true,false,false)) + ), None // warning ) @@ -128,7 +130,9 @@ class ScalaInlineInfoTest extends BytecodeTesting { "x4()I" -> MethodInlineInfo(false,false,false), // "x5()I" -> MethodInlineInfo(true ,false,false), -- there is no x5 in the class as it's implemented fully in the interface "T$$super$toString()Ljava/lang/String;" -> MethodInlineInfo(true ,false,false), - "()V" -> MethodInlineInfo(false,false,false)), + "()V" -> MethodInlineInfo(false,false,false), + "O$lzycompute$1()V" -> MethodInlineInfo(true,false,false) + ), None) assert(infoC == expectC, mapDiff(expectC.methodInfos, infoC.methodInfos) + infoC) @@ -179,6 +183,7 @@ class ScalaInlineInfoTest extends BytecodeTesting { val infoC = inlineInfo(c) val expected = Map( "()V" -> MethodInlineInfo(false,false,false), + "O$lzycompute$1()V" -> MethodInlineInfo(true,false,false), "O()LC$O$;" -> MethodInlineInfo(true,false,false)) assert(infoC.methodInfos == expected, mapDiff(infoC.methodInfos, expected)) assertSameMethods(c, expected.keySet) -- cgit v1.2.3 From e994c1c0becddc0d91fd4428f0d673bfac8941a3 Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Mon, 26 Sep 2016 09:47:49 +1000 Subject: SD-233 synchronized blocks are JIT-friendly again GenBCode, the new backend in Scala 2.12, subtly changed the way that synchronized blocks are emitted. It used `java/lang/Throwable` as an explicitly named exception type, rather than implying the same by omitting this in bytecode. This appears to confuse HotSpot JIT, which reports a error parsing the bytecode into its IR which leaves the enclosing method stuck in interpreted mode. This commit passes a `null` descriptor to restore the old pattern (the same one used by javac.) I've checked that the JIT warnings are gone and that the method can be compiled again. --- src/compiler/scala/tools/nsc/backend/jvm/BCodeSyncAndTry.scala | 4 +++- test/junit/scala/tools/nsc/backend/jvm/BytecodeTest.scala | 8 ++++++++ 2 files changed, 11 insertions(+), 1 deletion(-) (limited to 'test') diff --git a/src/compiler/scala/tools/nsc/backend/jvm/BCodeSyncAndTry.scala b/src/compiler/scala/tools/nsc/backend/jvm/BCodeSyncAndTry.scala index 3e53419573..466793010f 100644 --- a/src/compiler/scala/tools/nsc/backend/jvm/BCodeSyncAndTry.scala +++ b/src/compiler/scala/tools/nsc/backend/jvm/BCodeSyncAndTry.scala @@ -73,9 +73,11 @@ abstract class BCodeSyncAndTry extends BCodeBodyBuilder { /* ------ (4) exception-handler version of monitor-exit code. * Reached upon abrupt termination of (2). * Protected by whatever protects the whole synchronized expression. + * null => "any" exception in bytecode, like we emit for finally. + * Important not to use j/l/Throwable which dooms the method to a life of interpretation! (SD-233) * ------ */ - protect(startProtected, endProtected, currProgramPoint(), jlThrowableRef) + protect(startProtected, endProtected, currProgramPoint(), null) locals.load(monitor) emit(asm.Opcodes.MONITOREXIT) emit(asm.Opcodes.ATHROW) diff --git a/test/junit/scala/tools/nsc/backend/jvm/BytecodeTest.scala b/test/junit/scala/tools/nsc/backend/jvm/BytecodeTest.scala index b09a41969e..00b6d1cc42 100644 --- a/test/junit/scala/tools/nsc/backend/jvm/BytecodeTest.scala +++ b/test/junit/scala/tools/nsc/backend/jvm/BytecodeTest.scala @@ -187,4 +187,12 @@ class BytecodeTest extends BytecodeTesting { List(Label(0), LineNumber(2, Label(0)), VarOp(ALOAD, 0), Invoke(INVOKESPECIAL, "T", "t", "()V", true), Op(RETURN), Label(4)) ) } + + @Test + def sd233(): Unit = { + val code = "def f = { println(1); synchronized(println(2)) }" + val m = compileMethod(code) + val List(ExceptionHandler(_, _, _, desc)) = m.handlers + assert(desc == None, desc) + } } -- cgit v1.2.3 From 9a39e0c283ac60edabb8dba9ad8513199112882a Mon Sep 17 00:00:00 2001 From: Adriaan Moors Date: Thu, 8 Sep 2016 18:17:45 +0200 Subject: Avoid mismatched symbols in fields phase The info of the var that stores a trait's lazy val's computed value is expressed in terms of symbols that exist before the fields phase. When we're implementing the lazy val in a subclass of that trait, we now see symbols created by the fields phase, which results in mismatches between the types of the lhs and rhs in the assignment of `lazyVar = super.lazyImpl`. So, type check the super-call to the trait's lazy accessor before our own phase. If the lazy var's info depends on a val that is now implemented by an accessor synthesize by our info transformer, we'll get a mismatch when assigning `rhs` to `lazyVarOf(getter)`, unless we also run before our own phase (like when we were creating the info for the lazy var). This was revealed by Hanns Holger Rutz's efforts in compiling scala-refactoring's test suite (reported on scala-internals). Fixes scala/scala-dev#219 --- src/compiler/scala/tools/nsc/transform/Fields.scala | 17 +++++++++++++++-- test/files/pos/sd219.scala | 11 +++++++++++ 2 files changed, 26 insertions(+), 2 deletions(-) create mode 100644 test/files/pos/sd219.scala (limited to 'test') diff --git a/src/compiler/scala/tools/nsc/transform/Fields.scala b/src/compiler/scala/tools/nsc/transform/Fields.scala index 894d0a1701..44ea52f801 100644 --- a/src/compiler/scala/tools/nsc/transform/Fields.scala +++ b/src/compiler/scala/tools/nsc/transform/Fields.scala @@ -633,8 +633,21 @@ abstract class Fields extends InfoTransform with ast.TreeDSL with TypingTransfor val synthAccessorInClass = new SynthLazyAccessorsIn(clazz) def superLazy(getter: Symbol): List[ValOrDefDef] = { assert(!clazz.isTrait) - // this contortion was the only way I can get the super select to be type checked correctly.. TODO: why does SelectSuper not work? - val rhs = Apply(Select(Super(This(clazz), tpnme.EMPTY), getter.name), Nil) + // this contortion was the only way I can get the super select to be type checked correctly.. + // TODO: why does SelectSuper not work? + val selectSuper = Select(Super(This(clazz), tpnme.EMPTY), getter.name) + + // scala/scala-dev#219 + // Type check the super-call to the trait's lazy accessor before our own phase, + // so that we don't see other accessor symbols we mix into the class. + // The lazy var's info will not refer to symbols created during our info transformer, + // so if its type depends on a val that is now implemented after the info transformer, + // we'll get a mismatch when assigning `rhs` to `lazyVarOf(getter)`, + // unless we also run before our own phase (like when we were creating the info for the lazy var). + // + // TODO: are there other spots where we may get a mismatch like this? + val rhs = exitingUncurry(typedPos(getter.pos.focus)(Apply(selectSuper, Nil))) + explodeThicket(synthAccessorInClass.expandLazyClassMember(lazyVarOf(getter), getter, rhs, Map.empty)).asInstanceOf[List[ValOrDefDef]] } diff --git a/test/files/pos/sd219.scala b/test/files/pos/sd219.scala new file mode 100644 index 0000000000..3c3f4962f0 --- /dev/null +++ b/test/files/pos/sd219.scala @@ -0,0 +1,11 @@ +class Global { class Name } + +trait CommonPrintUtils { + val global: Global + + lazy val precedence: global.Name => Int = ??? +} + +trait CompilerProvider { val global: Global = ??? } + +class AbstractPrinter extends CommonPrintUtils with CompilerProvider \ No newline at end of file -- cgit v1.2.3 From 5f64ee5ad1148563409c4e7cfbdd51577589d3e1 Mon Sep 17 00:00:00 2001 From: Adriaan Moors Date: Fri, 9 Sep 2016 09:20:39 +0200 Subject: Cast more pro-actively in synthetic accessor trees. Also narrow scope of afterOwnPhase. --- .../scala/tools/nsc/transform/Fields.scala | 113 +++++++++++---------- test/files/run/delambdafy_t6028.check | 2 +- test/files/run/t6028.check | 2 +- 3 files changed, 64 insertions(+), 53 deletions(-) (limited to 'test') diff --git a/src/compiler/scala/tools/nsc/transform/Fields.scala b/src/compiler/scala/tools/nsc/transform/Fields.scala index 44ea52f801..f75e6f5efa 100644 --- a/src/compiler/scala/tools/nsc/transform/Fields.scala +++ b/src/compiler/scala/tools/nsc/transform/Fields.scala @@ -207,9 +207,8 @@ abstract class Fields extends InfoTransform with ast.TreeDSL with TypingTransfor moduleVar } - private def moduleInit(module: Symbol) = { + private def moduleInit(module: Symbol, moduleVar: Symbol) = { // println(s"moduleInit for $module in ${module.ownerChain} --> ${moduleVarOf.get(module)}") - val moduleVar = moduleOrLazyVarOf(module) def moduleVarRef = gen.mkAttributedRef(moduleVar) // for local modules, we synchronize on the owner of the method that owns the module @@ -238,7 +237,8 @@ abstract class Fields extends InfoTransform with ast.TreeDSL with TypingTransfor */ val computeName = nme.newLazyValSlowComputeName(module.name) val computeMethod = DefDef(NoMods, computeName, Nil, ListOfNil, TypeTree(UnitTpe), gen.mkSynchronized(monitorHolder)(If(needsInit, init, EmptyTree))) - Block(computeMethod :: If(needsInit, Apply(Ident(computeName), Nil), EmptyTree) :: Nil, moduleVarRef) + Block(computeMethod :: If(needsInit, Apply(Ident(computeName), Nil), EmptyTree) :: Nil, + gen.mkCast(moduleVarRef, module.info.resultType)) } // NoSymbol for lazy accessor sym with unit result type @@ -590,75 +590,81 @@ abstract class Fields extends InfoTransform with ast.TreeDSL with TypingTransfor } // synth trees for accessors/fields and trait setters when they are mixed into a class - def fieldsAndAccessors(clazz: Symbol): List[ValOrDefDef] = { - def fieldAccess(accessor: Symbol): List[Tree] = { - val fieldName = accessor.localName - val field = clazz.info.decl(fieldName) - // The `None` result denotes an error, but it's refchecks' job to report it (this fallback is for robustness). - // This is the result of overriding a val with a def, so that no field is found in the subclass. - if (field.exists) List(Select(This(clazz), field)) - else Nil - } - - def getterBody(getter: Symbol): List[Tree] = { + def fieldsAndAccessors(clazz: Symbol): List[Tree] = { + // scala/scala-dev#219 + // Cast to avoid spurious mismatch in paths containing trait vals that have + // not been rebound to accessors in the subclass we're in now. + // For example, for a lazy val mixed into a class, the lazy var's info + // will not refer to symbols created during our info transformer, + // so if its type depends on a val that is now implemented after the info transformer, + // we'll get a mismatch when assigning `rhs` to `lazyVarOf(getter)`. + // TODO: could we rebind more aggressively? consider overriding in type equality? + def cast(tree: Tree, pt: Type) = gen.mkAsInstanceOf(tree, pt) + + // Could be NoSymbol, which denotes an error, but it's refchecks' job to report it (this fallback is for robustness). + // This is the result of overriding a val with a def, so that no field is found in the subclass. + def fieldAccess(accessor: Symbol): Symbol = + afterOwnPhase { clazz.info.decl(accessor.localName) } + + def getterBody(getter: Symbol): Tree = // accessor created by newMatchingModuleAccessor for a static module that does need an accessor // (because there's a matching member in a super class) - if (getter.asTerm.referenced.isModule) { - List(gen.mkAttributedRef(clazz.thisType, getter.asTerm.referenced)) - } else { + if (getter.asTerm.referenced.isModule) + mkAccessor(getter)(cast(Select(This(clazz), getter.asTerm.referenced), getter.info.resultType)) + else { val fieldMemoization = fieldMemoizationIn(getter, clazz) - if (fieldMemoization.constantTyped) List(gen.mkAttributedQualifier(fieldMemoization.tp)) // TODO: drop when we no longer care about producing identical bytecode - else fieldAccess(getter) + // TODO: drop getter for constant? (when we no longer care about producing identical bytecode?) + if (fieldMemoization.constantTyped) mkAccessor(getter)(gen.mkAttributedQualifier(fieldMemoization.tp)) + else fieldAccess(getter) match { + case NoSymbol => EmptyTree + case fieldSel => mkAccessor(getter)(cast(Select(This(clazz), fieldSel), getter.info.resultType)) + } } - } // println(s"accessorsAndFieldsNeedingTrees for $templateSym: $accessorsAndFieldsNeedingTrees") - def setterBody(setter: Symbol): List[Tree] = { + def setterBody(setter: Symbol): Tree = // trait setter in trait - if (clazz.isTrait) List(EmptyTree) + if (clazz.isTrait) mkAccessor(setter)(EmptyTree) // trait setter for overridden val in class - else if (checkAndClearOverriddenTraitSetter(setter)) List(mkTypedUnit(setter.pos)) + else if (checkAndClearOverriddenTraitSetter(setter)) mkAccessor(setter)(mkTypedUnit(setter.pos)) // trait val/var setter mixed into class - else fieldAccess(setter) map (fieldSel => Assign(fieldSel, Ident(setter.firstParam))) - } + else fieldAccess(setter) match { + case NoSymbol => EmptyTree + case fieldSel => afterOwnPhase { // the assign only type checks after our phase (assignment to val) + mkAccessor(setter)(Assign(Select(This(clazz), fieldSel), cast(Ident(setter.firstParam), fieldSel.info))) + } + } - def moduleAccessorBody(module: Symbol): List[Tree] = List( + def moduleAccessorBody(module: Symbol): Tree = // added during synthFieldsAndAccessors using newModuleAccessor // a module defined in a trait by definition can't be static (it's a member of the trait and thus gets a new instance for every outer instance) - if (clazz.isTrait) EmptyTree + if (clazz.isTrait) mkAccessor(module)(EmptyTree) // symbol created by newModuleAccessor for a (non-trait) class - else moduleInit(module) - ) + else { + mkAccessor(module)(moduleInit(module, moduleOrLazyVarOf(module))) + } val synthAccessorInClass = new SynthLazyAccessorsIn(clazz) - def superLazy(getter: Symbol): List[ValOrDefDef] = { + def superLazy(getter: Symbol): Tree = { assert(!clazz.isTrait) // this contortion was the only way I can get the super select to be type checked correctly.. // TODO: why does SelectSuper not work? val selectSuper = Select(Super(This(clazz), tpnme.EMPTY), getter.name) - // scala/scala-dev#219 - // Type check the super-call to the trait's lazy accessor before our own phase, - // so that we don't see other accessor symbols we mix into the class. - // The lazy var's info will not refer to symbols created during our info transformer, - // so if its type depends on a val that is now implemented after the info transformer, - // we'll get a mismatch when assigning `rhs` to `lazyVarOf(getter)`, - // unless we also run before our own phase (like when we were creating the info for the lazy var). - // - // TODO: are there other spots where we may get a mismatch like this? - val rhs = exitingUncurry(typedPos(getter.pos.focus)(Apply(selectSuper, Nil))) - - explodeThicket(synthAccessorInClass.expandLazyClassMember(lazyVarOf(getter), getter, rhs, Map.empty)).asInstanceOf[List[ValOrDefDef]] + val lazyVar = lazyVarOf(getter) + val rhs = cast(Apply(selectSuper, Nil), lazyVar.info) + + synthAccessorInClass.expandLazyClassMember(lazyVar, getter, rhs, Map.empty) } - clazz.info.decls.toList.filter(checkAndClearNeedsTrees) flatMap { - case module if module hasAllFlags (MODULE | METHOD) => moduleAccessorBody(module) map mkAccessor(module) + (afterOwnPhase { clazz.info.decls } toList) filter checkAndClearNeedsTrees map { + case module if module hasAllFlags (MODULE | METHOD) => moduleAccessorBody(module) case getter if getter hasAllFlags (LAZY | METHOD) => superLazy(getter) - case setter if setter.isSetter => setterBody(setter) map mkAccessor(setter) - case getter if getter.hasFlag(ACCESSOR) => getterBody(getter) map mkAccessor(getter) - case field if !(field hasFlag METHOD) => Some(mkTypedValDef(field)) // vals/vars and module vars (cannot have flags PACKAGE | JAVA since those never receive NEEDS_TREES) - case _ => None - } + case setter if setter.isSetter => setterBody(setter) + case getter if getter.hasFlag(ACCESSOR) => getterBody(getter) + case field if !(field hasFlag METHOD) => mkTypedValDef(field) // vals/vars and module vars (cannot have flags PACKAGE | JAVA since those never receive NEEDS_TREES) + case _ => EmptyTree + } filterNot (_ == EmptyTree) // there will likely be many EmptyTrees, but perhaps no thicket blocks that need expanding } def rhsAtOwner(stat: ValOrDefDef, newOwner: Symbol): Tree = @@ -718,7 +724,7 @@ abstract class Fields extends InfoTransform with ast.TreeDSL with TypingTransfor if (currOwner.isClass) cd else { // local module -- symbols cannot be generated by info transformer, so do it all here val moduleVar = newModuleVarSymbol(currOwner, statSym, statSym.info.resultType) - Thicket(cd :: mkTypedValDef(moduleVar) :: mkAccessor(statSym)(moduleInit(statSym)) :: Nil) + Thicket(cd :: mkTypedValDef(moduleVar) :: mkAccessor(statSym)(moduleInit(statSym, moduleVar)) :: Nil) } case tree => @@ -737,7 +743,12 @@ abstract class Fields extends InfoTransform with ast.TreeDSL with TypingTransfor override def transformStats(stats: List[Tree], exprOwner: Symbol): List[Tree] = { val addedStats = if (!currentOwner.isClass || currentOwner.isPackageClass) Nil - else afterOwnPhase { fieldsAndAccessors(currentOwner) } + else { + val thickets = fieldsAndAccessors(currentOwner) + if (thickets exists mustExplodeThicket) + thickets flatMap explodeThicket + else thickets + } val inRealClass = currentOwner.isClass && !(currentOwner.isPackageClass || currentOwner.isTrait) if (inRealClass) diff --git a/test/files/run/delambdafy_t6028.check b/test/files/run/delambdafy_t6028.check index 6a15b3b003..7b319c92dd 100644 --- a/test/files/run/delambdafy_t6028.check +++ b/test/files/run/delambdafy_t6028.check @@ -47,7 +47,7 @@ package { final private[this] def MethodLocalObject$1(barParam$1: String, MethodLocalObject$module$1: runtime.VolatileObjectRef): T#MethodLocalObject$2.type = { if (MethodLocalObject$module$1.elem.$asInstanceOf[T#MethodLocalObject$2.type]().eq(null)) T.this.MethodLocalObject$lzycompute$1(barParam$1, MethodLocalObject$module$1); - MethodLocalObject$module$1.elem.$asInstanceOf[T#MethodLocalObject$2.type]() + (MethodLocalObject$module$1.elem.$asInstanceOf[T#MethodLocalObject$2.type](): T#MethodLocalObject$2.type) }; final private[this] def $anonfun$tryy$1(tryyParam$1: String, tryyLocal$1: runtime.ObjectRef): Unit = try { tryyLocal$1.elem = tryyParam$1 diff --git a/test/files/run/t6028.check b/test/files/run/t6028.check index 80f8698ecf..903ea3b753 100644 --- a/test/files/run/t6028.check +++ b/test/files/run/t6028.check @@ -59,7 +59,7 @@ package { final private[this] def MethodLocalObject$1(barParam$1: Int, MethodLocalObject$module$1: runtime.VolatileObjectRef): T#MethodLocalObject$2.type = { if (MethodLocalObject$module$1.elem.$asInstanceOf[T#MethodLocalObject$2.type]().eq(null)) T.this.MethodLocalObject$lzycompute$1(barParam$1, MethodLocalObject$module$1); - MethodLocalObject$module$1.elem.$asInstanceOf[T#MethodLocalObject$2.type]() + (MethodLocalObject$module$1.elem.$asInstanceOf[T#MethodLocalObject$2.type](): T#MethodLocalObject$2.type) }; @SerialVersionUID(value = 0) final class $anonfun$tryy$1 extends scala.runtime.AbstractFunction0$mcV$sp with Serializable { def ($outer: T, tryyParam$1: Int, tryyLocal$1: runtime.IntRef): <$anon: Function0> = { -- cgit v1.2.3 From e3e1e30c08d8bb532ac1d36d191fc8d4dbab0eb9 Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Tue, 13 Sep 2016 13:43:43 +1000 Subject: SI-9920 Avoid linkage errors with captured local objects + self types An outer parameter of a nested class is typed with the self type of the enclosing class: ``` class C; trait T { _: C => def x = 42; class D { x } } ``` leads to: ``` class D extends Object { def ($outer: C): T.this.D = { D.super.(); () }; D.this.$outer().$asInstanceOf[T]().x(); ``` Note that a cast is inserted before the call to `x`. If we modify that a little, to instead capture a local module: ``` class C; trait T { _: C => def y { object O; class D { O } } } ``` Scala 2.11 used to generate (after lambdalift): ``` class D$1 extends Object { def ($outer: C, O$module$1: runtime.VolatileObjectRef): C#D$1 = { D$1.super.(); () }; D$1.this.$outer().O$1(O$module$1); ``` That isn't type correct, `D$1.this.$outer() : C` does not have a member `O$1`. However, the old trait encoding would rewrite this in mixin to: ``` T$class.O$1($outer, O$module$1); ``` Trait implementation methods also used to accept the self type: ``` trait T$class { final def O$1($this: C, O$module$1: runtime.VolatileObjectRef): T$O$2.type } ``` So the problem was hidden. This commit changes replaces manual typecheckin of the selection in LambdaLift with a use of the local (erasure) typer, which will add casts as needed. For `run/t9220.scala`, this changes the post LambdaLift AST as follows: ``` class C1$1 extends Object { def ($outer: C0, Local$module$1: runtime.VolatileObjectRef): T#C1$1 = { C1$1.super.(); () }; - C1$1.this.$outer.Local$1(Local$module$1); + C1$1.this.$outer.$asInstanceOf[T]().Local$1(Local$module$1); private[this] val $outer: C0 = _; def $outer(): C0 = C1$1.this.$outer } ``` --- .../scala/tools/nsc/transform/ExplicitOuter.scala | 2 +- .../scala/tools/nsc/transform/LambdaLift.scala | 9 ++++++++- test/files/pos/t9920.scala | 6 ++++++ test/files/run/t9920.scala | 17 +++++++++++++++++ test/files/run/t9920b.scala | 17 +++++++++++++++++ test/files/run/t9920c.scala | 21 +++++++++++++++++++++ test/files/run/t9920d.scala | 14 ++++++++++++++ 7 files changed, 84 insertions(+), 2 deletions(-) create mode 100644 test/files/pos/t9920.scala create mode 100644 test/files/run/t9920.scala create mode 100644 test/files/run/t9920b.scala create mode 100644 test/files/run/t9920c.scala create mode 100644 test/files/run/t9920d.scala (limited to 'test') diff --git a/src/compiler/scala/tools/nsc/transform/ExplicitOuter.scala b/src/compiler/scala/tools/nsc/transform/ExplicitOuter.scala index f3d5ceb0f0..7d50c12852 100644 --- a/src/compiler/scala/tools/nsc/transform/ExplicitOuter.scala +++ b/src/compiler/scala/tools/nsc/transform/ExplicitOuter.scala @@ -8,7 +8,7 @@ package tools.nsc package transform import symtab._ -import Flags.{ CASE => _, _ } +import Flags.{CASE => _, _} import scala.collection.mutable.ListBuffer /** This class ... diff --git a/src/compiler/scala/tools/nsc/transform/LambdaLift.scala b/src/compiler/scala/tools/nsc/transform/LambdaLift.scala index 74e6c58388..10d9c5627b 100644 --- a/src/compiler/scala/tools/nsc/transform/LambdaLift.scala +++ b/src/compiler/scala/tools/nsc/transform/LambdaLift.scala @@ -319,7 +319,14 @@ abstract class LambdaLift extends InfoTransform { else if (clazz.isStaticOwner) gen.mkAttributedQualifier(clazz.thisType) else outerValue match { case EmptyTree => prematureSelfReference() - case o => outerPath(o, currentClass.outerClass, clazz) + case o => + val path = outerPath(o, currentClass.outerClass, clazz) + if (path.tpe <:< clazz.tpeHK) path + else { + // SI-9920 The outer accessor might have an erased type of the self type of a trait, + // rather than the trait itself. Add a cast if necessary. + gen.mkAttributedCast(path, clazz.tpeHK) + } } } diff --git a/test/files/pos/t9920.scala b/test/files/pos/t9920.scala new file mode 100644 index 0000000000..8612618cc4 --- /dev/null +++ b/test/files/pos/t9920.scala @@ -0,0 +1,6 @@ +object Test { + def o = { + def i: Int = { i; 0 } + i + } +} diff --git a/test/files/run/t9920.scala b/test/files/run/t9920.scala new file mode 100644 index 0000000000..5dc32e99b7 --- /dev/null +++ b/test/files/run/t9920.scala @@ -0,0 +1,17 @@ +class C0 +trait T { self: C0 => + def test = { + object Local + + class C1 { + Local + } + new C1() + } +} + +object Test extends C0 with T { + def main(args: Array[String]): Unit = { + test + } +} diff --git a/test/files/run/t9920b.scala b/test/files/run/t9920b.scala new file mode 100644 index 0000000000..fab196b669 --- /dev/null +++ b/test/files/run/t9920b.scala @@ -0,0 +1,17 @@ +class C0 +trait T { + def test = { + object Local + + class C1 { + Local + } + new C1() + } +} + +object Test extends C0 with T { + def main(args: Array[String]): Unit = { + test + } +} diff --git a/test/files/run/t9920c.scala b/test/files/run/t9920c.scala new file mode 100644 index 0000000000..9541dc650a --- /dev/null +++ b/test/files/run/t9920c.scala @@ -0,0 +1,21 @@ +class C0 +trait T { self: C0 => + def test = { + object Local + + class C2 { + class C1 { + Local + } + T.this.toString + new C1 + } + new C2() + } +} + +object Test extends C0 with T { + def main(args: Array[String]): Unit = { + test + } +} diff --git a/test/files/run/t9920d.scala b/test/files/run/t9920d.scala new file mode 100644 index 0000000000..debc99e199 --- /dev/null +++ b/test/files/run/t9920d.scala @@ -0,0 +1,14 @@ +class C { object O } +trait T { _: C => + def foo { + class D { O } + new D + } +} + + +object Test extends C with T { + def main(args: Array[String]): Unit = { + foo + } +} -- cgit v1.2.3 From 19f6209e5b1db295320bfbd3ef00eeaa729c1eec Mon Sep 17 00:00:00 2001 From: Lukas Rytz Date: Mon, 26 Sep 2016 22:11:40 +0200 Subject: SI-9697 / SD-229 Fix DelayedInit subclass capturing local value When a class captures an outer value, a field for that value is created in the class. The class also gets a constructor parameter for the captured value, the constructor will assign the field. LambdaLift re-writes accesses to the local value (Ident trees) to the field. However, if the statement accessing the local value will end up inside the constructor, the access is re-written to the constructor parameter instead. This is the case for constructor statements: class C { { println(capturedLocal) } } If C extends DelayedInit, the statement does not end up in C's constructor, but into a new synthetic method. The access to `capturedLocal` needs to be re-written to the field instead of the constructor parameter. LambdaLift takes the decision (field or constructor parameter) based on the owner chain of `currentOwner`. For the constructor statement block, the owner is a local dummy, for which `logicallyEnclosingMember` returns the constructor symbol. This commit introduces a special case in LambdaLift for local dummies of DelayedInit subclasses: instead of the constructor, we use a temporary symbol representing the synthetic method holding the initializer statements. --- .../scala/tools/nsc/transform/LambdaLift.scala | 44 +++++-- src/reflect/scala/reflect/internal/Symbols.scala | 1 - test/files/run/t9697.check | 1 + test/files/run/t9697.scala | 127 +++++++++++++++++++++ 4 files changed, 162 insertions(+), 11 deletions(-) create mode 100644 test/files/run/t9697.check create mode 100644 test/files/run/t9697.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 74e6c58388..1ec3d4d4cb 100644 --- a/src/compiler/scala/tools/nsc/transform/LambdaLift.scala +++ b/src/compiler/scala/tools/nsc/transform/LambdaLift.scala @@ -104,8 +104,31 @@ abstract class LambdaLift extends InfoTransform { /** Buffers for lifted out classes and methods */ private val liftedDefs = new LinkedHashMap[Symbol, List[Tree]] + val delayedInitDummies = new mutable.HashMap[Symbol, Symbol] + + /** + * For classes capturing locals, LambdaLift uses `local.logicallyEnclosingMember` to decide + * whether an access to the local is re-written to the field or constructor parameter. If the + * access is in a constructor statement, the constructor parameter is used. + * + * For DelayedInit subclasses, constructor statements end up in the synthetic init method + * instead of the constructor itself, so the access should go to the field. This method changes + * `logicallyEnclosingMember` in this case to return a temprorary symbol corresponding to that + * method. + */ + private def logicallyEnclosingMember(sym: Symbol): Symbol = { + if (sym.isLocalDummy) { + val enclClass = sym.enclClass + if (enclClass.isSubClass(DelayedInitClass)) + delayedInitDummies.getOrElseUpdate(enclClass, enclClass.newMethod(nme.delayedInit)) + else + enclClass.primaryConstructor + } else if (sym.isMethod || sym.isClass || sym == NoSymbol) sym + else logicallyEnclosingMember(sym.owner) + } + private def isSameOwnerEnclosure(sym: Symbol) = - sym.owner.logicallyEnclosingMember == currentOwner.logicallyEnclosingMember + logicallyEnclosingMember(sym.owner) == logicallyEnclosingMember(currentOwner) /** Mark symbol `sym` as being free in `enclosure`, unless `sym` * is defined in `enclosure` or there is a class between `enclosure`s owner @@ -139,9 +162,9 @@ abstract class LambdaLift extends InfoTransform { */ private def markFree(sym: Symbol, enclosure: Symbol): Boolean = { // println(s"mark free: ${sym.fullLocationString} marked free in $enclosure") - (enclosure == sym.owner.logicallyEnclosingMember) || { - debuglog("%s != %s".format(enclosure, sym.owner.logicallyEnclosingMember)) - if (enclosure.isPackageClass || !markFree(sym, enclosure.skipConstructor.owner.logicallyEnclosingMember)) false + (enclosure == logicallyEnclosingMember(sym.owner)) || { + debuglog("%s != %s".format(enclosure, logicallyEnclosingMember(sym.owner))) + if (enclosure.isPackageClass || !markFree(sym, logicallyEnclosingMember(enclosure.skipConstructor.owner))) false else { val ss = symSet(free, enclosure) if (!ss(sym)) { @@ -184,14 +207,14 @@ abstract class LambdaLift extends InfoTransform { if (sym == NoSymbol) { assert(name == nme.WILDCARD) } else if (sym.isLocalToBlock) { - val owner = currentOwner.logicallyEnclosingMember + val owner = logicallyEnclosingMember(currentOwner) if (sym.isTerm && !sym.isMethod) markFree(sym, owner) else if (sym.isMethod) markCalled(sym, owner) //symSet(called, owner) += sym } case Select(_, _) => if (sym.isConstructor && sym.owner.isLocalToBlock) - markCalled(sym, currentOwner.logicallyEnclosingMember) + markCalled(sym, logicallyEnclosingMember(currentOwner)) case _ => } super.traverse(tree) @@ -283,17 +306,18 @@ abstract class LambdaLift extends InfoTransform { private def proxy(sym: Symbol) = { def searchIn(enclosure: Symbol): Symbol = { - if (enclosure eq NoSymbol) throw new IllegalArgumentException("Could not find proxy for "+ sym.defString +" in "+ sym.ownerChain +" (currentOwner= "+ currentOwner +" )") - debuglog("searching for " + sym + "(" + sym.owner + ") in " + enclosure + " " + enclosure.logicallyEnclosingMember) + if (enclosure eq NoSymbol) + throw new IllegalArgumentException("Could not find proxy for "+ sym.defString +" in "+ sym.ownerChain +" (currentOwner= "+ currentOwner +" )") + debuglog("searching for " + sym + "(" + sym.owner + ") in " + enclosure + " " + logicallyEnclosingMember(enclosure)) val proxyName = proxyNames.getOrElse(sym, sym.name) - val ps = (proxies get enclosure.logicallyEnclosingMember).toList.flatten find (_.name == proxyName) + val ps = (proxies get logicallyEnclosingMember(enclosure)).toList.flatten find (_.name == proxyName) ps getOrElse searchIn(enclosure.skipConstructor.owner) } debuglog("proxy %s from %s has logical enclosure %s".format( sym.debugLocationString, currentOwner.debugLocationString, - sym.owner.logicallyEnclosingMember.debugLocationString) + logicallyEnclosingMember(sym.owner).debugLocationString) ) if (isSameOwnerEnclosure(sym)) sym diff --git a/src/reflect/scala/reflect/internal/Symbols.scala b/src/reflect/scala/reflect/internal/Symbols.scala index ac025e50ae..f870ecfc15 100644 --- a/src/reflect/scala/reflect/internal/Symbols.scala +++ b/src/reflect/scala/reflect/internal/Symbols.scala @@ -2166,7 +2166,6 @@ trait Symbols extends api.Symbols { self: SymbolTable => final def logicallyEnclosingMember: Symbol = if (isLocalDummy) enclClass.primaryConstructor else if (isMethod || isClass || this == NoSymbol) this - else if (this == NoSymbol) { devWarningDumpStack("NoSymbol.logicallyEnclosingMember", 15); this } else owner.logicallyEnclosingMember /** The top-level class containing this symbol. */ diff --git a/test/files/run/t9697.check b/test/files/run/t9697.check new file mode 100644 index 0000000000..bbd9fd19cf --- /dev/null +++ b/test/files/run/t9697.check @@ -0,0 +1 @@ +warning: there were 6 deprecation warnings (since 2.11.0); re-run with -deprecation for details diff --git a/test/files/run/t9697.scala b/test/files/run/t9697.scala new file mode 100644 index 0000000000..b837feb237 --- /dev/null +++ b/test/files/run/t9697.scala @@ -0,0 +1,127 @@ +object log { + val b = new collection.mutable.StringBuilder + def apply(s: Any): Unit = b.append(s) + def check(s: String) = { + val bs = b.toString + assert(s == bs, bs) + b.clear() + } +} + +package t9697 { + abstract class WA extends DelayedInit { + override def delayedInit(x: => Unit): Unit = x + val waField = "4" + } + + class C { + def b(s: String) = log(s) + val cField = "1" + + { + val dummyLocal = "2" + new WA { + val anonField = "3" + b(cField) + b(dummyLocal) + b(anonField) + b(waField) + } + } + } +} + +package sd229 { + class Broken { + def is(ee: AnyRef) = { + new Delayed { + log(ee) + } + } + } + + class Delayed extends DelayedInit { + def delayedInit(x: => Unit): Unit = x + } +} + + +// already fixed in 2.11.8, crashes in 2.10.6 +package t4683a { + class A { log("a") } + class B { log("b") } + class Bug extends DelayedInit { + log("bug") + def foo(a: A): B = new B + def delayedInit(init: => Unit): Unit = init + } +} + +// already fixed in 2.12.0-RC1, crashes in 2.11.8 +package t4683b { + class Entity extends DelayedInit { + def delayedInit(x: => Unit): Unit = x + + class Field + + protected def EntityField[T <: Entity: reflect.ClassTag] = new Field + + def find[T <: Entity: reflect.ClassTag] { + Nil.map(dbo => { + class EntityHolder extends Entity { + val entity = EntityField[T] + } + }) + log("find") + } + } +} + +package t4683c { + trait T extends DelayedInit { + def delayedInit(body: => Unit) = { + log("init") + body + } + } +} + +package t4683d { + class C extends DelayedInit { + def delayedInit(body: => Unit): Unit = body + } + class Injector { + def test: Object = { + val name = "k" + class crash extends C { + log(name) + } + new crash() + } + } +} + +object Test extends App { + new t9697.C() + log.check("1234") + + new sd229.Broken().is("hi") + log.check("hi") + + val a: t4683a.A = new t4683a.A + var b: t4683a.B = null + new t4683a.Bug { + val b = foo(a) + } + log.check("abugb") + + new t4683b.Entity().find[t4683b.Entity] + log.check("find") + + val f = (p1: Int) => new t4683c.T { log(p1) } + f(5) + log.check("init5") + + new t4683d.Injector().test + log.check("k") +} -- cgit v1.2.3 From ad6bf3033fbdbd1d2c8bdea245f8347cfe292c1b Mon Sep 17 00:00:00 2001 From: Lukas Rytz Date: Tue, 27 Sep 2016 08:05:47 +0200 Subject: SI-4683 fix $outer accesses in class bodies extending DelayedInit Constructors rewrites references to parameter accessor methods in the constructor to references to parameters. It avoids doing so for subclasses of DelayedInit. This commit makes sure the rewrite does not happen for the $outer paramter, a case that was simply forgotten. --- .../scala/tools/nsc/transform/Constructors.scala | 2 +- test/files/run/t9697.check | 2 +- test/files/run/t9697.scala | 77 ++++++++++++++++++++++ 3 files changed, 79 insertions(+), 2 deletions(-) (limited to 'test') diff --git a/src/compiler/scala/tools/nsc/transform/Constructors.scala b/src/compiler/scala/tools/nsc/transform/Constructors.scala index 8d362f13dd..daf645fd20 100644 --- a/src/compiler/scala/tools/nsc/transform/Constructors.scala +++ b/src/compiler/scala/tools/nsc/transform/Constructors.scala @@ -527,7 +527,7 @@ abstract class Constructors extends Statics with Transform with TypingTransforme super.transform(tree) else if (canBeSupplanted(tree.symbol)) gen.mkAttributedIdent(parameter(tree.symbol)) setPos tree.pos - else if (tree.symbol.outerSource == clazz) + else if (tree.symbol.outerSource == clazz && !isDelayedInitSubclass) gen.mkAttributedIdent(parameterNamed(nme.OUTER)) setPos tree.pos else super.transform(tree) diff --git a/test/files/run/t9697.check b/test/files/run/t9697.check index bbd9fd19cf..2a4f01c14f 100644 --- a/test/files/run/t9697.check +++ b/test/files/run/t9697.check @@ -1 +1 @@ -warning: there were 6 deprecation warnings (since 2.11.0); re-run with -deprecation for details +warning: there were 9 deprecation warnings (since 2.11.0); re-run with -deprecation for details diff --git a/test/files/run/t9697.scala b/test/files/run/t9697.scala index b837feb237..eb8e44f8fc 100644 --- a/test/files/run/t9697.scala +++ b/test/files/run/t9697.scala @@ -101,6 +101,57 @@ package t4683d { } } +package t4683e { + class DelayedInitTest { + def a = log("uh") + class B extends DelayedInit { + a + def delayedInit(body: => Unit): Unit = body + } + } +} + +package t4683f { + class Foo extends DelayedInit { + log("fooInit") + def delayedInit(newBody: => Unit): Unit = { + log("delayedInit") + inits = { + val f = () => newBody + if (inits == null) { + log("initsNull") + List(f) + } else + f :: inits + } + } + def foo = log("foo") + var inits: List[() => Unit] = Nil + } + + class Bar extends Foo { + log("barInit") + def bar = foo + def newBaz: Foo = new Baz + private class Baz extends Foo { + log("bazInit") + bar + } + } +} + +package t4683g { + trait MatExpWorld { self => + class T extends Runner { val expWorld: self.type = self } + } + + trait Runner extends DelayedInit { + def delayedInit(init: => Unit): Unit = init + val expWorld: MatExpWorld + } +} + + object Test extends App { new t9697.C() log.check("1234") @@ -124,4 +175,30 @@ object Test extends App { new t4683d.Injector().test log.check("k") + + val dit = new t4683e.DelayedInitTest() + new dit.B() + log.check("uh") + + val fuu = new t4683f.Foo + log.check("delayedInitinitsNull") + fuu.inits.foreach(_.apply()) + log.check("fooInit") + assert(fuu.inits == Nil) // the (delayed) initializer of Foo sets the inits field to Nil + + val brr = new t4683f.Bar + log.check("delayedInitinitsNulldelayedInit") // delayedInit is called once for each constructor + brr.inits.foreach(_.apply()) + log.check("barInitfooInit") + assert(brr.inits == Nil) + + val bzz = brr.newBaz + log.check("delayedInitinitsNulldelayedInit") + bzz.inits.foreach(_.apply()) + log.check("bazInitfoofooInit") + assert(bzz.inits == Nil) + + val mew = new t4683g.MatExpWorld { } + val mt = new mew.T + assert(mt.expWorld == mew) } -- cgit v1.2.3 From 493e22f321d6e7774e74419242b6e3d61eff6bad Mon Sep 17 00:00:00 2001 From: Adriaan Moors Date: Tue, 27 Sep 2016 16:58:25 -0700 Subject: Emit local module like lazy val The motivation is to use the new fine-grained lock scoping that local lazies have since #5294. Fixes scala/scala-dev#235 Co-Authored-By: Jason Zaugg --- .../scala/tools/nsc/transform/Fields.scala | 42 +++++++++++----------- test/files/run/SD-235.scala | 39 ++++++++++++++++++++ test/files/run/delambdafy_t6028.check | 17 ++++----- test/files/run/local_obj.scala | 9 +++++ test/files/run/t6028.check | 17 ++++----- .../nsc/backend/jvm/opt/ScalaInlineInfoTest.scala | 4 +-- 6 files changed, 88 insertions(+), 40 deletions(-) create mode 100644 test/files/run/SD-235.scala create mode 100644 test/files/run/local_obj.scala (limited to 'test') diff --git a/src/compiler/scala/tools/nsc/transform/Fields.scala b/src/compiler/scala/tools/nsc/transform/Fields.scala index 6cf6a5abce..0c6982384d 100644 --- a/src/compiler/scala/tools/nsc/transform/Fields.scala +++ b/src/compiler/scala/tools/nsc/transform/Fields.scala @@ -193,20 +193,6 @@ abstract class Fields extends InfoTransform with ast.TreeDSL with TypingTransfor // not be emitted as ACC_FINAL. They are FINAL in the Scala sense, though: cannot be overridden. private final val ModuleOrLazyFieldFlags = FINAL | PrivateLocal | SYNTHETIC | NEEDS_TREES - private def newModuleVarSymbol(owner: Symbol, module: Symbol, tp: Type): TermSymbol = { -// println(s"new module var in $site for $module of type $tp") - val flags = MODULEVAR | (if (owner.isClass) ModuleOrLazyFieldFlags else 0) - - val moduleVar = - (owner.newVariable(nme.moduleVarName(module.name.toTermName), module.pos.focus, flags) - setInfo tp - addAnnotation VolatileAttr) - - moduleOrLazyVarOf(module) = moduleVar - - moduleVar - } - private def moduleInit(module: Symbol, moduleVar: Symbol) = { // println(s"moduleInit for $module in ${module.ownerChain} --> ${moduleVarOf.get(module)}") def moduleVarRef = gen.mkAttributedRef(moduleVar) @@ -380,8 +366,16 @@ abstract class Fields extends InfoTransform with ast.TreeDSL with TypingTransfor (existingGetter ne NoSymbol) && (tp matches (site memberInfo existingGetter).resultType) // !existingGetter.isDeferred && -- see (3) } - def newModuleVarMember(member: Symbol): TermSymbol = - newModuleVarSymbol(clazz, member, site.memberType(member).resultType) + def newModuleVarMember(module: Symbol): TermSymbol = { + val moduleVar = + (clazz.newVariable(nme.moduleVarName(module.name.toTermName), module.pos.focus, MODULEVAR | ModuleOrLazyFieldFlags) + setInfo site.memberType(module).resultType + addAnnotation VolatileAttr) + + moduleOrLazyVarOf(module) = moduleVar + + moduleVar + } def newLazyVarMember(member: Symbol): TermSymbol = Fields.this.newLazyVarMember(clazz, member, site.memberType(member).resultType) @@ -531,7 +525,8 @@ abstract class Fields extends InfoTransform with ast.TreeDSL with TypingTransfor def mkTypedValDef(sym: Symbol, rhs: Tree = EmptyTree) = typedPos(sym.pos)(ValDef(sym, rhs)).asInstanceOf[ValDef] /** - * Desugar a local `lazy val x: Int = rhs` into + * Desugar a local `lazy val x: Int = rhs` or a local object into + * * ``` * val x$lzy = new scala.runtime.LazyInt() * def x$lzycompute(): Int = @@ -541,10 +536,13 @@ abstract class Fields extends InfoTransform with ast.TreeDSL with TypingTransfor * } * def x(): Int = if (x$lzy.initialized()) x$lzy.value() else x$lzycompute() * ``` + * + * The expansion is the same for local lazy vals and local objects, + * except for the name of the val ($lzy or */ private def mkLazyLocalDef(lazyVal: Symbol, rhs: Tree): Tree = { import CODE._ - import scala.reflect.NameTransformer.LAZY_LOCAL_SUFFIX_STRING + import scala.reflect.{NameTransformer => nx} val owner = lazyVal.owner val lazyValType = lazyVal.tpe.resultType @@ -555,8 +553,7 @@ abstract class Fields extends InfoTransform with ast.TreeDSL with TypingTransfor val lazyName = lazyVal.name.toTermName val pos = lazyVal.pos.focus - // used twice: once in the same owner as the lazy val, another time inside the compute method - val localLazyName = lazyName append LAZY_LOCAL_SUFFIX_STRING + val localLazyName = lazyName append (if (lazyVal.isModule) nx.MODULE_VAR_SUFFIX_STRING else nx.LAZY_LOCAL_SUFFIX_STRING) // The lazy holder val need not be mutable, as we write to its field. // In fact, it MUST not be mutable to avoid capturing it as an ObjectRef in lambdalift @@ -730,8 +727,9 @@ abstract class Fields extends InfoTransform with ast.TreeDSL with TypingTransfor val cd = super.transform(ClassDef(statSym.moduleClass, impl) setType NoType) if (currOwner.isClass) cd else { // local module -- symbols cannot be generated by info transformer, so do it all here - val moduleVar = newModuleVarSymbol(currOwner, statSym, statSym.info.resultType) - Thicket(cd :: mkTypedValDef(moduleVar) :: mkAccessor(statSym)(moduleInit(statSym, moduleVar)) :: Nil) + val Block(stats, _) = mkLazyLocalDef(statSym, gen.newModule(statSym, statSym.info.resultType)) + + Thicket(cd :: stats) } case tree => diff --git a/test/files/run/SD-235.scala b/test/files/run/SD-235.scala new file mode 100644 index 0000000000..eb79c6fe71 --- /dev/null +++ b/test/files/run/SD-235.scala @@ -0,0 +1,39 @@ +class C { + var ORef: Object = null + def test = { + object O { + assert(!Thread.holdsLock(C.this)) + assert(Thread.holdsLock(ORef)) + } + val captor = new { def oh = O } + val refField = captor.getClass.getDeclaredFields.last + refField.setAccessible(true) + assert(refField.getType.toString.contains("LazyRef"), refField) + ORef = refField.get(captor) + O + } +} + +class D { + var ORef: Object = null + def test = { + lazy val O = { + assert(!Thread.holdsLock(D.this)) + assert(Thread.holdsLock(ORef)) + "O" + } + val captor = new { def oh = O } + val refField = captor.getClass.getDeclaredFields.last + refField.setAccessible(true) + assert(refField.getType.toString.contains("LazyRef"), refField) + ORef = refField.get(captor) + O + } +} + +object Test { + def main(args: Array[String]): Unit = { + new C().test + new D().test + } +} diff --git a/test/files/run/delambdafy_t6028.check b/test/files/run/delambdafy_t6028.check index 7b319c92dd..86cb1d5e97 100644 --- a/test/files/run/delambdafy_t6028.check +++ b/test/files/run/delambdafy_t6028.check @@ -15,7 +15,7 @@ package { } }; def bar(barParam: String): Object = { - @volatile var MethodLocalObject$module: runtime.VolatileObjectRef = scala.runtime.VolatileObjectRef.zero(); + lazy val MethodLocalObject$module: scala.runtime.LazyRef = new scala.runtime.LazyRef(); T.this.MethodLocalObject$1(barParam, MethodLocalObject$module) }; def tryy(tryyParam: String): Function0 = { @@ -42,13 +42,14 @@ package { def $outer(): T = MethodLocalObject$2.this.$outer; def $outer(): T = MethodLocalObject$2.this.$outer }; - final private[this] def MethodLocalObject$lzycompute$1(barParam$1: String, MethodLocalObject$module$1: runtime.VolatileObjectRef): Unit = T.this.synchronized[Unit](if (MethodLocalObject$module$1.elem.$asInstanceOf[T#MethodLocalObject$2.type]().eq(null)) - MethodLocalObject$module$1.elem = new T#MethodLocalObject$2.type(T.this, barParam$1)); - final private[this] def MethodLocalObject$1(barParam$1: String, MethodLocalObject$module$1: runtime.VolatileObjectRef): T#MethodLocalObject$2.type = { - if (MethodLocalObject$module$1.elem.$asInstanceOf[T#MethodLocalObject$2.type]().eq(null)) - T.this.MethodLocalObject$lzycompute$1(barParam$1, MethodLocalObject$module$1); - (MethodLocalObject$module$1.elem.$asInstanceOf[T#MethodLocalObject$2.type](): T#MethodLocalObject$2.type) - }; + final private[this] def MethodLocalObject$lzycompute$1(barParam$1: String, MethodLocalObject$module$1: scala.runtime.LazyRef): T#MethodLocalObject$2.type = MethodLocalObject$module$1.synchronized[T#MethodLocalObject$2.type](if (MethodLocalObject$module$1.initialized()) + MethodLocalObject$module$1.value().$asInstanceOf[T#MethodLocalObject$2.type]() + else + MethodLocalObject$module$1.initialize(new T#MethodLocalObject$2.type(T.this, barParam$1)).$asInstanceOf[T#MethodLocalObject$2.type]()); + final private[this] def MethodLocalObject$1(barParam$1: String, MethodLocalObject$module$1: scala.runtime.LazyRef): T#MethodLocalObject$2.type = if (MethodLocalObject$module$1.initialized()) + MethodLocalObject$module$1.value().$asInstanceOf[T#MethodLocalObject$2.type]() + else + T.this.MethodLocalObject$lzycompute$1(barParam$1, MethodLocalObject$module$1); final private[this] def $anonfun$tryy$1(tryyParam$1: String, tryyLocal$1: runtime.ObjectRef): Unit = try { tryyLocal$1.elem = tryyParam$1 } finally () diff --git a/test/files/run/local_obj.scala b/test/files/run/local_obj.scala new file mode 100644 index 0000000000..25123f7078 --- /dev/null +++ b/test/files/run/local_obj.scala @@ -0,0 +1,9 @@ +class C { + val z = 2 + def mod = { object x { val y = z } ; x.y } +} + +object Test extends App { + val c = new C + assert(c.mod == c.z, s"${c.mod} != ${c.z}") +} diff --git a/test/files/run/t6028.check b/test/files/run/t6028.check index 903ea3b753..05634fa8eb 100644 --- a/test/files/run/t6028.check +++ b/test/files/run/t6028.check @@ -15,7 +15,7 @@ package { } }; def bar(barParam: Int): Object = { - @volatile var MethodLocalObject$module: runtime.VolatileObjectRef = scala.runtime.VolatileObjectRef.zero(); + lazy val MethodLocalObject$module: scala.runtime.LazyRef = new scala.runtime.LazyRef(); T.this.MethodLocalObject$1(barParam, MethodLocalObject$module) }; def tryy(tryyParam: Int): Function0 = { @@ -54,13 +54,14 @@ package { def $outer(): T = MethodLocalObject$2.this.$outer; def $outer(): T = MethodLocalObject$2.this.$outer }; - final private[this] def MethodLocalObject$lzycompute$1(barParam$1: Int, MethodLocalObject$module$1: runtime.VolatileObjectRef): Unit = T.this.synchronized[Unit](if (MethodLocalObject$module$1.elem.$asInstanceOf[T#MethodLocalObject$2.type]().eq(null)) - MethodLocalObject$module$1.elem = new T#MethodLocalObject$2.type(T.this, barParam$1)); - final private[this] def MethodLocalObject$1(barParam$1: Int, MethodLocalObject$module$1: runtime.VolatileObjectRef): T#MethodLocalObject$2.type = { - if (MethodLocalObject$module$1.elem.$asInstanceOf[T#MethodLocalObject$2.type]().eq(null)) - T.this.MethodLocalObject$lzycompute$1(barParam$1, MethodLocalObject$module$1); - (MethodLocalObject$module$1.elem.$asInstanceOf[T#MethodLocalObject$2.type](): T#MethodLocalObject$2.type) - }; + final private[this] def MethodLocalObject$lzycompute$1(barParam$1: Int, MethodLocalObject$module$1: scala.runtime.LazyRef): T#MethodLocalObject$2.type = MethodLocalObject$module$1.synchronized[T#MethodLocalObject$2.type](if (MethodLocalObject$module$1.initialized()) + MethodLocalObject$module$1.value().$asInstanceOf[T#MethodLocalObject$2.type]() + else + MethodLocalObject$module$1.initialize(new T#MethodLocalObject$2.type(T.this, barParam$1)).$asInstanceOf[T#MethodLocalObject$2.type]()); + final private[this] def MethodLocalObject$1(barParam$1: Int, MethodLocalObject$module$1: scala.runtime.LazyRef): T#MethodLocalObject$2.type = if (MethodLocalObject$module$1.initialized()) + MethodLocalObject$module$1.value().$asInstanceOf[T#MethodLocalObject$2.type]() + else + T.this.MethodLocalObject$lzycompute$1(barParam$1, MethodLocalObject$module$1); @SerialVersionUID(value = 0) final class $anonfun$tryy$1 extends scala.runtime.AbstractFunction0$mcV$sp with Serializable { def ($outer: T, tryyParam$1: Int, tryyLocal$1: runtime.IntRef): <$anon: Function0> = { $anonfun$tryy$1.super.(); diff --git a/test/junit/scala/tools/nsc/backend/jvm/opt/ScalaInlineInfoTest.scala b/test/junit/scala/tools/nsc/backend/jvm/opt/ScalaInlineInfoTest.scala index 8861577366..5cedc483cd 100644 --- a/test/junit/scala/tools/nsc/backend/jvm/opt/ScalaInlineInfoTest.scala +++ b/test/junit/scala/tools/nsc/backend/jvm/opt/ScalaInlineInfoTest.scala @@ -105,10 +105,10 @@ class ScalaInlineInfoTest extends BytecodeTesting { ("x4$(LT;)I", MethodInlineInfo(true ,false,false)), ("x5()I", MethodInlineInfo(true, false,false)), ("x5$(LT;)I", MethodInlineInfo(true ,false,false)), - ("L$1(Lscala/runtime/VolatileObjectRef;)LT$L$2$;", MethodInlineInfo(true, false,false)), + ("L$1(Lscala/runtime/LazyRef;)LT$L$2$;", MethodInlineInfo(true, false,false)), ("nest$1()I", MethodInlineInfo(true, false,false)), ("$init$(LT;)V", MethodInlineInfo(true,false,false)), - ("L$lzycompute$1(Lscala/runtime/VolatileObjectRef;)V", MethodInlineInfo(true,false,false)) + ("L$lzycompute$1(Lscala/runtime/LazyRef;)LT$L$2$;", MethodInlineInfo(true,false,false)) ), None // warning ) -- cgit v1.2.3