From c0ba5eb196cc3c7174c9278dd4d050513e55b4ce Mon Sep 17 00:00:00 2001 From: Vlad Ureche Date: Wed, 12 Jun 2013 14:48:30 +0200 Subject: Removed redundant `retypedMethod` in `Duplicators` It was never used since its introduction in 3ee6b3653 by @dragos. Review by @dragos or @axel22 or @paulp. --- .../scala/tools/nsc/typechecker/Duplicators.scala | 20 ++------------------ 1 file changed, 2 insertions(+), 18 deletions(-) (limited to 'src') diff --git a/src/compiler/scala/tools/nsc/typechecker/Duplicators.scala b/src/compiler/scala/tools/nsc/typechecker/Duplicators.scala index f6142a81be..2b18a0eba7 100644 --- a/src/compiler/scala/tools/nsc/typechecker/Duplicators.scala +++ b/src/compiler/scala/tools/nsc/typechecker/Duplicators.scala @@ -42,9 +42,6 @@ abstract class Duplicators extends Analyzer { protected def newBodyDuplicator(context: Context) = new BodyDuplicator(context) - def retypedMethod(context: Context, tree: Tree, oldThis: Symbol, newThis: Symbol): Tree = - (newBodyDuplicator(context)).retypedMethod(tree.asInstanceOf[DefDef], oldThis, newThis) - /** Return the special typer for duplicate method bodies. */ override def newTyper(context: Context): Typer = newBodyDuplicator(context) @@ -186,20 +183,6 @@ abstract class Duplicators extends Analyzer { stats.foreach(invalidate(_, owner)) } - def retypedMethod(ddef: DefDef, oldThis: Symbol, newThis: Symbol): Tree = { - oldClassOwner = oldThis - newClassOwner = newThis - invalidateAll(ddef.tparams) - mforeach(ddef.vparamss) { vdef => - invalidate(vdef) - vdef.tpe = null - } - ddef.symbol = NoSymbol - enterSym(context, ddef) - debuglog("remapping this of " + oldClassOwner + " to " + newClassOwner) - typed(ddef) - } - private def inspectTpe(tpe: Type) = { tpe match { case MethodType(_, res) => @@ -401,7 +384,8 @@ abstract class Duplicators extends Analyzer { tree.symbol = NoSymbol // maybe we can find a more specific member in a subclass of Any (see AnyVal members, like ==) } val ntree = castType(tree, pt) - super.typed(ntree, mode, pt) + val res = super.typed(ntree, mode, pt) + res } } -- cgit v1.2.3 From c43b504ac1d843a580683a78eca0cb55bb427c15 Mon Sep 17 00:00:00 2001 From: Vlad Ureche Date: Wed, 12 Jun 2013 14:58:09 +0200 Subject: SI-7343 Fixed phase ordering in specialization Specialization rewires class parents during info transformation, and the new info then guides the tree changes. But if a symbol is created during duplication, which runs after specialization, its info is not visited and thus the corresponding tree is not specialized. One manifestation is the following: ``` object Test { class Parent[@specialized(Int) T] def spec_method[@specialized(Int) T](t: T, expectedXSuper: String) = { class X extends Parent[T]() // even in the specialized variant, the local X class // doesn't extend Parent$mcI$sp, since its symbol has // been created after specialization and was not seen // by specialzation's info transformer. ... } } ``` We can fix this by forcing duplication to take place before specialization. Review by @dragos, @paulp or @axel22. --- .../tools/nsc/transform/SpecializeTypes.scala | 32 ++++++++++++- .../scala/tools/nsc/typechecker/Duplicators.scala | 1 + test/files/specialized/SI-7343.scala | 55 ++++++++++++++++++++++ test/files/specialized/spec-ame.check | 2 +- test/files/specialized/spec-ame.scala | 3 ++ 5 files changed, 90 insertions(+), 3 deletions(-) create mode 100644 test/files/specialized/SI-7343.scala (limited to 'src') diff --git a/src/compiler/scala/tools/nsc/transform/SpecializeTypes.scala b/src/compiler/scala/tools/nsc/transform/SpecializeTypes.scala index 39716d4ed0..5ef5d4031b 100644 --- a/src/compiler/scala/tools/nsc/transform/SpecializeTypes.scala +++ b/src/compiler/scala/tools/nsc/transform/SpecializeTypes.scala @@ -1264,7 +1264,35 @@ abstract class SpecializeTypes extends InfoTransform with TypingTransformers { } protected override def newBodyDuplicator(context: Context) = new BodyDuplicator(context) + } + /** Introduced to fix SI-7343: Phase ordering problem between Duplicators and Specialization. + * brief explanation: specialization rewires class parents during info transformation, and + * the new info then guides the tree changes. But if a symbol is created during duplication, + * which runs after specialization, its info is not visited and thus the corresponding tree + * is not specialized. One manifestation is the following: + * ``` + * object Test { + * class Parent[@specialized(Int) T] + * + * def spec_method[@specialized(Int) T](t: T, expectedXSuper: String) = { + * class X extends Parent[T]() + * // even in the specialized variant, the local X class + * // doesn't extend Parent$mcI$sp, since its symbol has + * // been created after specialization and was not seen + * // by specialzation's info transformer. + * ... + * } + * } + * ``` + * We fix this by forcing duplication to take place before specialization. + * + * Note: The constructors phase (which also uses duplication) comes after erasure and uses the + * post-erasure typer => we must protect it from the beforeSpecialization phase shifting. + */ + class SpecializationDuplicator(casts: Map[Symbol, Type]) extends Duplicator(casts) { + override def retyped(context: Context, tree: Tree, oldThis: Symbol, newThis: Symbol, env: scala.collection.Map[Symbol, Type]): Tree = + beforeSpecialize(super.retyped(context, tree, oldThis, newThis, env)) } /** A tree symbol substituter that substitutes on type skolems. @@ -1637,7 +1665,7 @@ abstract class SpecializeTypes extends InfoTransform with TypingTransformers { val tree1 = deriveValDef(tree)(_ => body(symbol.alias).duplicate) debuglog("now typing: " + tree1 + " in " + tree.symbol.owner.fullName) - val d = new Duplicator(emptyEnv) + val d = new SpecializationDuplicator(emptyEnv) val newValDef = d.retyped( localTyper.context1.asInstanceOf[d.Context], tree1, @@ -1664,7 +1692,7 @@ abstract class SpecializeTypes extends InfoTransform with TypingTransformers { val symbol = tree.symbol val meth = addBody(tree, source) - val d = new Duplicator(castmap) + val d = new SpecializationDuplicator(castmap) debuglog("-->d DUPLICATING: " + meth) d.retyped( localTyper.context1.asInstanceOf[d.Context], diff --git a/src/compiler/scala/tools/nsc/typechecker/Duplicators.scala b/src/compiler/scala/tools/nsc/typechecker/Duplicators.scala index 2b18a0eba7..25a1228bf6 100644 --- a/src/compiler/scala/tools/nsc/typechecker/Duplicators.scala +++ b/src/compiler/scala/tools/nsc/typechecker/Duplicators.scala @@ -37,6 +37,7 @@ abstract class Duplicators extends Analyzer { envSubstitution = new SubstSkolemsTypeMap(env.keysIterator.toList, env.valuesIterator.toList) debuglog("retyped with env: " + env) + newBodyDuplicator(context).typed(tree) } diff --git a/test/files/specialized/SI-7343.scala b/test/files/specialized/SI-7343.scala new file mode 100644 index 0000000000..5ee683064c --- /dev/null +++ b/test/files/specialized/SI-7343.scala @@ -0,0 +1,55 @@ +class Parent[@specialized(Int) T] + +object Test extends App { + + /** + * This method will check if specialization is correctly rewiring parents + * for classes defined inside methods. The pattern is important since this + * is how closures are currently represented: as locally-defined anonymous + * classes, which usually end up inside methods. For these closures we do + * want their parents rewired correctly: + * + * ``` + * def checkSuperClass$mIc$sp[T](t: T, ...) = { + * class X extends Parent$mcI$sp // instead of just Parent + * ... + * } + */ + def checkSuperClass[@specialized(Int) T](t: T, expectedXSuper: String) = { + // test target: + // - in checkSuperClass, X should extend Parent + // - in checkSuperClass$mIc$sp, X should extend Parent$mcI$sp + class X extends Parent[T]() + + // get the superclass for X and make sure it's correct + val actualXSuper = (new X).getClass().getSuperclass().getSimpleName() + assert(actualXSuper == expectedXSuper, actualXSuper + " != " + expectedXSuper) + } + + checkSuperClass("x", "Parent") + checkSuperClass(101, "Parent$mcI$sp") + + /** + * This is the same check, but in value. It should work exactly the same + * as its method counterpart. + */ + class Val[@specialized(Int) T](t: T, expectedXSuper: String) { + val check: T = { + class X extends Parent[T]() + + // get the superclass for X and make sure it's correct + val actualXSuper = (new X).getClass().getSuperclass().getSimpleName() + assert(actualXSuper == expectedXSuper, actualXSuper + " != " + expectedXSuper) + t + } + } + + new Val("x", "Parent") + new Val(101, "Parent$mcI$sp") + + /** + * NOTE: The the same check, only modified to affect constructors, won't + * work since the class X definition will always be lifted to become a + * member of the class, making it impossible to force its duplication. + */ +} diff --git a/test/files/specialized/spec-ame.check b/test/files/specialized/spec-ame.check index 9c1713cc8a..cf18c01191 100644 --- a/test/files/specialized/spec-ame.check +++ b/test/files/specialized/spec-ame.check @@ -1,3 +1,3 @@ abc 10 -3 \ No newline at end of file +2 diff --git a/test/files/specialized/spec-ame.scala b/test/files/specialized/spec-ame.scala index 79ee4217ed..129fb9f447 100644 --- a/test/files/specialized/spec-ame.scala +++ b/test/files/specialized/spec-ame.scala @@ -13,6 +13,9 @@ object Test { def main(args: Array[String]) { println((new A("abc")).foo.value) println((new A(10)).foo.value) + // before fixing SI-7343, this was printing 3. Now it's printing 2, + // since the anonymous class created by doing new B[T] { ... } when + // T = Int is now rewired to B$mcI$sp instead of just B[Int] println(runtime.BoxesRunTime.integerBoxCount) } } -- cgit v1.2.3 From e7ac254349e56678824ade3027bca3908882e291 Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Tue, 11 Jun 2013 10:56:48 -0400 Subject: SI-7571 Allow nesting of anonymous classes in value classes 5d9cde105e added deep prohibition of nested classes within a value class. This has the undesirable side effect of prohibiting partial functions literals in method bodies of a value class. The intention of that prohibition was to avoid problems in code using Type Tests, such as: class C(val inner: A) extends AnyVal { class D } def foo(a: Any, other: C) = a match { case _ : other.D } Here, the pattern usually checks that `a.$outer == other`. But that is incongruent with the way that `other` is erased to `A`. However, not all nested classes could lead us into this trap. This commit slightly relaxes the restriction to allow anonymous classes, which can't appear in a type test. The test shows that the translation generates working code. --- src/compiler/scala/tools/nsc/typechecker/Typers.scala | 4 ++-- test/files/neg/valueclasses-impl-restrictions.check | 8 ++------ test/files/neg/valueclasses-impl-restrictions.scala | 4 +++- test/files/run/t7571.scala | 12 ++++++++++++ 4 files changed, 19 insertions(+), 9 deletions(-) create mode 100644 test/files/run/t7571.scala (limited to 'src') diff --git a/src/compiler/scala/tools/nsc/typechecker/Typers.scala b/src/compiler/scala/tools/nsc/typechecker/Typers.scala index 29cd3d4bfa..ed2963fb0f 100644 --- a/src/compiler/scala/tools/nsc/typechecker/Typers.scala +++ b/src/compiler/scala/tools/nsc/typechecker/Typers.scala @@ -1455,8 +1455,8 @@ trait Typers extends Modes with Adaptations with Tags { implRestriction(tree, "nested object") //see https://issues.scala-lang.org/browse/SI-6444 //see https://issues.scala-lang.org/browse/SI-6463 - case _: ClassDef => - implRestriction(tree, "nested class") + case cd: ClassDef if !cd.symbol.isAnonymousClass => // Don't warn about partial functions, etc. SI-7571 + implRestriction(tree, "nested class") // avoiding Type Tests that might check the $outer pointer. case Select(sup @ Super(qual, mix), selector) if selector != nme.CONSTRUCTOR && qual.symbol == clazz && mix != tpnme.EMPTY => //see https://issues.scala-lang.org/browse/SI-6483 implRestriction(sup, "qualified super reference") diff --git a/test/files/neg/valueclasses-impl-restrictions.check b/test/files/neg/valueclasses-impl-restrictions.check index 63924493aa..0af9173f74 100644 --- a/test/files/neg/valueclasses-impl-restrictions.check +++ b/test/files/neg/valueclasses-impl-restrictions.check @@ -6,12 +6,8 @@ valueclasses-impl-restrictions.scala:9: error: implementation restriction: neste This restriction is planned to be removed in subsequent releases. trait I2 { ^ -valueclasses-impl-restrictions.scala:15: error: implementation restriction: nested class is not allowed in value class -This restriction is planned to be removed in subsequent releases. - val i2 = new I2 { val q = x.s } - ^ -valueclasses-impl-restrictions.scala:21: error: implementation restriction: nested class is not allowed in value class +valueclasses-impl-restrictions.scala:23: error: implementation restriction: nested class is not allowed in value class This restriction is planned to be removed in subsequent releases. private[this] class I2(val q: String) ^ -four errors found +three errors found diff --git a/test/files/neg/valueclasses-impl-restrictions.scala b/test/files/neg/valueclasses-impl-restrictions.scala index 137f3f854c..f0577a94aa 100644 --- a/test/files/neg/valueclasses-impl-restrictions.scala +++ b/test/files/neg/valueclasses-impl-restrictions.scala @@ -12,8 +12,10 @@ class X1(val s: String) extends AnyVal { } def y(x: X1) = { - val i2 = new I2 { val q = x.s } + val i2 = new I2 { val q = x.s } // allowed as of SI-7571 i2.z + + { case x => x } : PartialFunction[Int, Int] // allowed } } diff --git a/test/files/run/t7571.scala b/test/files/run/t7571.scala new file mode 100644 index 0000000000..00b9695168 --- /dev/null +++ b/test/files/run/t7571.scala @@ -0,0 +1,12 @@ +class Foo(val a: Int) extends AnyVal { + def foo = { {case x => x + a}: PartialFunction[Int, Int]} + + def bar = (new {}).toString +} + +object Test extends App { + val x = new Foo(1).foo.apply(2) + assert(x == 3, x) + val s = new Foo(1).bar + assert(s.nonEmpty, s) +} -- cgit v1.2.3 From da54f34a6526b49b9e13e60c4fce242325c1c36e Mon Sep 17 00:00:00 2001 From: Viktor Klang Date: Wed, 19 Jun 2013 23:16:53 +0200 Subject: Cleaning up method implementations in Future Optimizations: 1) Avoiding isDefinedAt + apply and using applyOrElse to allow for optimizations later 2) Reducing method sizes to be more JIT + inliner friendly 3) Reusing core combinators to reuse inliner/JIT optimizations and be more code-cache friendly --- src/library/scala/concurrent/Future.scala | 181 ++++++++---------------------- 1 file changed, 44 insertions(+), 137 deletions(-) (limited to 'src') diff --git a/src/library/scala/concurrent/Future.scala b/src/library/scala/concurrent/Future.scala index 6b6ad29074..bc3a241ce7 100644 --- a/src/library/scala/concurrent/Future.scala +++ b/src/library/scala/concurrent/Future.scala @@ -14,7 +14,7 @@ import java.util.concurrent.{ ConcurrentLinkedQueue, TimeUnit, Callable } import java.util.concurrent.TimeUnit.{ NANOSECONDS => NANOS, MILLISECONDS ⇒ MILLIS } import java.lang.{ Iterable => JIterable } import java.util.{ LinkedList => JLinkedList } -import java.util.concurrent.atomic.{ AtomicReferenceFieldUpdater, AtomicInteger, AtomicBoolean } +import java.util.concurrent.atomic.{ AtomicReferenceFieldUpdater, AtomicInteger, AtomicLong, AtomicBoolean } import scala.util.control.NonFatal import scala.Option @@ -101,7 +101,7 @@ trait Future[+T] extends Awaitable[T] { // that also have an executor parameter, which // keeps us from accidentally forgetting to use // the executor parameter. - private implicit def internalExecutor: ExecutionContext = Future.InternalCallbackExecutor + private def internalExecutor = Future.InternalCallbackExecutor /* Callbacks */ @@ -116,9 +116,10 @@ trait Future[+T] extends Awaitable[T] { * $callbackInContext */ def onSuccess[U](pf: PartialFunction[T, U])(implicit executor: ExecutionContext): Unit = onComplete { - case Success(v) if pf isDefinedAt v => pf(v) + case Success(v) => + pf.applyOrElse[T, Any](v, Predef.conforms[T]) // Exploiting the cached function to avoid MatchError case _ => - }(executor) + } /** When this future is completed with a failure (i.e. with a throwable), * apply the provided callback to the throwable. @@ -134,9 +135,10 @@ trait Future[+T] extends Awaitable[T] { * $callbackInContext */ def onFailure[U](callback: PartialFunction[Throwable, U])(implicit executor: ExecutionContext): Unit = onComplete { - case Failure(t) if NonFatal(t) && callback.isDefinedAt(t) => callback(t) + case Failure(t) => + callback.applyOrElse[Throwable, Any](t, Predef.conforms[Throwable]) // Exploiting the cached function to avoid MatchError case _ => - }(executor) + } /** When this future is completed, either through an exception, or a value, * apply the provided function. @@ -186,13 +188,12 @@ trait Future[+T] extends Awaitable[T] { * and throws a corresponding exception if the original future fails. */ def failed: Future[Throwable] = { + implicit val ec = internalExecutor val p = Promise[Throwable]() - onComplete { case Failure(t) => p success t case Success(v) => p failure (new NoSuchElementException("Future.failed not completed with a throwable.")) } - p.future } @@ -203,10 +204,7 @@ trait Future[+T] extends Awaitable[T] { * * Will not be called if the future fails. */ - def foreach[U](f: T => U)(implicit executor: ExecutionContext): Unit = onComplete { - case Success(r) => f(r) - case _ => // do nothing - }(executor) + def foreach[U](f: T => U)(implicit executor: ExecutionContext): Unit = onComplete { _ foreach f } /** Creates a new future by applying the 's' function to the successful result of * this future, or the 'f' function to the failed result. If there is any non-fatal @@ -221,19 +219,11 @@ trait Future[+T] extends Awaitable[T] { */ def transform[S](s: T => S, f: Throwable => Throwable)(implicit executor: ExecutionContext): Future[S] = { val p = Promise[S]() - + // transform on Try has the wrong shape for us here onComplete { - case result => - try { - result match { - case Failure(t) => p failure f(t) - case Success(r) => p success s(r) - } - } catch { - case NonFatal(t) => p failure t - } - }(executor) - + case Success(r) => p complete Try(s(r)) + case Failure(t) => p complete Try(throw f(t)) // will throw fatal errors! + } p.future } @@ -245,19 +235,7 @@ trait Future[+T] extends Awaitable[T] { */ def map[S](f: T => S)(implicit executor: ExecutionContext): Future[S] = { // transform(f, identity) val p = Promise[S]() - - onComplete { - case result => - try { - result match { - case Success(r) => p success f(r) - case f: Failure[_] => p complete f.asInstanceOf[Failure[S]] - } - } catch { - case NonFatal(t) => p failure t - } - }(executor) - + onComplete { v => p complete (v map f) } p.future } @@ -270,20 +248,10 @@ trait Future[+T] extends Awaitable[T] { */ def flatMap[S](f: T => Future[S])(implicit executor: ExecutionContext): Future[S] = { val p = Promise[S]() - onComplete { case f: Failure[_] => p complete f.asInstanceOf[Failure[S]] - case Success(v) => - try { - f(v).onComplete({ - case f: Failure[_] => p complete f.asInstanceOf[Failure[S]] - case Success(v) => p success v - })(internalExecutor) - } catch { - case NonFatal(t) => p failure t - } - }(executor) - + case Success(v) => try f(v) onComplete p.complete catch { case NonFatal(t) => p failure t } + } p.future } @@ -303,34 +271,14 @@ trait Future[+T] extends Awaitable[T] { * Await.result(h, Duration.Zero) // throw a NoSuchElementException * }}} */ - def filter(pred: T => Boolean)(implicit executor: ExecutionContext): Future[T] = { - val p = Promise[T]() - - onComplete { - case f: Failure[_] => p complete f.asInstanceOf[Failure[T]] - case Success(v) => - try { - if (pred(v)) p success v - else p failure new NoSuchElementException("Future.filter predicate is not satisfied") - } catch { - case NonFatal(t) => p failure t - } - }(executor) - - p.future - } + def filter(pred: T => Boolean)(implicit executor: ExecutionContext): Future[T] = + map { + r => if (pred(r)) r else throw new NoSuchElementException("Future.filter predicate is not satisfied") + } /** Used by for-comprehensions. */ final def withFilter(p: T => Boolean)(implicit executor: ExecutionContext): Future[T] = filter(p)(executor) - // final def withFilter(p: T => Boolean) = new FutureWithFilter[T](this, p) - - // final class FutureWithFilter[+S](self: Future[S], p: S => Boolean) { - // def foreach(f: S => Unit): Unit = self filter p foreach f - // def map[R](f: S => R) = self filter p map f - // def flatMap[R](f: S => Future[R]) = self filter p flatMap f - // def withFilter(q: S => Boolean): FutureWithFilter[S] = new FutureWithFilter[S](self, x => p(x) && q(x)) - // } /** Creates a new future by mapping the value of the current future, if the given partial function is defined at that value. * @@ -352,22 +300,10 @@ trait Future[+T] extends Awaitable[T] { * Await.result(h, Duration.Zero) // throw a NoSuchElementException * }}} */ - def collect[S](pf: PartialFunction[T, S])(implicit executor: ExecutionContext): Future[S] = { - val p = Promise[S]() - - onComplete { - case f: Failure[_] => p complete f.asInstanceOf[Failure[S]] - case Success(v) => - try { - if (pf.isDefinedAt(v)) p success pf(v) - else p failure new NoSuchElementException("Future.collect partial function is not defined at: " + v) - } catch { - case NonFatal(t) => p failure t - } - }(executor) - - p.future - } + def collect[S](pf: PartialFunction[T, S])(implicit executor: ExecutionContext): Future[S] = + map { + r => pf.applyOrElse(r, (t: T) => throw new NoSuchElementException("Future.collect partial function is not defined at: " + t)) + } /** Creates a new future that will handle any matching throwable that this * future might contain. If there is no match, or if this future contains @@ -383,9 +319,7 @@ trait Future[+T] extends Awaitable[T] { */ def recover[U >: T](pf: PartialFunction[Throwable, U])(implicit executor: ExecutionContext): Future[U] = { val p = Promise[U]() - - onComplete { case tr => p.complete(tr recover pf) }(executor) - + onComplete { v => p complete (v recover pf) } p.future } @@ -404,17 +338,10 @@ trait Future[+T] extends Awaitable[T] { */ def recoverWith[U >: T](pf: PartialFunction[Throwable, Future[U]])(implicit executor: ExecutionContext): Future[U] = { val p = Promise[U]() - onComplete { - case Failure(t) if pf isDefinedAt t => - try { - p completeWith pf(t) - } catch { - case NonFatal(t) => p failure t - } - case otherwise => p complete otherwise - }(executor) - + case Failure(t) => try pf.applyOrElse(t, (_: Throwable) => this) onComplete p.complete catch { case NonFatal(t) => p failure t } + case other => p complete other + } p.future } @@ -427,19 +354,12 @@ trait Future[+T] extends Awaitable[T] { * with the throwable stored in `that`. */ def zip[U](that: Future[U]): Future[(T, U)] = { + implicit val ec = internalExecutor val p = Promise[(T, U)]() - - this onComplete { + onComplete { case f: Failure[_] => p complete f.asInstanceOf[Failure[(T, U)]] - case Success(r) => - that onSuccess { - case r2 => p success ((r, r2)) - } - that onFailure { - case f => p failure f - } + case Success(s) => that onComplete { c => p.complete(c map { s2 => (s, s2) }) } } - p.future } @@ -458,6 +378,7 @@ trait Future[+T] extends Awaitable[T] { * }}} */ def fallbackTo[U >: T](that: Future[U]): Future[U] = { + implicit val ec = internalExecutor val p = Promise[U]() onComplete { case s @ Success(_) => p complete s @@ -470,23 +391,13 @@ trait Future[+T] extends Awaitable[T] { * that conforms to `S`'s erased type or a `ClassCastException` otherwise. */ def mapTo[S](implicit tag: ClassTag[S]): Future[S] = { - def boxedType(c: Class[_]): Class[_] = { + implicit val ec = internalExecutor + val boxedClass = { + val c = tag.runtimeClass if (c.isPrimitive) Future.toBoxed(c) else c } - - val p = Promise[S]() - - onComplete { - case f: Failure[_] => p complete f.asInstanceOf[Failure[S]] - case Success(t) => - p complete (try { - Success(boxedType(tag.runtimeClass).cast(t).asInstanceOf[S]) - } catch { - case e: ClassCastException => Failure(e) - }) - } - - p.future + require(boxedClass ne null) + map(s => boxedClass.cast(s).asInstanceOf[S]) } /** Applies the side-effecting function to the result of this future, and returns @@ -514,11 +425,9 @@ trait Future[+T] extends Awaitable[T] { */ def andThen[U](pf: PartialFunction[Try[T], U])(implicit executor: ExecutionContext): Future[T] = { val p = Promise[T]() - onComplete { - case r => try if (pf isDefinedAt r) pf(r) finally p complete r - }(executor) - + case r => try pf.applyOrElse[Try[T], Any](r, Predef.conforms[Try[T]]) finally p complete r + } p.future } @@ -579,14 +488,12 @@ object Future { } map (_.result) } - /** Returns a `Future` to the result of the first future in the list that is completed. + /** Returns a new `Future` to the result of the first future in the list that is completed. */ def firstCompletedOf[T](futures: TraversableOnce[Future[T]])(implicit executor: ExecutionContext): Future[T] = { val p = Promise[T]() - val completeFirst: Try[T] => Unit = p tryComplete _ - futures.foreach(_ onComplete completeFirst) - + futures foreach { _ onComplete completeFirst } p.future } @@ -626,7 +533,7 @@ object Future { * }}} */ def fold[T, R](futures: TraversableOnce[Future[T]])(zero: R)(foldFun: (R, T) => R)(implicit executor: ExecutionContext): Future[R] = { - if (futures.isEmpty) Promise.successful(zero).future + if (futures.isEmpty) Future.successful(zero) else sequence(futures).map(_.foldLeft(zero)(foldFun)) } @@ -638,7 +545,7 @@ object Future { * }}} */ def reduce[T, R >: T](futures: TraversableOnce[Future[T]])(op: (R, T) => R)(implicit executor: ExecutionContext): Future[R] = { - if (futures.isEmpty) Promise[R].failure(new NoSuchElementException("reduce attempted on empty collection")).future + if (futures.isEmpty) Future.failed(new NoSuchElementException("reduce attempted on empty collection")) else sequence(futures).map(_ reduceLeft op) } -- cgit v1.2.3 From eebaae55bad824a98fdf4a1811809b8776f36825 Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Mon, 24 Jun 2013 14:23:17 +0200 Subject: SI-7603 Speculative fix for annotation binding error Reports of: error: trait Test is abstract; cannot be instantiated 11:09:50 [ant:scalac] @Test def testClientRequestNum = { 11:09:50 [ant:scalac] ^ Suggest that the deferred processing of a LazyAnnotationInfo is binding the identifier `Test` to the wrong symbol. Inspection of the code shows that the closure also defers capture of the (mutable) field `Namer#typer.context`. This commit captures the context eagerly, and adds logging to let us know if that eagerly captured context ever differs from the its value at the point when the annotation info is forced. I spent a few hours trying to craft a test to back this up, but to no avail. Here's what the log output will look like: [log typer] The var `typer.context` in scala.tools.nsc.typechecker.Namers$NormalNamer@1f5ebb08 was mutated before the annotation new a() was forced. current value = Context(C@Import unit= scope=123861466 errors=false, reportErrors=true, throwErrors=false) original value = Context(C@Import unit= scope=123861466 errors=false, reportErrors=true, throwErrors=false) This confirms the hypothesis for the cause of SI-7603. If you see this message, please comment on that ticket. --- src/compiler/scala/tools/nsc/typechecker/Namers.scala | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) (limited to 'src') diff --git a/src/compiler/scala/tools/nsc/typechecker/Namers.scala b/src/compiler/scala/tools/nsc/typechecker/Namers.scala index 379f56521b..8f542af417 100644 --- a/src/compiler/scala/tools/nsc/typechecker/Namers.scala +++ b/src/compiler/scala/tools/nsc/typechecker/Namers.scala @@ -1420,11 +1420,20 @@ trait Namers extends MethodSynthesis { if (!annotated.isInitialized) tree match { case defn: MemberDef => val ainfos = defn.mods.annotations filterNot (_ eq null) map { ann => + val ctx = typer.context + val annCtx = ctx.make(ann) + annCtx.setReportErrors() // need to be lazy, #1782. beforeTyper to allow inferView in annotation args, SI-5892. AnnotationInfo lazily { - val context1 = typer.context.make(ann) - context1.setReportErrors() - beforeTyper(newTyper(context1) typedAnnotation ann) + if (typer.context ne ctx) + log(sm"""|The var `typer.context` in ${Namer.this} was mutated before the annotation ${ann} was forced. + | + |current value = ${typer.context} + |original value = $ctx + | + |This confirms the hypothesis for the cause of SI-7603. If you see this message, please comment on that ticket.""") + + beforeTyper(newTyper(annCtx) typedAnnotation ann) } } if (ainfos.nonEmpty) { -- cgit v1.2.3 From 228549356ce5dda0305dc2e9e08870859e4de06e Mon Sep 17 00:00:00 2001 From: Vlad Ureche Date: Fri, 14 Jun 2013 15:01:37 +0200 Subject: SI-7344 Specialize methods in private scopes This performs method specialization inside a scope other than a {class, trait, object}: could be another method or a value. This specialization is much simpler, since there is no need to record the new members in the class signature, their signatures are only visible locally. It works according to the usual logic: - we use normalizeMember to create the specialized symbols - we leave DefDef stubs in the tree that are later filled in by tree duplication and adaptation The solution is limited by SI-7579: since the duplicator loses the sym annotations when duplicating, this expansion and rewiring can only take place in code that has not been subject to duplication. You can see the test case for an example. Review by @dragos, @paulp or @axel22. --- .../tools/nsc/transform/SpecializeTypes.scala | 118 ++++++++++++++++----- test/files/specialized/SI-7344.scala | 53 +++++++++ 2 files changed, 147 insertions(+), 24 deletions(-) create mode 100644 test/files/specialized/SI-7344.scala (limited to 'src') diff --git a/src/compiler/scala/tools/nsc/transform/SpecializeTypes.scala b/src/compiler/scala/tools/nsc/transform/SpecializeTypes.scala index 5ef5d4031b..7e85647592 100644 --- a/src/compiler/scala/tools/nsc/transform/SpecializeTypes.scala +++ b/src/compiler/scala/tools/nsc/transform/SpecializeTypes.scala @@ -844,7 +844,11 @@ abstract class SpecializeTypes extends InfoTransform with TypingTransformers { debuglog("%s expands to %s in %s".format(sym, specMember.name.decode, pp(env))) info(specMember) = NormalizedMember(sym) newOverload(sym, specMember, env) - owner.info.decls.enter(specMember) + // if this is a class, we insert the normalized member in scope, + // if this is a method, there's no attached scope for it (EmptyScope) + val decls = owner.info.decls + if (decls != EmptyScope) + decls.enter(specMember) specMember } } @@ -1420,7 +1424,7 @@ abstract class SpecializeTypes extends InfoTransform with TypingTransformers { val env = unify(symbol.tpe, tree.tpe, emptyEnv, false) debuglog("[specSym] checking for rerouting: %s with \n\tsym.tpe: %s, \n\ttree.tpe: %s \n\tenv: %s \n\tname: %s" .format(tree, symbol.tpe, tree.tpe, env, specializedName(symbol, env))) - if (!env.isEmpty) { // a method? + if (env.nonEmpty) { // a method? val specCandidates = qual.tpe.member(specializedName(symbol, env)) val specMember = specCandidates suchThat { s => doesConform(symbol, tree.tpe, qual.tpe.memberType(s), env) @@ -1438,6 +1442,34 @@ abstract class SpecializeTypes extends InfoTransform with TypingTransformers { } else None } + /** Computes residual type parameters after rewiring, like "String" in the following example: + * ``` + * def specMe[@specialized T, U](t: T, u: U) = ??? + * specMe[Int, String](1, "2") => specMe$mIc$sp[String](1, "2") + * ``` + */ + def computeResidualTypeVars(baseTree: Tree, specTree: Tree, baseTargs: List[Tree], env: TypeEnv) = { + val baseSym: Symbol = baseTree.symbol + val specSym: Symbol = specTree.symbol + val residualTargs = baseSym.info.typeParams zip baseTargs collect { + case (tvar, targ) if !env.contains(tvar) || !isPrimitiveValueClass(env(tvar).typeSymbol) => targ + } + + if (specSym.info.typeParams.isEmpty && residualTargs.nonEmpty) { + log("!!! Type args to be applied, but symbol says no parameters: " + ((specSym.defString, residualTargs))) + baseTree + } + else { + ifDebug(assert(residualTargs.length == specSym.info.typeParams.length, + "residual: %s, tparams: %s, env: %s".format(residualTargs, specSym.info.typeParams, env)) + ) + + val tree1 = gen.mkTypeApply(specTree, residualTargs) + debuglog("rewrote " + tree + " to " + tree1) + localTyper.typedOperator(atPos(tree.pos)(tree1)) + } + } + curTree = tree tree match { case Apply(Select(New(tpt), nme.CONSTRUCTOR), args) => @@ -1470,43 +1502,44 @@ abstract class SpecializeTypes extends InfoTransform with TypingTransformers { } transformSuperApply + // This rewires calls to specialized methods defined in a class (which have a receiver) + // class C { + // def foo[@specialized T](t: T): T = t + // C.this.foo(3) // TypeApply(Select(This(C), foo), List(Int)) => C.this.foo$mIc$sp(3) + // } case TypeApply(sel @ Select(qual, name), targs) - if (!specializedTypeVars(symbol.info).isEmpty && name != nme.CONSTRUCTOR) => - def transformTypeApply = { + if (specializedTypeVars(symbol.info).nonEmpty && name != nme.CONSTRUCTOR) => debuglog("checking typeapp for rerouting: " + tree + " with sym.tpe: " + symbol.tpe + " tree.tpe: " + tree.tpe) val qual1 = transform(qual) - // log(">>> TypeApply: " + tree + ", qual1: " + qual1) + log(">>> TypeApply: " + tree + ", qual1: " + qual1) specSym(qual1) match { case Some(specMember) => debuglog("found " + specMember.fullName) ifDebug(assert(symbol.info.typeParams.length == targs.length, symbol.info.typeParams + " / " + targs)) val env = typeEnv(specMember) - val residualTargs = symbol.info.typeParams zip targs collect { - case (tvar, targ) if !env.contains(tvar) || !isPrimitiveValueClass(env(tvar).typeSymbol) => targ - } - // See SI-5583. Don't know why it happens now if it didn't before. - if (specMember.info.typeParams.isEmpty && residualTargs.nonEmpty) { - log("!!! Type args to be applied, but symbol says no parameters: " + ((specMember.defString, residualTargs))) - localTyper.typed(sel) - } - else { - ifDebug(assert(residualTargs.length == specMember.info.typeParams.length, - "residual: %s, tparams: %s, env: %s".format(residualTargs, specMember.info.typeParams, env)) - ) - - val tree1 = gen.mkTypeApply(Select(qual1, specMember), residualTargs) - debuglog("rewrote " + tree + " to " + tree1) - localTyper.typedOperator(atPos(tree.pos)(tree1)) // being polymorphic, it must be a method - } + computeResidualTypeVars(tree, gen.mkAttributedSelect(qual1, specMember), targs, env) case None => treeCopy.TypeApply(tree, treeCopy.Select(sel, qual1, name), super.transformTrees(targs)) // See pos/exponential-spec.scala - can't call transform on the whole tree again. // super.transform(tree) } + + // This rewires calls to specialized methods defined in the local scope. For example: + // def outerMethod = { + // def foo[@specialized T](t: T): T = t + // foo(3) // TypeApply(Ident(foo), List(Int)) => foo$mIc$sp(3) + // } + case TypeApply(sel @ Ident(name), targs) if name != nme.CONSTRUCTOR => + val env = unify(symbol.tpe, tree.tpe, emptyEnv, false) + if (env.isEmpty) super.transform(tree) + else { + overloads(symbol) find (_ matchesEnv env) match { + case Some(Overload(specMember, _)) => computeResidualTypeVars(tree, Ident(specMember), targs, env) + case _ => super.transform(tree) + } } - transformTypeApply case Select(Super(_, _), _) if illegalSpecializedInheritance(currentClass) => val pos = tree.pos @@ -1657,7 +1690,11 @@ abstract class SpecializeTypes extends InfoTransform with TypingTransformers { localTyper.typed(deriveDefDef(tree)(rhs => rhs)) } } - transformDefDef + expandInnerNormalizedMembers(transformDefDef) + + case ddef @ DefDef(_, _, _, _, _, _) => + val tree1 = expandInnerNormalizedMembers(tree) + super.transform(tree1) case ValDef(_, _, _, _) if symbol.hasFlag(SPECIALIZED) && !symbol.isParamAccessor => def transformValDef = { @@ -1682,6 +1719,39 @@ abstract class SpecializeTypes extends InfoTransform with TypingTransformers { } } + /** + * This performs method specialization inside a scope other than a {class, trait, object}: could be another method + * or a value. This specialization is much simpler, since there is no need to record the new members in the class + * signature, their signatures are only visible locally. It works according to the usual logic: + * - we use normalizeMember to create the specialized symbols + * - we leave DefDef stubs in the tree that are later filled in by tree duplication and adaptation + * @see duplicateBody + */ + private def expandInnerNormalizedMembers(tree: Tree) = tree match { + case ddef @ DefDef(_, _, _, vparams :: Nil, _, rhs) + if ddef.symbol.owner.isMethod && + specializedTypeVars(ddef.symbol.info).nonEmpty && + !ddef.symbol.hasFlag(SPECIALIZED) => + + val sym = ddef.symbol + val owner = sym.owner + val norm = normalizeMember(owner, sym, emptyEnv) + + if (norm.length > 1) { + // record the body for duplication + body(sym) = rhs + parameters(sym) = vparams.map(_.symbol) + // to avoid revisiting the member, we can set the SPECIALIZED + // flag. nobody has to see this anyway :) + sym.setFlag(SPECIALIZED) + // create empty bodies for specializations + localTyper.typed(Block(norm.tail.map(sym => DefDef(sym, { vparamss => EmptyTree })), ddef)) + } else + tree + case _ => + tree + } + /** Duplicate the body of the given method `tree` to the new symbol `source`. * * Knowing that the method can be invoked only in the `castmap` type environment, diff --git a/test/files/specialized/SI-7344.scala b/test/files/specialized/SI-7344.scala new file mode 100644 index 0000000000..1040460bd1 --- /dev/null +++ b/test/files/specialized/SI-7344.scala @@ -0,0 +1,53 @@ +/* Test for SI-7344, where specialized methods inside the bodies of other + * methods are not specialized, although they might as well be. The name + * for the specialized method should not be different depending on the + * outside method/class' specialization. */ + +class Test[@specialized(Int, Double) X](val x: X) { + + def checkSpecialization[Y](@specialized(Int, Double) y: Y): X = { + + // checking the specialization using the method name, which we can + // extract from an exception's stack trace. We can match just the + // prefix, since the compiler will add a suffix to the method name + // during lambdalift, when it lifts the local methods outside. + def specMe[@specialized(Int, Double) T, N](t: T, n: N): Unit = checkNameStartsWith(n.toString) + + // expected to specialize: + specMe("x", "specMe") + specMe(123, "specMe$mIc$sp") + specMe(1.3, new { override def toString = "specMe$mDc$sp" }) + + x + } + + // name matching: + private[this] def checkNameStartsWith(prefix: String): Unit = { + val method = (new Exception).getStackTrace()(1).getMethodName() + assert(method.startsWith(prefix), method + ".startsWith(" + prefix + ") should be true") + } +} + +object Test extends App { + val t1 = new Test("x") + val t2 = new Test(123) + val t3 = new Test(1.3) + + // we want specialization to rewire these, + // that's why they're not in a for loop: + t1.checkSpecialization("x") + + // Prevented by SI-7579: + // The duplicator loses the @specialized annotation, + // so our tree transformation doesn't know it needs to + // specialize specMe inside the duplicated (and specialized) + // variants of the `checkSpecialization` method + // t1.checkSpecialization(123) + // t1.checkSpecialization(1.3) + // t2.checkSpecialization("x") + // t2.checkSpecialization(123) + // t2.checkSpecialization(1.3) + // t3.checkSpecialization("x") + // t3.checkSpecialization(123) + // t3.checkSpecialization(1.3) +} -- cgit v1.2.3 From 0c752d7ff85b908b97b1921497dd8b863f8f9693 Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Thu, 27 Jun 2013 11:22:39 +0200 Subject: Less noise on a partest failure. Throwing a BuildException is the polite way to fail the Ant build. Before: BUILD FAILED /Users/jason/code/scala2/build.xml:1522: java.lang.RuntimeException: Test suite finished with 1 case failing: /Users/jason/code/scala2/test/files/pos/lub-dealias-widen.scala [FAILED] at scala.sys.package$.error(package.scala:27) [20 lines elided] at org.apache.tools.ant.launch.Launcher.main(Launcher.java:109) Total time: 2 minutes 35 seconds After: BUILD FAILED /Users/jason/code/scala2/build.xml:1522: Test suite finished with 1 case failing: /Users/jason/code/scala2/test/files/pos/lub-dealias-widen.scala [FAILED] Total time: 2 minutes 34 seconds --- src/partest/scala/tools/partest/PartestTask.scala | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'src') diff --git a/src/partest/scala/tools/partest/PartestTask.scala b/src/partest/scala/tools/partest/PartestTask.scala index 0199400ada..dc40f9f81b 100644 --- a/src/partest/scala/tools/partest/PartestTask.scala +++ b/src/partest/scala/tools/partest/PartestTask.scala @@ -19,6 +19,7 @@ import java.lang.reflect.Method import org.apache.tools.ant.Task import org.apache.tools.ant.types.{Path, Reference, FileSet} import org.apache.tools.ant.types.Commandline.Argument +import scala.tools.ant.ScalaTask /** An Ant task to execute the Scala test suite (NSC). * @@ -54,7 +55,7 @@ import org.apache.tools.ant.types.Commandline.Argument * * @author Philippe Haller */ -class PartestTask extends Task with CompilationPathProperty { +class PartestTask extends Task with CompilationPathProperty with ScalaTask { def addConfiguredPosTests(input: FileSet) { posFiles = Some(input) @@ -406,7 +407,7 @@ class PartestTask extends Task with CompilationPathProperty { val allFailures = _results map (_._2) sum val allFailedPaths = _results flatMap (_._3) - def f = if (errorOnFailed && allFailures > 0) (sys error _) else log(_: String) + def f = if (errorOnFailed && allFailures > 0) buildError(_: String) else log(_: String) def s = if (allFailures > 1) "s" else "" val msg = if (allFailures > 0) -- cgit v1.2.3