From f7d5f45aa7d5b1977043aa169bf2cdb42f07a002 Mon Sep 17 00:00:00 2001 From: Erik Osheim Date: Mon, 14 May 2012 00:13:16 -0400 Subject: Specialize lazy vals (closes SI-5552) Previously, specialized lazy vals would not work at all when used in specialized classes, and would just return an uninitialized value. After this patch, they work in the same way as other specialized fields do (i.e. a new specialized field is created, and the specialized class uses that instead of the base class' field). Note that there are still known problems with specialized lazy vals (for instance SI-4717) but it seemed to me that this was better than nothing. --- src/compiler/scala/tools/nsc/transform/Mixin.scala | 14 +++++++------- .../scala/tools/nsc/transform/SpecializeTypes.scala | 11 +++++++---- 2 files changed, 14 insertions(+), 11 deletions(-) diff --git a/src/compiler/scala/tools/nsc/transform/Mixin.scala b/src/compiler/scala/tools/nsc/transform/Mixin.scala index 4ce5985af8..79b9317f20 100644 --- a/src/compiler/scala/tools/nsc/transform/Mixin.scala +++ b/src/compiler/scala/tools/nsc/transform/Mixin.scala @@ -903,14 +903,14 @@ abstract class Mixin extends InfoTransform with ast.TreeDSL { if (sym.isLazy && !isEmpty && !clazz.isImplClass) { assert(fieldOffset contains sym, sym) - deriveDefDef(stat)(rhs => - if (isUnit) - mkLazyDef(clazz, sym, List(rhs), UNIT, fieldOffset(sym)) - else { - val Block(stats, res) = rhs + deriveDefDef(stat) { + case t if isUnit => mkLazyDef(clazz, sym, List(t), UNIT, fieldOffset(sym)) + + case Block(stats, res) => mkLazyDef(clazz, sym, stats, Select(This(clazz), res.symbol), fieldOffset(sym)) - } - ) + + case t => t // pass specialized lazy vals through + } } else if (needsInitFlag(sym) && !isEmpty && !clazz.hasFlag(IMPLCLASS | TRAIT)) { assert(fieldOffset contains sym, sym) diff --git a/src/compiler/scala/tools/nsc/transform/SpecializeTypes.scala b/src/compiler/scala/tools/nsc/transform/SpecializeTypes.scala index 7e780304e7..07a10ecb1f 100644 --- a/src/compiler/scala/tools/nsc/transform/SpecializeTypes.scala +++ b/src/compiler/scala/tools/nsc/transform/SpecializeTypes.scala @@ -666,16 +666,19 @@ abstract class SpecializeTypes extends InfoTransform with TypingTransformers { // log("other concrete " + m) forwardToOverload(m) - } else if (m.isValue && !m.isMethod) { // concrete value definition + } else if (m.isMethod && m.hasFlag(LAZY)) { + forwardToOverload(m) + + } else if (m.isValue && !m.isMethod && !m.hasFlag(LAZY)) { // concrete value definition def mkAccessor(field: Symbol, name: Name) = { - val newFlags = (SPECIALIZED | m.getter(clazz).flags) & ~(LOCAL | CASEACCESSOR | PARAMACCESSOR | LAZY) + val newFlags = (SPECIALIZED | m.getter(clazz).flags) & ~(LOCAL | CASEACCESSOR | PARAMACCESSOR) // we rely on the super class to initialize param accessors val sym = sClass.newMethod(name, field.pos, newFlags) info(sym) = SpecializedAccessor(field) sym } def overrideIn(clazz: Symbol, sym: Symbol) = { - val newFlags = (sym.flags | OVERRIDE | SPECIALIZED) & ~(DEFERRED | CASEACCESSOR | PARAMACCESSOR | LAZY) + val newFlags = (sym.flags | OVERRIDE | SPECIALIZED) & ~(DEFERRED | CASEACCESSOR | PARAMACCESSOR) val sym1 = sym.cloneSymbol(clazz, newFlags) sym1 modifyInfo (_ asSeenFrom (clazz.tpe, sym1.owner)) } @@ -881,7 +884,7 @@ abstract class SpecializeTypes extends InfoTransform with TypingTransformers { /** Return the specialized overload of `m`, in the given environment. */ private def specializedOverload(owner: Symbol, sym: Symbol, env: TypeEnv): Symbol = { - val newFlags = (sym.flags | SPECIALIZED) & ~(DEFERRED | CASEACCESSOR | ACCESSOR | LAZY) + val newFlags = (sym.flags | SPECIALIZED) & ~(DEFERRED | CASEACCESSOR) // this method properly duplicates the symbol's info ( sym.cloneSymbol(owner, newFlags, specializedName(sym, env)) modifyInfo (info => subst(env, info.asSeenFrom(owner.thisType, sym.owner))) -- cgit v1.2.3 From b6241963e61f1eaef1a53627197cc13afa0db234 Mon Sep 17 00:00:00 2001 From: Erik Osheim Date: Mon, 14 May 2012 01:01:31 -0400 Subject: Added test case for commit f7d5f45 (re SI-5552) --- test/files/run/t5552.check | 2 ++ test/files/run/t5552.scala | 10 ++++++++++ 2 files changed, 12 insertions(+) create mode 100644 test/files/run/t5552.check create mode 100644 test/files/run/t5552.scala diff --git a/test/files/run/t5552.check b/test/files/run/t5552.check new file mode 100644 index 0000000000..a19a60840e --- /dev/null +++ b/test/files/run/t5552.check @@ -0,0 +1,2 @@ +(3,3) +(3.0,3.0) diff --git a/test/files/run/t5552.scala b/test/files/run/t5552.scala new file mode 100644 index 0000000000..afb8a1f0be --- /dev/null +++ b/test/files/run/t5552.scala @@ -0,0 +1,10 @@ +class C[@specialized(Int) A](a:A) { + lazy val b = (a, a) + def c = b +} +object Test { + def main(args:Array[String]) { + println(new C(3).c) + println(new C(3.0).c) + } +} -- cgit v1.2.3 From efdac16e04bc1cd64113ad22fb4aa8f5191a316f Mon Sep 17 00:00:00 2001 From: Hubert Plociniczak Date: Mon, 14 May 2012 10:32:50 +0200 Subject: Closes SI-5796. --- src/compiler/scala/tools/nsc/transform/LazyVals.scala | 5 +++-- test/files/pos/t5796.scala | 8 ++++++++ 2 files changed, 11 insertions(+), 2 deletions(-) create mode 100644 test/files/pos/t5796.scala diff --git a/src/compiler/scala/tools/nsc/transform/LazyVals.scala b/src/compiler/scala/tools/nsc/transform/LazyVals.scala index 2b26f1590d..e8387c80f5 100644 --- a/src/compiler/scala/tools/nsc/transform/LazyVals.scala +++ b/src/compiler/scala/tools/nsc/transform/LazyVals.scala @@ -75,7 +75,8 @@ abstract class LazyVals extends Transform with TypingTransformers with ast.TreeD val stats1 = stats.flatMap(_ match { case Block(List(d1@DefDef(_, n1, _, _, _, _)), d2@DefDef(_, n2, _, _, _, _)) if (nme.newLazyValSlowComputeName(n2) == n1) => List(d1, d2) - case stat => List(stat) + case stat => + List(stat) }) treeCopy.Block(block1, stats1, expr) @@ -138,7 +139,7 @@ abstract class LazyVals extends Transform with TypingTransformers with ast.TreeD case ValDef(_, _, _, _) if !sym.owner.isModule && !sym.owner.isClass => deriveValDef(tree) { rhs0 => - val rhs = super.transform(rhs0) + val rhs = transform(rhs0) if (LocalLazyValFinder.find(rhs)) typed(addBitmapDefs(sym, rhs)) else rhs } diff --git a/test/files/pos/t5796.scala b/test/files/pos/t5796.scala new file mode 100644 index 0000000000..d05350c535 --- /dev/null +++ b/test/files/pos/t5796.scala @@ -0,0 +1,8 @@ +object Bug { + def foo() { + val v = { + lazy val s = 0 + s + } + } +} -- cgit v1.2.3 From 2d33c00ec0fd0b4a4b863ac02630cb87bc71c53d Mon Sep 17 00:00:00 2001 From: Lukas Rytz Date: Mon, 14 May 2012 11:25:26 +0200 Subject: fix SI-5384 make TreeInfo recognize constructor calls after named arguments transformation. --- src/compiler/scala/reflect/internal/TreeInfo.scala | 24 ++++++++++++++++++++-- test/files/pos/t5384.scala | 7 +++++++ 2 files changed, 29 insertions(+), 2 deletions(-) create mode 100644 test/files/pos/t5384.scala diff --git a/src/compiler/scala/reflect/internal/TreeInfo.scala b/src/compiler/scala/reflect/internal/TreeInfo.scala index 2251310f35..1528061adb 100644 --- a/src/compiler/scala/reflect/internal/TreeInfo.scala +++ b/src/compiler/scala/reflect/internal/TreeInfo.scala @@ -221,9 +221,29 @@ abstract class TreeInfo { case _ => false } + /** + * Named arguments can transform a constructor call into a block, e.g. + * (b = foo, a = bar) + * is transformed to + * { val x$1 = foo + * val x$2 = bar + * (x$2, x$1) + * } + */ + def stripNamedApplyBlock(tree: Tree) = tree match { + case Block(stats, expr) if stats.forall(_.isInstanceOf[ValDef]) => + expr + case _ => + tree + } + /** Is tree a self or super constructor call? */ - def isSelfOrSuperConstrCall(tree: Tree) = - isSelfConstrCall(tree) || isSuperConstrCall(tree) + def isSelfOrSuperConstrCall(tree: Tree) = { + // stripNamedApply for SI-3584: adaptToImplicitMethod in Typers creates a special context + // for implicit search in constructor calls, adaptToImplicitMethod(isSelfOrConstrCall) + val tree1 = stripNamedApplyBlock(tree) + isSelfConstrCall(tree1) || isSuperConstrCall(tree1) + } /** Is tree a variable pattern? */ def isVarPattern(pat: Tree): Boolean = pat match { diff --git a/test/files/pos/t5384.scala b/test/files/pos/t5384.scala new file mode 100644 index 0000000000..4e297d5935 --- /dev/null +++ b/test/files/pos/t5384.scala @@ -0,0 +1,7 @@ +class A(x: String, y: Int)(implicit o: String) +class B1(implicit o: String) extends A(y = 5, x = "a") +class B2(implicit o: String) extends A("a", 5) +class B3(implicit o: String) extends A(y = 5, x = "a")(o) + +class AM[E: Manifest](val x: Unit = (), y: Unit) +class BM[E: Manifest] extends AM[E](y = ()) -- cgit v1.2.3 From 1b198fadd1f4b45042be2b1ecba7d060d7cdfded Mon Sep 17 00:00:00 2001 From: Adriaan Moors Date: Mon, 14 May 2012 11:44:40 +0200 Subject: suspend type vars in SubTypePair's equals SubTypePair's equals method calls =:= on the involved types, which mutates the TypeVars contained in them this is undesirable since we're simply checking whether a subtype test is pending in addition to making subtyping "more correct" for type vars, it should avoid the stackoverflow that's been plaguing us (https://groups.google.com/d/topic/scala-internals/2gHzNjtB4xA/discussion) SubTypePair's equals method method is only called when subtype checking hits a recursion threshold (subsametypeRecursions >= LogPendingSubTypesThreshold) --- src/compiler/scala/reflect/internal/Types.scala | 25 +++++++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/src/compiler/scala/reflect/internal/Types.scala b/src/compiler/scala/reflect/internal/Types.scala index 81582db5f2..808198598b 100644 --- a/src/compiler/scala/reflect/internal/Types.scala +++ b/src/compiler/scala/reflect/internal/Types.scala @@ -3068,7 +3068,8 @@ trait Types extends api.Types { self: SymbolTable => } def registerTypeEquality(tp: Type, typeVarLHS: Boolean): Boolean = { - //println("regTypeEq: "+(safeToString, debugString(tp), typeVarLHS)) //@MDEBUG +// println("regTypeEq: "+(safeToString, debugString(tp), tp.getClass, if (typeVarLHS) "in LHS" else "in RHS", if (suspended) "ZZ" else if (constr.instValid) "IV" else "")) //@MDEBUG +// println("constr: "+ constr) def checkIsSameType(tp: Type) = if(typeVarLHS) constr.inst =:= tp else tp =:= constr.inst @@ -3127,7 +3128,7 @@ trait Types extends api.Types { self: SymbolTable => } def originName = { val name = origin.typeSymbolDirect.decodedName - if (name contains "_$") origin.typeSymbolDirect.decodedName else name + if (name contains "_$") origin.typeSymbolDirect.decodedName else name // wait, what? - what? } def originLocation = { val sym = origin.typeSymbolDirect @@ -3146,7 +3147,7 @@ trait Types extends api.Types { self: SymbolTable => protected def typeVarString = originName override def safeToString = ( if ((constr eq null) || (constr.inst eq null)) "TVar<" + originName + "=null>" - else if (constr.inst ne NoType) "" + constr.inst + else if (constr.inst ne NoType) "=?" + constr.inst else (if(untouchable) "!?" else "?") + levelString + originName ) override def kind = "TypeVar" @@ -4925,7 +4926,23 @@ trait Types extends api.Types { self: SymbolTable => override def hashCode = tp1.hashCode * 41 + tp2.hashCode override def equals(other: Any) = other match { case stp: SubTypePair => - (tp1 =:= stp.tp1) && (tp2 =:= stp.tp2) + // suspend TypeVars in types compared by =:=, + // since we don't want to mutate them simply to check whether a subtype test is pending + // in addition to making subtyping "more correct" for type vars, + // it should avoid the stackoverflow that's been plaguing us (https://groups.google.com/d/topic/scala-internals/2gHzNjtB4xA/discussion) + // this method is only called when subtyping hits a recursion threshold (subsametypeRecursions >= LogPendingSubTypesThreshold) + @inline def suspend(tp: Type) = + if (tp.isGround) null else suspendTypeVarsInType(tp) + @inline def revive(suspension: List[TypeVar]) = + if (suspension ne null) suspension foreach (_.suspended = false) + + val suspensions = Array(tp1, stp.tp1, tp2, stp.tp2) map suspend + + val sameTypes = (tp1 =:= stp.tp1) && (tp2 =:= stp.tp2) + + suspensions foreach revive + + sameTypes case _ => false } -- cgit v1.2.3 From 18efdedfb97de7ca9f6f6ce385194d5a6902769d Mon Sep 17 00:00:00 2001 From: Hubert Plociniczak Date: Wed, 9 May 2012 22:33:09 +0200 Subject: Better fix for SI-5676. Review by @paulp --- .../scala/tools/nsc/backend/jvm/GenASM.scala | 7 +++--- .../scala/tools/nsc/backend/jvm/GenJVM.scala | 6 ++++-- test/files/run/t5676.check | 3 +++ test/files/run/t5676.flags | 1 + test/files/run/t5676.scala | 24 +++++++++++++++++++++ test/pending/run/t5676.flags | 1 - test/pending/run/t5676.scala | 25 ---------------------- 7 files changed, 36 insertions(+), 31 deletions(-) create mode 100644 test/files/run/t5676.check create mode 100644 test/files/run/t5676.flags create mode 100644 test/files/run/t5676.scala delete mode 100644 test/pending/run/t5676.flags delete mode 100644 test/pending/run/t5676.scala diff --git a/src/compiler/scala/tools/nsc/backend/jvm/GenASM.scala b/src/compiler/scala/tools/nsc/backend/jvm/GenASM.scala index aa8f469513..225afaf0fb 100644 --- a/src/compiler/scala/tools/nsc/backend/jvm/GenASM.scala +++ b/src/compiler/scala/tools/nsc/backend/jvm/GenASM.scala @@ -274,8 +274,10 @@ abstract class GenASM extends SubComponent with BytecodeWriters { // we can exclude lateFINAL. Such symbols are eligible for inlining, but to // avoid breaking proxy software which depends on subclassing, we do not // emit ACC_FINAL. + // Nested objects won't receive ACC_FINAL in order to allow for their overloading. + val finalFlag = ( - ((sym.rawflags & (Flags.FINAL | Flags.MODULE)) != 0) + (sym.hasFlag(Flags.FINAL) || isTopLevelModule(sym)) && !sym.enclClass.isInterface && !sym.isClassConstructor && !sym.isMutable // lazy vals and vars both @@ -649,8 +651,7 @@ abstract class GenASM extends SubComponent with BytecodeWriters { val INNER_CLASSES_FLAGS = (asm.Opcodes.ACC_PUBLIC | asm.Opcodes.ACC_PRIVATE | asm.Opcodes.ACC_PROTECTED | - asm.Opcodes.ACC_STATIC | asm.Opcodes.ACC_FINAL | - asm.Opcodes.ACC_INTERFACE | asm.Opcodes.ACC_ABSTRACT) + asm.Opcodes.ACC_STATIC | asm.Opcodes.ACC_INTERFACE | asm.Opcodes.ACC_ABSTRACT) val PublicStatic = asm.Opcodes.ACC_PUBLIC | asm.Opcodes.ACC_STATIC val PublicStaticFinal = asm.Opcodes.ACC_PUBLIC | asm.Opcodes.ACC_STATIC | asm.Opcodes.ACC_FINAL diff --git a/src/compiler/scala/tools/nsc/backend/jvm/GenJVM.scala b/src/compiler/scala/tools/nsc/backend/jvm/GenJVM.scala index 042e739abe..f303a8d3fa 100644 --- a/src/compiler/scala/tools/nsc/backend/jvm/GenJVM.scala +++ b/src/compiler/scala/tools/nsc/backend/jvm/GenJVM.scala @@ -191,7 +191,7 @@ abstract class GenJVM extends SubComponent with GenJVMUtil with GenAndroid with val MIN_SWITCH_DENSITY = 0.7 val INNER_CLASSES_FLAGS = - (ACC_PUBLIC | ACC_PRIVATE | ACC_PROTECTED | ACC_STATIC | ACC_FINAL | ACC_INTERFACE | ACC_ABSTRACT) + (ACC_PUBLIC | ACC_PRIVATE | ACC_PROTECTED | ACC_STATIC | ACC_INTERFACE | ACC_ABSTRACT) val PublicStatic = ACC_PUBLIC | ACC_STATIC val PublicStaticFinal = ACC_PUBLIC | ACC_STATIC | ACC_FINAL @@ -1954,8 +1954,10 @@ abstract class GenJVM extends SubComponent with GenJVMUtil with GenAndroid with // we can exclude lateFINAL. Such symbols are eligible for inlining, but to // avoid breaking proxy software which depends on subclassing, we do not // emit ACC_FINAL. + // Nested objects won't receive ACC_FINAL in order to allow for their overloading. + val finalFlag = ( - ((sym.rawflags & (Flags.FINAL | Flags.MODULE)) != 0) + (sym.hasFlag(Flags.FINAL) || isTopLevelModule(sym)) && !sym.enclClass.isInterface && !sym.isClassConstructor && !sym.isMutable // lazy vals and vars both diff --git a/test/files/run/t5676.check b/test/files/run/t5676.check new file mode 100644 index 0000000000..3b562d3046 --- /dev/null +++ b/test/files/run/t5676.check @@ -0,0 +1,3 @@ +ok +false +true diff --git a/test/files/run/t5676.flags b/test/files/run/t5676.flags new file mode 100644 index 0000000000..e1b37447c9 --- /dev/null +++ b/test/files/run/t5676.flags @@ -0,0 +1 @@ +-Xexperimental \ No newline at end of file diff --git a/test/files/run/t5676.scala b/test/files/run/t5676.scala new file mode 100644 index 0000000000..b643c300ce --- /dev/null +++ b/test/files/run/t5676.scala @@ -0,0 +1,24 @@ +import java.lang.reflect.Modifier + +class Bar[T] + +class Foo[T] { + object A extends Bar[T] +} + +class Baz[S] extends Foo[S] { + override object A extends Bar[S] { + def foo(): String = "ok" + } +} + +object Test { + + def main(a: Array[String]) { + val b = new Baz[Any] + println(b.A.foo()) + println(Modifier.isFinal(classOf[Baz[Any]].getModifiers())) + println(Modifier.isFinal(Test.getClass.getModifiers())) + } + +} diff --git a/test/pending/run/t5676.flags b/test/pending/run/t5676.flags deleted file mode 100644 index e1b37447c9..0000000000 --- a/test/pending/run/t5676.flags +++ /dev/null @@ -1 +0,0 @@ --Xexperimental \ No newline at end of file diff --git a/test/pending/run/t5676.scala b/test/pending/run/t5676.scala deleted file mode 100644 index 3ff498eaa2..0000000000 --- a/test/pending/run/t5676.scala +++ /dev/null @@ -1,25 +0,0 @@ - - - - -class Bar[T] - - -class Foo[T] { - object A extends Bar[T] -} - - -class Baz[S] extends Foo[S] { - override object A extends Bar[S] -} - - -object Test { - - def main(a: Array[String]) { - val b = new Baz[Any] - println(b) - } - -} -- cgit v1.2.3