From da90207a7c5411d73a3f6b97c14451d68372c005 Mon Sep 17 00:00:00 2001 From: Gyuhang Shim Date: Mon, 25 Mar 2013 12:32:40 +0900 Subject: Correct sorting example for Ordering in scaladoc Below code snippet, Sorting.quickSort(pairs)(Ordering.by[(String, Int, Int), Int](_._2) should be Sorting.quickSort(pairs)(Ordering.by[(String, Int, Int), Int](_._2)) --- src/library/scala/math/Ordering.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src') diff --git a/src/library/scala/math/Ordering.scala b/src/library/scala/math/Ordering.scala index e9b92541c2..11b12050c9 100644 --- a/src/library/scala/math/Ordering.scala +++ b/src/library/scala/math/Ordering.scala @@ -26,7 +26,7 @@ import scala.language.{implicitConversions, higherKinds} * val pairs = Array(("a", 5, 2), ("c", 3, 1), ("b", 1, 3)) * * // sort by 2nd element - * Sorting.quickSort(pairs)(Ordering.by[(String, Int, Int), Int](_._2) + * Sorting.quickSort(pairs)(Ordering.by[(String, Int, Int), Int](_._2)) * * // sort by the 3rd element, then 1st * Sorting.quickSort(pairs)(Ordering[(Int, String)].on(x => (x._3, x._1))) -- cgit v1.2.3 From ca9c8efac5694d6dbfe1d0393c2e7485a01c3ef5 Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Tue, 26 Mar 2013 10:45:17 +0100 Subject: SI-6793 Don't use super param accessors if inaccessible. "Alias replacement" has been with us since 2005 (13c59adf9). Given: package a { class C1(val v0: String) class C2(v1: String) extends a.C1(v1) { v1 } } The reference to `v1` is rewritten as `C2.super.v0()`, and no field is generated in `C2`. (Oddly, this optimization doesn't seem to kick in if these classes are in the empty package. That's probably a distinct bug.) However, this rewriting is done without consideration of the accessibility of `v0` from `C2`. This commit disables this optimization if there if `v0` is not accessible. --- src/compiler/scala/tools/nsc/typechecker/SuperAccessors.scala | 11 +++++++++-- test/files/run/t6793.scala | 9 +++++++++ test/files/run/t6793b.scala | 11 +++++++++++ test/files/run/t6793c.scala | 11 +++++++++++ 4 files changed, 40 insertions(+), 2 deletions(-) create mode 100644 test/files/run/t6793.scala create mode 100644 test/files/run/t6793b.scala create mode 100644 test/files/run/t6793c.scala (limited to 'src') diff --git a/src/compiler/scala/tools/nsc/typechecker/SuperAccessors.scala b/src/compiler/scala/tools/nsc/typechecker/SuperAccessors.scala index 67639eb530..fa72ad64bf 100644 --- a/src/compiler/scala/tools/nsc/typechecker/SuperAccessors.scala +++ b/src/compiler/scala/tools/nsc/typechecker/SuperAccessors.scala @@ -267,9 +267,16 @@ abstract class SuperAccessors extends transform.Transform with transform.TypingT } } - // direct calls to aliases of param accessors to the superclass in order to avoid + + def isAccessibleFromSuper(sym: Symbol) = { + val pre = SuperType(sym.owner.tpe, qual.tpe) + localTyper.context.isAccessible(sym, pre, superAccess = true) + } + + // Direct calls to aliases of param accessors to the superclass in order to avoid // duplicating fields. - if (sym.isParamAccessor && sym.alias != NoSymbol) { + // ... but, only if accessible (SI-6793) + if (sym.isParamAccessor && sym.alias != NoSymbol && isAccessibleFromSuper(sym.alias)) { val result = (localTyper.typedPos(tree.pos) { Select(Super(qual, tpnme.EMPTY) setPos qual.pos, sym.alias) }).asInstanceOf[Select] diff --git a/test/files/run/t6793.scala b/test/files/run/t6793.scala new file mode 100644 index 0000000000..0b1f1619af --- /dev/null +++ b/test/files/run/t6793.scala @@ -0,0 +1,9 @@ +package a { class C1(private[a] val v0: String) } +package b { class C2(v1: String) extends a.C1(v1) { def foo = v1 } } + +object Test extends App { + new b.C2("x") + + val c2Fields = classOf[b.C2].getDeclaredFields + assert(c2Fields.size == 1, c2Fields.map(_.getName).toList) +} diff --git a/test/files/run/t6793b.scala b/test/files/run/t6793b.scala new file mode 100644 index 0000000000..cb3f2fb2fa --- /dev/null +++ b/test/files/run/t6793b.scala @@ -0,0 +1,11 @@ +package a { + class C1(val v0: String) + class C2(v1: String) extends a.C1(v1) { def foo = v1 } +} + +object Test extends App { + new a.C2("x") + + val c2Fields = classOf[a.C2].getDeclaredFields + assert(c2Fields.isEmpty, c2Fields.map(_.getName).mkString(", ")) +} diff --git a/test/files/run/t6793c.scala b/test/files/run/t6793c.scala new file mode 100644 index 0000000000..e28c7c81a1 --- /dev/null +++ b/test/files/run/t6793c.scala @@ -0,0 +1,11 @@ +package a { + class C1(private[a] val v0: String) + class C2(v1: String) extends a.C1(v1) { def foo = v1 } +} + +object Test extends App { + new a.C2("x").foo + + val c2Fields = classOf[a.C2].getDeclaredFields + assert(c2Fields.isEmpty, c2Fields.map(_.getName).toList) +} -- cgit v1.2.3 From d21f90c2cec03113a7d5971d68bb2d934d8d751f Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Tue, 2 Apr 2013 14:58:46 +0200 Subject: SI-7147 Diagnostic for unexplained assertion in presentation compiler. We don't have a reproducible test for this, so the best we can do is beef up the assertion to shine a little light on the problem. --- src/compiler/scala/tools/nsc/symtab/SymbolLoaders.scala | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) (limited to 'src') diff --git a/src/compiler/scala/tools/nsc/symtab/SymbolLoaders.scala b/src/compiler/scala/tools/nsc/symtab/SymbolLoaders.scala index 348c7f688f..9e5186b731 100644 --- a/src/compiler/scala/tools/nsc/symtab/SymbolLoaders.scala +++ b/src/compiler/scala/tools/nsc/symtab/SymbolLoaders.scala @@ -97,8 +97,15 @@ abstract class SymbolLoaders { val clazz = enterClass(root, name, completer) val module = enterModule(root, name, completer) if (!clazz.isAnonymousClass) { - assert(clazz.companionModule == module, module) - assert(module.companionClass == clazz, clazz) + // Diagnostic for SI-7147 + def msg: String = { + def symLocation(sym: Symbol) = if (sym == null) "null" else s"${clazz.fullLocationString} (from ${clazz.associatedFile})" + sm"""Inconsistent class/module symbol pair for `$name` loaded from ${symLocation(root)}. + |clazz = ${symLocation(clazz)}; clazz.companionModule = ${clazz.companionModule} + |module = ${symLocation(module)}; module.companionClass = ${module.companionClass}""" + } + assert(clazz.companionModule == module, msg) + assert(module.companionClass == clazz, msg) } } -- cgit v1.2.3 From 8e8370362f7c779139a07acaf5de32137dd2ae1b Mon Sep 17 00:00:00 2001 From: Kato Kazuyoshi Date: Tue, 2 Apr 2013 23:19:48 +0900 Subject: Backport #2289's TermNames.unexpandedName as TermNames.originalName Because this implementation is more clear than 2.10.x's and it will simplify a further commit to fix SI-6715. --- src/reflect/scala/reflect/internal/StdNames.scala | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) (limited to 'src') diff --git a/src/reflect/scala/reflect/internal/StdNames.scala b/src/reflect/scala/reflect/internal/StdNames.scala index bcda2bc1ae..d97eaabf72 100644 --- a/src/reflect/scala/reflect/internal/StdNames.scala +++ b/src/reflect/scala/reflect/internal/StdNames.scala @@ -381,13 +381,16 @@ trait StdNames { /** If `name` is an expandedName name, the original name. * Otherwise `name` itself. */ - def originalName(name: Name): Name = { - var i = name.length - while (i >= 2 && !(name.charAt(i - 1) == '$' && name.charAt(i - 2) == '$')) i -= 1 - if (i >= 2) { - while (i >= 3 && name.charAt(i - 3) == '$') i -= 1 - name.subName(i, name.length) - } else name + def originalName(name: Name): Name = name.toString lastIndexOf "$$" match { + case -1 => name + case idx0 => + // Sketchville - We've found $$ but if it's part of $$$ or $$$$ + // or something we need to keep the bonus dollars, so e.g. foo$$$outer + // has an original name of $outer. + var idx = idx0 + while (idx > 0 && name.charAt(idx - 1) == '$') + idx -= 1 + name drop idx + 2 } def unspecializedName(name: Name): Name = ( -- cgit v1.2.3 From 5f9bc0570f0e2fc4a80eee592bf2eb3eaddf1390 Mon Sep 17 00:00:00 2001 From: Kato Kazuyoshi Date: Wed, 20 Mar 2013 04:10:07 +0900 Subject: SI-6715 Shouldn't return "" from TermNames.originalName --- src/reflect/scala/reflect/internal/StdNames.scala | 2 +- test/files/run/t6715.scala | 15 +++++++++++++++ test/scaladoc/run/SI-6715.check | 1 + test/scaladoc/run/SI-6715.scala | 15 +++++++++++++++ 4 files changed, 32 insertions(+), 1 deletion(-) create mode 100644 test/files/run/t6715.scala create mode 100644 test/scaladoc/run/SI-6715.check create mode 100644 test/scaladoc/run/SI-6715.scala (limited to 'src') diff --git a/src/reflect/scala/reflect/internal/StdNames.scala b/src/reflect/scala/reflect/internal/StdNames.scala index d97eaabf72..de7af4340d 100644 --- a/src/reflect/scala/reflect/internal/StdNames.scala +++ b/src/reflect/scala/reflect/internal/StdNames.scala @@ -382,7 +382,7 @@ trait StdNames { * Otherwise `name` itself. */ def originalName(name: Name): Name = name.toString lastIndexOf "$$" match { - case -1 => name + case -1 | 0 => name case idx0 => // Sketchville - We've found $$ but if it's part of $$$ or $$$$ // or something we need to keep the bonus dollars, so e.g. foo$$$outer diff --git a/test/files/run/t6715.scala b/test/files/run/t6715.scala new file mode 100644 index 0000000000..07ff34218a --- /dev/null +++ b/test/files/run/t6715.scala @@ -0,0 +1,15 @@ +import scala.reflect.runtime.universe._ + +class A { + def $$ = 1 + def $times = 1 +} + +object Test { + def main(args: Array[String]): Unit = { + val memberSet: Set[String] = typeOf[A].members.map{ _.toString }.toSet + assert(memberSet contains "method *") + assert(memberSet contains "method $$") + assert(! (memberSet contains "method")) + } +} diff --git a/test/scaladoc/run/SI-6715.check b/test/scaladoc/run/SI-6715.check new file mode 100644 index 0000000000..619c56180b --- /dev/null +++ b/test/scaladoc/run/SI-6715.check @@ -0,0 +1 @@ +Done. diff --git a/test/scaladoc/run/SI-6715.scala b/test/scaladoc/run/SI-6715.scala new file mode 100644 index 0000000000..92d3376234 --- /dev/null +++ b/test/scaladoc/run/SI-6715.scala @@ -0,0 +1,15 @@ +import scala.tools.nsc.doc.model._ +import scala.tools.partest.ScaladocModelTest + +object Test extends ScaladocModelTest { + def scaladocSettings = "" + + override def code = "object A { def $$ = 123 }" + + def testModel(rootPackage: Package) = { + import access._ + + val method = rootPackage._object("A")._method("$$") + assert(method != null) + } +} -- cgit v1.2.3 From f7c9adcd6156a301f3a1cc33f0e07289f44fbf7a Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Tue, 12 Mar 2013 19:54:25 +0100 Subject: Add a cautionary comment to TreeSymSubstitutor. --- src/reflect/scala/reflect/internal/Trees.scala | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'src') diff --git a/src/reflect/scala/reflect/internal/Trees.scala b/src/reflect/scala/reflect/internal/Trees.scala index 754adcb80d..2585b541ed 100644 --- a/src/reflect/scala/reflect/internal/Trees.scala +++ b/src/reflect/scala/reflect/internal/Trees.scala @@ -1434,6 +1434,11 @@ trait Trees extends api.Trees { self: SymbolTable => /** Substitute symbols in `from` with symbols in `to`. Returns a new * tree using the new symbols and whose Ident and Select nodes are * name-consistent with the new symbols. + * + * Note: This is currently a destructive operation on the original Tree. + * Trees currently assigned a symbol in `from` will be assigned the new symbols + * without copying, and trees that define symbols with an `info` that refer + * a symbol in `from` will have a new type assigned. */ class TreeSymSubstituter(from: List[Symbol], to: List[Symbol]) extends Transformer { val symSubst = new SubstSymMap(from, to) -- cgit v1.2.3 From 3ac185b6168a8f526446dacafd883ca5a1cf7a44 Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Tue, 12 Mar 2013 21:11:35 +0100 Subject: Refactor existential related code out of types. For imminent reuse in the subsequent commit. The binary compatibility whitelist is updated to ignore these, as they live in reflect.internal. --- bincompat-backward.whitelist.conf | 24 +++++++ bincompat-forward.whitelist.conf | 48 ++++++++++++++ .../scala/tools/nsc/typechecker/Typers.scala | 70 +------------------- .../reflect/internal/ExistentialsAndSkolems.scala | 77 ++++++++++++++++++++++ 4 files changed, 150 insertions(+), 69 deletions(-) (limited to 'src') diff --git a/bincompat-backward.whitelist.conf b/bincompat-backward.whitelist.conf index 4f627780e6..aa06e1d21c 100644 --- a/bincompat-backward.whitelist.conf +++ b/bincompat-backward.whitelist.conf @@ -159,6 +159,30 @@ filter { matchName="scala.reflect.internal.Names#NameOps.name" problemName=MissingFieldProblem }, + { + matchName="scala.reflect.internal.ExistentialsAndSkolems.existentialTransform$default$3" + problemName=MissingMethodProblem + }, + { + matchName="scala.reflect.internal.ExistentialsAndSkolems.existentialTransform" + problemName=MissingMethodProblem + }, + { + matchName="scala.reflect.internal.ExistentialsAndSkolems.packSymbols" + problemName=MissingMethodProblem + }, + { + matchName="scala.reflect.internal.ExistentialsAndSkolems.packSymbols$default$3" + problemName=MissingMethodProblem + }, + { + matchName="scala.reflect.internal.ExistentialsAndSkolems.isRawParameter" + problemName=MissingMethodProblem + }, + { + matchName="scala.reflect.internal.Trees.substituteInfoParamsIntoDefDef" + problemName=MissingMethodProblem + }, { matchName="scala.reflect.internal.ClassfileConstants.xxxunusedxxxx" problemName=MissingMethodProblem diff --git a/bincompat-forward.whitelist.conf b/bincompat-forward.whitelist.conf index 76e189653b..d0893c5a28 100644 --- a/bincompat-forward.whitelist.conf +++ b/bincompat-forward.whitelist.conf @@ -355,6 +355,54 @@ filter { matchName="scala.reflect.internal.StdNames#TermNames.SelectFromTypeTree" problemName=MissingMethodProblem }, + { + matchName="scala.reflect.internal.ExistentialsAndSkolems.existentialTransform$default$3" + problemName=MissingMethodProblem + }, + { + matchName="scala.reflect.internal.ExistentialsAndSkolems.existentialTransform" + problemName=MissingMethodProblem + }, + { + matchName="scala.reflect.internal.ExistentialsAndSkolems.packSymbols" + problemName=MissingMethodProblem + }, + { + matchName="scala.reflect.internal.ExistentialsAndSkolems.packSymbols$default$3" + problemName=MissingMethodProblem + }, + { + matchName="scala.reflect.internal.ExistentialsAndSkolems.isRawParameter" + problemName=MissingMethodProblem + }, + { + matchName="scala.reflect.internal.Trees.substituteInfoParamsIntoDefDef" + problemName=MissingMethodProblem + }, + { + matchName="scala.reflect.internal.SymbolTable.existentialTransform$default$3" + problemName=MissingMethodProblem + }, + { + matchName="scala.reflect.internal.SymbolTable.existentialTransform" + problemName=MissingMethodProblem + }, + { + matchName="scala.reflect.internal.SymbolTable.substituteInfoParamsIntoDefDef" + problemName=MissingMethodProblem + }, + { + matchName="scala.reflect.internal.SymbolTable.packSymbols" + problemName=MissingMethodProblem + }, + { + matchName="scala.reflect.internal.SymbolTable.packSymbols$default$3" + problemName=MissingMethodProblem + }, + { + matchName="scala.reflect.internal.SymbolTable.isRawParameter" + problemName=MissingMethodProblem + }, { matchName="scala.reflect.internal.ClassfileConstants.CONSTANT_INVOKEDYNAMIC" problemName=MissingMethodProblem diff --git a/src/compiler/scala/tools/nsc/typechecker/Typers.scala b/src/compiler/scala/tools/nsc/typechecker/Typers.scala index a98f20a971..34ba8b46f9 100644 --- a/src/compiler/scala/tools/nsc/typechecker/Typers.scala +++ b/src/compiler/scala/tools/nsc/typechecker/Typers.scala @@ -3767,77 +3767,9 @@ trait Typers extends Modes with Adaptations with Tags { } else res } - def isRawParameter(sym: Symbol) = // is it a type parameter leaked by a raw type? - sym.isTypeParameter && sym.owner.isJavaDefined - - /** If we map a set of hidden symbols to their existential bounds, we - * have a problem: the bounds may themselves contain references to the - * hidden symbols. So this recursively calls existentialBound until - * the typeSymbol is not amongst the symbols being hidden. - */ - def existentialBoundsExcludingHidden(hidden: List[Symbol]): Map[Symbol, Type] = { - def safeBound(t: Type): Type = - if (hidden contains t.typeSymbol) safeBound(t.typeSymbol.existentialBound.bounds.hi) else t - - def hiBound(s: Symbol): Type = safeBound(s.existentialBound.bounds.hi) match { - case tp @ RefinedType(parents, decls) => - val parents1 = parents mapConserve safeBound - if (parents eq parents1) tp - else copyRefinedType(tp, parents1, decls) - case tp => tp - } - - // Hanging onto lower bound in case anything interesting - // happens with it. - mapFrom(hidden)(s => s.existentialBound match { - case TypeBounds(lo, hi) => TypeBounds(lo, hiBound(s)) - case _ => hiBound(s) - }) - } - - /** Given a set `rawSyms` of term- and type-symbols, and a type - * `tp`, produce a set of fresh type parameters and a type so that - * it can be abstracted to an existential type. Every type symbol - * `T` in `rawSyms` is mapped to a clone. Every term symbol `x` of - * type `T` in `rawSyms` is given an associated type symbol of the - * following form: - * - * type x.type <: T with Singleton - * - * The name of the type parameter is `x.type`, to produce nice - * diagnostics. The Singleton parent ensures that the type - * parameter is still seen as a stable type. Type symbols in - * rawSyms are fully replaced by the new symbols. Term symbols are - * also replaced, except for term symbols of an Ident tree, where - * only the type of the Ident is changed. - */ - protected def existentialTransform[T](rawSyms: List[Symbol], tp: Type)(creator: (List[Symbol], Type) => T): T = { - val allBounds = existentialBoundsExcludingHidden(rawSyms) - val typeParams: List[Symbol] = rawSyms map { sym => - val name = sym.name match { - case x: TypeName => x - case x => tpnme.singletonName(x) - } - val bound = allBounds(sym) - val sowner = if (isRawParameter(sym)) context.owner else sym.owner - val quantified = sowner.newExistential(name, sym.pos) - - quantified setInfo bound.cloneInfo(quantified) - } - // Higher-kinded existentials are not yet supported, but this is - // tpeHK for when they are: "if a type constructor is expected/allowed, - // tpeHK must be called instead of tpe." - val typeParamTypes = typeParams map (_.tpeHK) - def doSubst(info: Type) = info.subst(rawSyms, typeParamTypes) - - creator(typeParams map (_ modifyInfo doSubst), doSubst(tp)) - } - /** Compute an existential type from raw hidden symbols `syms` and type `tp` */ - def packSymbols(hidden: List[Symbol], tp: Type): Type = - if (hidden.isEmpty) tp - else existentialTransform(hidden, tp)(existentialAbstraction) + def packSymbols(hidden: List[Symbol], tp: Type): Type = global.packSymbols(hidden, tp, Some(context0.owner)) def isReferencedFrom(ctx: Context, sym: Symbol): Boolean = ctx.owner.isTerm && diff --git a/src/reflect/scala/reflect/internal/ExistentialsAndSkolems.scala b/src/reflect/scala/reflect/internal/ExistentialsAndSkolems.scala index 8b24678fd6..3bcb793926 100644 --- a/src/reflect/scala/reflect/internal/ExistentialsAndSkolems.scala +++ b/src/reflect/scala/reflect/internal/ExistentialsAndSkolems.scala @@ -32,4 +32,81 @@ trait ExistentialsAndSkolems { } (new Deskolemizer).typeSkolems } + + def isRawParameter(sym: Symbol) = // is it a type parameter leaked by a raw type? + sym.isTypeParameter && sym.owner.isJavaDefined + + /** If we map a set of hidden symbols to their existential bounds, we + * have a problem: the bounds may themselves contain references to the + * hidden symbols. So this recursively calls existentialBound until + * the typeSymbol is not amongst the symbols being hidden. + */ + private def existentialBoundsExcludingHidden(hidden: List[Symbol]): Map[Symbol, Type] = { + def safeBound(t: Type): Type = + if (hidden contains t.typeSymbol) safeBound(t.typeSymbol.existentialBound.bounds.hi) else t + + def hiBound(s: Symbol): Type = safeBound(s.existentialBound.bounds.hi) match { + case tp @ RefinedType(parents, decls) => + val parents1 = parents mapConserve safeBound + if (parents eq parents1) tp + else copyRefinedType(tp, parents1, decls) + case tp => tp + } + + // Hanging onto lower bound in case anything interesting + // happens with it. + mapFrom(hidden)(s => s.existentialBound match { + case TypeBounds(lo, hi) => TypeBounds(lo, hiBound(s)) + case _ => hiBound(s) + }) + } + + /** Given a set `rawSyms` of term- and type-symbols, and a type + * `tp`, produce a set of fresh type parameters and a type so that + * it can be abstracted to an existential type. Every type symbol + * `T` in `rawSyms` is mapped to a clone. Every term symbol `x` of + * type `T` in `rawSyms` is given an associated type symbol of the + * following form: + * + * type x.type <: T with Singleton + * + * The name of the type parameter is `x.type`, to produce nice + * diagnostics. The Singleton parent ensures that the type + * parameter is still seen as a stable type. Type symbols in + * rawSyms are fully replaced by the new symbols. Term symbols are + * also replaced, except for term symbols of an Ident tree, where + * only the type of the Ident is changed. + */ + final def existentialTransform[T](rawSyms: List[Symbol], tp: Type, rawOwner: Option[Symbol] = None)(creator: (List[Symbol], Type) => T): T = { + val allBounds = existentialBoundsExcludingHidden(rawSyms) + val typeParams: List[Symbol] = rawSyms map { sym => + val name = sym.name match { + case x: TypeName => x + case x => tpnme.singletonName(x) + } + def rawOwner0 = rawOwner.getOrElse(abort(s"no owner provided for existential transform over raw parameter: $sym")) + val bound = allBounds(sym) + val sowner = if (isRawParameter(sym)) rawOwner0 else sym.owner + val quantified = sowner.newExistential(name, sym.pos) + + quantified setInfo bound.cloneInfo(quantified) + } + // Higher-kinded existentials are not yet supported, but this is + // tpeHK for when they are: "if a type constructor is expected/allowed, + // tpeHK must be called instead of tpe." + val typeParamTypes = typeParams map (_.tpeHK) + def doSubst(info: Type) = info.subst(rawSyms, typeParamTypes) + + creator(typeParams map (_ modifyInfo doSubst), doSubst(tp)) + } + + /** + * Compute an existential type from hidden symbols `hidden` and type `tp`. + * @param hidden The symbols that will be existentially abstracted + * @param hidden The original type + * @param rawOwner The owner for Java raw types. + */ + final def packSymbols(hidden: List[Symbol], tp: Type, rawOwner: Option[Symbol] = None): Type = + if (hidden.isEmpty) tp + else existentialTransform(hidden, tp, rawOwner)(existentialAbstraction) } -- cgit v1.2.3 From d7545ec36bde6a21e5f3edb1b5982e801a53f6a9 Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Tue, 12 Mar 2013 22:45:13 +0100 Subject: Simplify interplay between Uncurry Info- and Tree-Transformers Now, the InfoTransformer is responsible for erasing the path dependent types of formal parameters, at the same place where it flattens nested method types. This is preferable to having the tree transformer overwrite the result of the info transformer, as used to be the case after my previous work on SI-6135 / 493197fc. --- .../scala/tools/nsc/transform/UnCurry.scala | 27 ++++++++-------------- .../scala/reflect/internal/transform/UnCurry.scala | 11 +++++++-- 2 files changed, 19 insertions(+), 19 deletions(-) (limited to 'src') diff --git a/src/compiler/scala/tools/nsc/transform/UnCurry.scala b/src/compiler/scala/tools/nsc/transform/UnCurry.scala index 66328e23bb..2c191096d6 100644 --- a/src/compiler/scala/tools/nsc/transform/UnCurry.scala +++ b/src/compiler/scala/tools/nsc/transform/UnCurry.scala @@ -818,7 +818,8 @@ abstract class UnCurry extends InfoTransform * * This transformation erases the dependent method types by: * - Widening the formal parameter type to existentially abstract - * over the prior parameters (using `packSymbols`) + * over the prior parameters (using `packSymbols`). This transformation + * is performed in the the `InfoTransform`er [[scala.reflect.internal.transform.UnCurry]]. * - Inserting casts in the method body to cast to the original, * precise type. * @@ -846,15 +847,14 @@ abstract class UnCurry extends InfoTransform */ def erase(dd: DefDef): (List[List[ValDef]], Tree) = { import dd.{ vparamss, rhs } - val vparamSyms = vparamss flatMap (_ map (_.symbol)) - val paramTransforms: List[ParamTransform] = - vparamss.flatten.map { p => - val declaredType = p.symbol.info - // existentially abstract over value parameters - val packedType = typer.packSymbols(vparamSyms, declaredType) - if (packedType =:= declaredType) Identity(p) + map2(vparamss.flatten, dd.symbol.info.paramss.flatten) { (p, infoParam) => + val packedType = infoParam.info + if (packedType =:= p.symbol.info) Identity(p) else { + // The Uncurry info transformer existentially abstracted over value parameters + // from the previous parameter lists. + // Change the type of the param symbol p.symbol updateInfo packedType @@ -866,8 +866,8 @@ abstract class UnCurry extends InfoTransform // the method body to refer to this, rather than the parameter. val tempVal: ValDef = { val tempValName = unit freshTermName (p.name + "$") - val newSym = dd.symbol.newTermSymbol(tempValName, p.pos, SYNTHETIC).setInfo(declaredType) - atPos(p.pos)(ValDef(newSym, gen.mkAttributedCast(Ident(p.symbol), declaredType))) + val newSym = dd.symbol.newTermSymbol(tempValName, p.pos, SYNTHETIC).setInfo(p.symbol.info) + atPos(p.pos)(ValDef(newSym, gen.mkAttributedCast(Ident(p.symbol), p.symbol.info))) } Packed(newParam, tempVal) } @@ -885,13 +885,6 @@ abstract class UnCurry extends InfoTransform Block(tempVals, rhsSubstituted) } - // update the type of the method after uncurry. - dd.symbol updateInfo { - val GenPolyType(tparams, tp) = dd.symbol.info - logResult("erased dependent param types for ${dd.symbol.info}") { - GenPolyType(tparams, MethodType(allParams map (_.symbol), tp.finalResultType)) - } - } (allParams :: Nil, rhs1) } } diff --git a/src/reflect/scala/reflect/internal/transform/UnCurry.scala b/src/reflect/scala/reflect/internal/transform/UnCurry.scala index 6dc6a0f7b8..00c7c3de9a 100644 --- a/src/reflect/scala/reflect/internal/transform/UnCurry.scala +++ b/src/reflect/scala/reflect/internal/transform/UnCurry.scala @@ -17,7 +17,14 @@ trait UnCurry { val tp = expandAlias(tp0) tp match { case MethodType(params, MethodType(params1, restpe)) => - apply(MethodType(params ::: params1, restpe)) + // This transformation is described in UnCurryTransformer.dependentParamTypeErasure + val packSymbolsMap = new TypeMap { + // Wrapping in a TypeMap to reuse the code that opts for a fast path if the function is an identity. + def apply(tp: Type): Type = packSymbols(params, tp) + } + val existentiallyAbstractedParam1s = packSymbolsMap.mapOver(params1) + val substitutedResult = restpe.substSym(params1, existentiallyAbstractedParam1s) + apply(MethodType(params ::: existentiallyAbstractedParam1s, substitutedResult)) case MethodType(params, ExistentialType(tparams, restpe @ MethodType(_, _))) => abort("unexpected curried method types with intervening existential") case MethodType(h :: t, restpe) if h.isImplicit => @@ -60,4 +67,4 @@ trait UnCurry { */ def transformInfo(sym: Symbol, tp: Type): Type = if (sym.isType) uncurryType(tp) else uncurry(tp) -} \ No newline at end of file +} -- cgit v1.2.3 From c2534bf61b61b76cebaad09ba303234193709b60 Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Fri, 22 Feb 2013 15:53:29 +0100 Subject: SI-6900 Fix tailrec for dependent method types Uncurry's info transformer could generate a MethodType with cloned parameter symbols. This type was used for the LabelDef generated in the TailCalls phase. But, the RHS of the method still contains types that refer to the original parmameter symbol. Spurious type errors ensued. I've spent a good chunk of time pursuing a more principled fix, in which we keep the symbols in the tree in sync with those in the MethodType. You can relive the procession of false dawns: https://github.com/scala/scala/pull/2248 Ultimately that scheme was derailed by a mismatch between the type parameter `T` and the skolem `T&` in the example below. trait Endo[A] { def apply(a: => A): A } class Test { def foo[T] = new Endo[(T, Unit)] { def apply(v1: => (T, Unit)) = v1 // no bridge created } } Interestingly, by removing the caching in SingleType, I got past that problem. But I didn't characterize it further. This commit sets asides the noble goal of operating in the world of types, and sledgehammers past the crash by casting the arguments to and the result of the label jump generated in TailCalls. --- .../scala/tools/nsc/transform/TailCalls.scala | 14 +++++++-- test/files/run/t6900.scala | 36 ++++++++++++++++++++++ 2 files changed, 48 insertions(+), 2 deletions(-) create mode 100644 test/files/run/t6900.scala (limited to 'src') diff --git a/src/compiler/scala/tools/nsc/transform/TailCalls.scala b/src/compiler/scala/tools/nsc/transform/TailCalls.scala index a767850cba..938499261e 100644 --- a/src/compiler/scala/tools/nsc/transform/TailCalls.scala +++ b/src/compiler/scala/tools/nsc/transform/TailCalls.scala @@ -220,7 +220,10 @@ abstract class TailCalls extends Transform { debuglog("Rewriting tail recursive call: " + fun.pos.lineContent.trim) accessed += ctx.label - typedPos(fun.pos)(Apply(Ident(ctx.label), noTailTransform(recv) :: transformArgs)) + typedPos(fun.pos) { + val args = mapWithIndex(transformArgs)((arg, i) => mkAttributedCastHack(arg, ctx.label.info.params(i + 1).tpe)) + Apply(Ident(ctx.label), noTailTransform(recv) :: args) + } } if (!ctx.isEligible) fail("it is neither private nor final so can be overridden") @@ -280,7 +283,7 @@ abstract class TailCalls extends Transform { typedPos(tree.pos)(Block( List(ValDef(newThis, This(currentClass))), - LabelDef(newCtx.label, newThis :: vpSyms, newRHS) + LabelDef(newCtx.label, newThis :: vpSyms, mkAttributedCastHack(newRHS, newCtx.label.tpe.resultType)) )) } else { @@ -377,6 +380,13 @@ abstract class TailCalls extends Transform { super.transform(tree) } } + + // Workaround for SI-6900. Uncurry installs an InfoTransformer and a tree Transformer. + // These leave us with conflicting view on method signatures; the parameter symbols in + // the MethodType can be clones of the ones originally found on the parameter ValDef, and + // consequently appearing in the typechecked RHS of the method. + private def mkAttributedCastHack(tree: Tree, tpe: Type) = + gen.mkAttributedCast(tree, tpe) } // collect the LabelDefs (generated by the pattern matcher) in a DefDef that are in tail position diff --git a/test/files/run/t6900.scala b/test/files/run/t6900.scala new file mode 100644 index 0000000000..a29d388129 --- /dev/null +++ b/test/files/run/t6900.scala @@ -0,0 +1,36 @@ +import annotation.tailrec + +trait Universe { + type T <: AnyRef +} + +final class Bug { + var i = 1 + def stop() = { i -= 1; i < 0 } + // the alias bypasses the fast path in erasures InfoTransformer + // predicated on `TypeMap.noChangeToSymbols` + type Alias = Any + + @tailrec + // So we get two symbols for `universe`, the original on the ValDef + // and a clone in the MethodType of `f`. + def f(universe: Universe, l: Alias): universe.T = { + if (stop()) null.asInstanceOf[universe.T] else f(universe, null) + } + + @tailrec + def g(universe: Universe)(l: Alias): universe.T = { + if (stop()) null.asInstanceOf[universe.T] else g(universe)(l) + } + + @tailrec + def h(universe: Universe)(l: List[universe.T]): List[universe.T] = { + if (stop()) Nil else h(universe)(l) + } +} + +object Test extends App { + assert(new Bug().f(null, null) == null) + assert(new Bug().g(null)(null) == null) + assert(new Bug().h(null)(null) == Nil) +} \ No newline at end of file -- cgit v1.2.3 From 61308be6dc769257240fdfb32c328a401e82dd85 Mon Sep 17 00:00:00 2001 From: Paul Phillips Date: Wed, 3 Apr 2013 07:44:28 -0700 Subject: Take the N^2 out of the compiler's TreeSet. The code responsible for this performance bug lives on somewhere in the combination of Iterator.single and Iterator.++ and is tracked by SI-7316. What this commit does is bypass the creation and composition of iterators entirely in favor of applying foreach to walking the tree. The important lesson of a bug like this: the occurrence depends on the existence of multiple implementations of basic structures like Trees. For each redundant implementation, scrutiny and testing are divided and "bug diversity" is increased. We should labor hard to structure collections in such a way that people have no good reason not to take advantage of the basic and hopefully battle-tested logic - especially when those people are us. I hope to remove util.TreeSet entirely. Until then, here is the impact of this commit on the time to compile a piece of generated test code. % time scalac3 ./target/generated/src.scala Mar 28 13:20:31 [running phase parser on src.scala] ... Mar 28 13:21:28 [running phase lazyvals on src.scala] Mar 28 13:21:28 [running phase lambdalift on src.scala] <-- WHOA Mar 28 13:25:05 [running phase constructors on src.scala] ... Mar 28 13:25:19 [running phase jvm on icode] 316.387 real, 438.182 user, 8.163 sys To this: 97.927 real, 211.015 user, 8.043 sys % time pscalac ./target/generated/src.scala Mar 28 13:18:47 [running phase parser on src.scala] ... Mar 28 13:19:44 [running phase lazyvals on src.scala] Mar 28 13:19:44 [running phase lambdalift on src.scala] Mar 28 13:19:46 [running phase constructors on src.scala] ... Mar 28 13:19:57 [running phase jvm on icode] 99.909 real, 223.605 user, 7.847 sys That's lambdalift dropping from 217 seconds to 2 seconds. --- src/compiler/scala/tools/nsc/util/TreeSet.scala | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) (limited to 'src') diff --git a/src/compiler/scala/tools/nsc/util/TreeSet.scala b/src/compiler/scala/tools/nsc/util/TreeSet.scala index 3cdbcc5110..d2e9238e8f 100644 --- a/src/compiler/scala/tools/nsc/util/TreeSet.scala +++ b/src/compiler/scala/tools/nsc/util/TreeSet.scala @@ -40,12 +40,22 @@ class TreeSet[T >: Null <: AnyRef](less: (T, T) => Boolean) extends Set[T] { tree = add(tree) } - def iterator = { - def elems(t: Tree): Iterator[T] = { - if (t eq null) Iterator.empty - else elems(t.l) ++ (Iterator single t.elem) ++ elems(t.r) + def iterator = toList.iterator + + override def foreach[U](f: T => U) { + def loop(t: Tree) { + if (t ne null) { + loop(t.l) + f(t.elem) + loop(t.r) + } } - elems(tree) + loop(tree) + } + override def toList = { + val xs = scala.collection.mutable.ListBuffer[T]() + foreach(xs += _) + xs.toList } override def toString(): String = { -- cgit v1.2.3 From 0affa9423bf190137fadaf6513db0e0a515343af Mon Sep 17 00:00:00 2001 From: Eugene Vigdorchik Date: Thu, 4 Apr 2013 15:37:16 +0400 Subject: SI-7321 Memory leak in specialize on multiple compiler runs. Currently the map is declared LinkedHashMap, which doesn't align with other caching maps that are cleared on every run. Linkedness is only needed to ensure deterministic order on the generated specialized classes. The same can be accomplished by sorting generated classes a-posteriori. --- .../tools/nsc/transform/SpecializeTypes.scala | 28 ++++++++++------------ 1 file changed, 12 insertions(+), 16 deletions(-) (limited to 'src') diff --git a/src/compiler/scala/tools/nsc/transform/SpecializeTypes.scala b/src/compiler/scala/tools/nsc/transform/SpecializeTypes.scala index 232148676c..a71920f787 100644 --- a/src/compiler/scala/tools/nsc/transform/SpecializeTypes.scala +++ b/src/compiler/scala/tools/nsc/transform/SpecializeTypes.scala @@ -78,7 +78,7 @@ abstract class SpecializeTypes extends InfoTransform with TypingTransformers { */ /** For a given class and concrete type arguments, give its specialized class */ - val specializedClass: mutable.Map[(Symbol, TypeEnv), Symbol] = new mutable.LinkedHashMap + val specializedClass = perRunCaches.newMap[(Symbol, TypeEnv), Symbol] /** Map a method symbol to a list of its specialized overloads in the same class. */ private val overloads = perRunCaches.newMap[Symbol, List[Overload]]() withDefaultValue Nil @@ -1776,21 +1776,17 @@ abstract class SpecializeTypes extends InfoTransform with TypingTransformers { /** Create specialized class definitions */ def implSpecClasses(trees: List[Tree]): List[Tree] = { - val buf = new mutable.ListBuffer[Tree] - for (tree <- trees) - tree match { - case ClassDef(_, _, _, impl) => - tree.symbol.info // force specialization - for (((sym1, env), specCls) <- specializedClass if sym1 == tree.symbol) { - val parents = specCls.info.parents.map(TypeTree) - buf += - ClassDef(specCls, atPos(impl.pos)(Template(parents, emptyValDef, List())) - .setSymbol(specCls.newLocalDummy(sym1.pos))) setPos tree.pos - debuglog("created synthetic class: " + specCls + " of " + sym1 + " in " + pp(env)) - } - case _ => - } - buf.toList + trees flatMap { + case tree @ ClassDef(_, _, _, impl) => + tree.symbol.info // force specialization + for (((sym1, env), specCls) <- specializedClass if sym1 == tree.symbol) yield { + debuglog("created synthetic class: " + specCls + " of " + sym1 + " in " + pp(env)) + val parents = specCls.info.parents.map(TypeTree) + ClassDef(specCls, atPos(impl.pos)(Template(parents, emptyValDef, List())) + .setSymbol(specCls.newLocalDummy(sym1.pos))) setPos tree.pos + } + case _ => Nil + } sortBy (_.name.decoded) } } -- cgit v1.2.3