From 54cfe6a5adb24da9f8ccc8eb9db4373679c827ab Mon Sep 17 00:00:00 2001 From: Olivier Blanvillain Date: Thu, 6 Jun 2019 12:22:24 +0200 Subject: [PATCH 1/4] Remove nonlocal returns from dotty's codebase --- .../classpath/VirtualDirectoryClassPath.scala | 5 +- .../tools/dotc/core/OrderingConstraint.scala | 9 +-- .../tools/dotc/printing/RefinedPrinter.scala | 71 ++++++++++--------- .../dotty/tools/dotc/transform/Splicer.scala | 8 ++- .../tools/dotc/util/SimpleIdentityMap.scala | 16 +++++ 5 files changed, 64 insertions(+), 45 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/classpath/VirtualDirectoryClassPath.scala b/compiler/src/dotty/tools/dotc/classpath/VirtualDirectoryClassPath.scala index ae285f80790b..7fd04fa36f72 100644 --- a/compiler/src/dotty/tools/dotc/classpath/VirtualDirectoryClassPath.scala +++ b/compiler/src/dotty/tools/dotc/classpath/VirtualDirectoryClassPath.scala @@ -13,12 +13,13 @@ case class VirtualDirectoryClassPath(dir: VirtualDirectory) extends ClassPath wi // From AbstractFileClassLoader private final def lookupPath(base: AbstractFile)(pathParts: Seq[String], directory: Boolean): AbstractFile = { var file: AbstractFile = base - for (dirPart <- pathParts.init) { + val dirParts = pathParts.init.toIterator + while (dirParts.hasNext) { + val dirPart = dirParts.next file = file.lookupName(dirPart, directory = true) if (file == null) return null } - file.lookupName(pathParts.last, directory = directory) } diff --git a/compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala b/compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala index 869c8330a5a3..740882a1cc63 100644 --- a/compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala +++ b/compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala @@ -471,13 +471,10 @@ class OrderingConstraint(private val boundsMap: ParamBounds, if entries(n).exists } yield poly.paramRefs(n) - def forallParams(p: TypeParamRef => Boolean): Boolean = { - boundsMap.foreachBinding { (poly, entries) => - for (i <- 0 until paramCount(entries)) - if (isBounds(entries(i)) && !p(poly.paramRefs(i))) return false + def forallParams(p: TypeParamRef => Boolean): Boolean = + boundsMap.forallBinding { (poly, entries) => + !0.until(paramCount(entries)).exists(i => isBounds(entries(i)) && !p(poly.paramRefs(i))) } - true - } def foreachParam(p: (TypeLambda, Int) => Unit): Unit = boundsMap.foreachBinding { (poly, entries) => diff --git a/compiler/src/dotty/tools/dotc/printing/RefinedPrinter.scala b/compiler/src/dotty/tools/dotc/printing/RefinedPrinter.scala index 33ae95b5fc7e..98b01f29979d 100644 --- a/compiler/src/dotty/tools/dotc/printing/RefinedPrinter.scala +++ b/compiler/src/dotty/tools/dotc/printing/RefinedPrinter.scala @@ -92,11 +92,12 @@ class RefinedPrinter(_ctx: Context) extends PlainPrinter(_ctx) { override def toTextRef(tp: SingletonType): Text = controlled { tp match { case tp: ThisType if !printDebug => - if (tp.cls.isAnonymousClass) return keywordStr("this") - if (tp.cls.is(ModuleClass)) return fullNameString(tp.cls.sourceModule) + if (tp.cls.isAnonymousClass) keywordStr("this") + if (tp.cls.is(ModuleClass)) fullNameString(tp.cls.sourceModule) + else super.toTextRef(tp) case _ => + super.toTextRef(tp) } - super.toTextRef(tp) } override def toTextPrefix(tp: Type): Text = controlled { @@ -105,15 +106,15 @@ class RefinedPrinter(_ctx: Context) extends PlainPrinter(_ctx) { else if (homogenizedView) isEmptyPrefix(sym) // drop and anonymous classes, but not scala, Predef. else isOmittablePrefix(sym) tp match { - case tp: ThisType => - if (isOmittable(tp.cls)) return "" + case tp: ThisType if isOmittable(tp.cls) => + "" case tp @ TermRef(pre, _) => val sym = tp.symbol - if (sym.isPackageObject && !homogenizedView) return toTextPrefix(pre) - if (isOmittable(sym)) return "" - case _ => + if (sym.isPackageObject && !homogenizedView) toTextPrefix(pre) + else if (isOmittable(sym)) "" + else super.toTextPrefix(tp) + case _ => super.toTextPrefix(tp) } - super.toTextPrefix(tp) } override protected def toTextParents(parents: List[Type]): Text = @@ -180,69 +181,69 @@ class RefinedPrinter(_ctx: Context) extends PlainPrinter(_ctx) { homogenize(tp) match { case tp @ AppliedType(tycon, args) => val cls = tycon.typeSymbol - if (tycon.isRepeatedParam) return toTextLocal(args.head) ~ "*" - if (defn.isFunctionClass(cls)) return toTextFunction(args, cls.name.isImplicitFunction, cls.name.isErasedFunction) - if (tp.tupleArity >= 2 && !printDebug) return toTextTuple(tp.tupleElementTypes) - if (isInfixType(tp)) { + if (tycon.isRepeatedParam) toTextLocal(args.head) ~ "*" + else if (defn.isFunctionClass(cls)) toTextFunction(args, cls.name.isImplicitFunction, cls.name.isErasedFunction) + else if (tp.tupleArity >= 2 && !printDebug) toTextTuple(tp.tupleElementTypes) + else if (isInfixType(tp)) { val l :: r :: Nil = args val opName = tyconName(tycon) - - return toTextInfixType(tyconName(tycon), l, r) { simpleNameString(tycon.typeSymbol) } + toTextInfixType(tyconName(tycon), l, r) { simpleNameString(tycon.typeSymbol) } } + else super.toText(tp) // Since RefinedPrinter, unlike PlainPrinter, can output right-associative type-operators, we must override handling // of AndType and OrType to account for associativity case AndType(tp1, tp2) => - return toTextInfixType(tpnme.raw.AMP, tp1, tp2) { toText(tpnme.raw.AMP) } + toTextInfixType(tpnme.raw.AMP, tp1, tp2) { toText(tpnme.raw.AMP) } case OrType(tp1, tp2) => - return toTextInfixType(tpnme.raw.BAR, tp1, tp2) { toText(tpnme.raw.BAR) } - + toTextInfixType(tpnme.raw.BAR, tp1, tp2) { toText(tpnme.raw.BAR) } case EtaExpansion(tycon) if !printDebug => - return toText(tycon) + toText(tycon) case tp: RefinedType if defn.isFunctionType(tp) => - return toTextDependentFunction(tp.refinedInfo.asInstanceOf[MethodType]) + toTextDependentFunction(tp.refinedInfo.asInstanceOf[MethodType]) case tp: TypeRef => if (tp.symbol.isAnonymousClass && !ctx.settings.uniqid.value) - return toText(tp.info) - if (tp.symbol.is(Param)) + toText(tp.info) + else if (tp.symbol.is(Param)) tp.prefix match { case pre: ThisType if pre.cls == tp.symbol.owner => - return nameString(tp.symbol) - case _ => + nameString(tp.symbol) + case _ => super.toText(tp) } + else super.toText(tp) case tp: ExprType => - return exprToText(tp) + exprToText(tp) case ErasedValueType(tycon, underlying) => - return "ErasedValueType(" ~ toText(tycon) ~ ", " ~ toText(underlying) ~ ")" + "ErasedValueType(" ~ toText(tycon) ~ ", " ~ toText(underlying) ~ ")" case tp: ClassInfo => - return toTextParents(tp.parents) ~~ "{...}" + toTextParents(tp.parents) ~~ "{...}" case JavaArrayType(elemtp) => - return toText(elemtp) ~ "[]" + toText(elemtp) ~ "[]" case tp: AnnotatedType if homogenizedView => // Positions of annotations in types are not serialized // (they don't need to because we keep the original type tree with // the original annotation anyway. Therefore, there will always be // one version of the annotation tree that has the correct positions). - return withoutPos(super.toText(tp)) + withoutPos(super.toText(tp)) case tp: SelectionProto => - return "?{ " ~ toText(tp.name) ~ + "?{ " ~ toText(tp.name) ~ (Str(" ") provided !tp.name.toSimpleName.last.isLetterOrDigit) ~ ": " ~ toText(tp.memberProto) ~ " }" case tp: ViewProto => - return toText(tp.argType) ~ " ?=>? " ~ toText(tp.resultType) + toText(tp.argType) ~ " ?=>? " ~ toText(tp.resultType) case tp @ FunProto(args, resultType) => val argsText = args match { case dummyTreeOfType(tp) :: Nil if !(tp isRef defn.NullClass) => "null: " ~ toText(tp) case _ => toTextGlobal(args, ", ") } - return "[applied to " ~ (Str("given ") provided tp.isContextualMethod) ~ (Str("erased ") provided tp.isErasedMethod) ~ "(" ~ argsText ~ ") returning " ~ toText(resultType) ~ "]" + "[applied to " ~ (Str("given ") provided tp.isContextualMethod) ~ (Str("erased ") provided tp.isErasedMethod) ~ "(" ~ argsText ~ ") returning " ~ toText(resultType) ~ "]" case IgnoredProto(ignored) => - return "?" ~ (("(ignored: " ~ toText(ignored) ~ ")") provided printDebug) + "?" ~ (("(ignored: " ~ toText(ignored) ~ ")") provided printDebug) case tp @ PolyProto(targs, resType) => - return "[applied to [" ~ toTextGlobal(targs, ", ") ~ "] returning " ~ toText(resType) + "[applied to [" ~ toTextGlobal(targs, ", ") ~ "] returning " ~ toText(resType) case _ => + super.toText(tp) } - super.toText(tp) } protected def exprToText(tp: ExprType): Text = diff --git a/compiler/src/dotty/tools/dotc/transform/Splicer.scala b/compiler/src/dotty/tools/dotc/transform/Splicer.scala index c3b17958c433..5b97d8df63c3 100644 --- a/compiler/src/dotty/tools/dotc/transform/Splicer.scala +++ b/compiler/src/dotty/tools/dotc/transform/Splicer.scala @@ -355,12 +355,16 @@ object Splicer { // Interpret `foo(j = x, i = y)` which it is expanded to // `val j$1 = x; val i$1 = y; foo(i = y, j = x)` case Block(stats, expr) => + var unexpected: Option[Result] = None val newEnv = stats.foldLeft(env)((accEnv, stat) => stat match { case stat: ValDef if stat.symbol.is(Synthetic) => accEnv.updated(stat.name, interpretTree(stat.rhs)(accEnv)) - case stat => return unexpectedTree(stat) + case stat => + if (unexpected.isEmpty) + unexpected = Some(unexpectedTree(stat)) + accEnv }) - interpretTree(expr)(newEnv) + unexpected.getOrElse(interpretTree(expr)(newEnv)) case NamedArg(_, arg) => interpretTree(arg) case Inlined(_, Nil, expansion) => interpretTree(expansion) diff --git a/compiler/src/dotty/tools/dotc/util/SimpleIdentityMap.scala b/compiler/src/dotty/tools/dotc/util/SimpleIdentityMap.scala index ce91764fb4ca..6d34760efd4f 100644 --- a/compiler/src/dotty/tools/dotc/util/SimpleIdentityMap.scala +++ b/compiler/src/dotty/tools/dotc/util/SimpleIdentityMap.scala @@ -13,6 +13,7 @@ abstract class SimpleIdentityMap[K <: AnyRef, +V >: Null <: AnyRef] extends (K = def contains(k: K): Boolean = apply(k) != null def mapValuesNow[V1 >: V <: AnyRef](f: (K, V1) => V1): SimpleIdentityMap[K, V1] def foreachBinding(f: (K, V) => Unit): Unit + def forallBinding(f: (K, V) => Boolean): Boolean def map2[T](f: (K, V) => T): List[T] = { val buf = new ListBuffer[T] foreachBinding((k, v) => buf += f(k, v)) @@ -37,6 +38,7 @@ object SimpleIdentityMap { def updated[V1 >: Null <: AnyRef](k: AnyRef, v: V1) = new Map1(k, v) def mapValuesNow[V1 >: Null <: AnyRef](f: (AnyRef, V1) => V1) = this def foreachBinding(f: (AnyRef, Null) => Unit) = () + def forallBinding(f: (AnyRef, Null) => Boolean) = true } def Empty[K <: AnyRef]: SimpleIdentityMap[K, Null] = myEmpty.asInstanceOf[SimpleIdentityMap[K, Null]] @@ -57,6 +59,7 @@ object SimpleIdentityMap { if (v1 eq w1) this else new Map1(k1, w1) } def foreachBinding(f: (K, V) => Unit): Unit = f(k1, v1) + def forallBinding(f: (K, V) => Boolean): Boolean = f(k1, v1) } class Map2[K <: AnyRef, +V >: Null <: AnyRef] (k1: K, v1: V, k2: K, v2: V) extends SimpleIdentityMap[K, V] { @@ -79,6 +82,7 @@ object SimpleIdentityMap { else new Map2(k1, w1, k2, w2) } def foreachBinding(f: (K, V) => Unit): Unit = { f(k1, v1); f(k2, v2) } + def forallBinding(f: (K, V) => Boolean): Boolean = f(k1, v1) && f(k2, v2) } class Map3[K <: AnyRef, +V >: Null <: AnyRef] (k1: K, v1: V, k2: K, v2: V, k3: K, v3: V) extends SimpleIdentityMap[K, V] { @@ -104,6 +108,7 @@ object SimpleIdentityMap { else new Map3(k1, w1, k2, w2, k3, w3) } def foreachBinding(f: (K, V) => Unit): Unit = { f(k1, v1); f(k2, v2); f(k3, v3) } + def forallBinding(f: (K, V) => Boolean): Boolean = f(k1, v1) && f(k2, v2) && f(k3, v3) } class Map4[K <: AnyRef, +V >: Null <: AnyRef] (k1: K, v1: V, k2: K, v2: V, k3: K, v3: V, k4: K, v4: V) extends SimpleIdentityMap[K, V] { @@ -132,6 +137,7 @@ object SimpleIdentityMap { else new Map4(k1, w1, k2, w2, k3, w3, k4, w4) } def foreachBinding(f: (K, V) => Unit): Unit = { f(k1, v1); f(k2, v2); f(k3, v3); f(k4, v4) } + def forallBinding(f: (K, V) => Boolean): Boolean = f(k1, v1) && f(k2, v2) && f(k3, v3) && f(k4, v4) } class MapMore[K <: AnyRef, +V >: Null <: AnyRef](bindings: Array[AnyRef]) extends SimpleIdentityMap[K, V] { @@ -223,5 +229,15 @@ object SimpleIdentityMap { i += 2 } } + + def forallBinding(f: (K, V) => Boolean): Boolean = { + var i = 0 + while (i < bindings.length) { + if (!f(key(i), value(i))) + return false + i += 2 + } + return true + } } } From 70e3537da1510fc1df6ca2456a943faf9dd8bb3b Mon Sep 17 00:00:00 2001 From: Olivier Blanvillain Date: Thu, 6 Jun 2019 12:23:19 +0200 Subject: [PATCH 2/4] Add library replacement to nonlocal returns --- .../scala/util/control/NonLocalReturns.scala | 34 +++++++++++++++++++ tests/run/nonlocal-return.scala | 16 +++++++++ 2 files changed, 50 insertions(+) create mode 100644 library/src/scala/util/control/NonLocalReturns.scala create mode 100644 tests/run/nonlocal-return.scala diff --git a/library/src/scala/util/control/NonLocalReturns.scala b/library/src/scala/util/control/NonLocalReturns.scala new file mode 100644 index 000000000000..bf8d481dcdcf --- /dev/null +++ b/library/src/scala/util/control/NonLocalReturns.scala @@ -0,0 +1,34 @@ +package scala.util.control + +/** Library implementation of nonlocal return. + * + * Usage: + * + * import scala.util.control.NonLocalReturns._ + * + * returning { ... throwReturn(x) ... } + */ +object NonLocalReturns { + class ReturnThrowable[T] extends ControlThrowable { + private var myResult: T = _ + def throwReturn(result: T): Nothing = { + myResult = result + throw this + } + def result: T = myResult + } + + /** Performs a nonlocal return by throwing an exception. */ + def throwReturn[T](result: T) given (returner: ReturnThrowable[T]): Nothing = + returner.throwReturn(result) + + /** Enable nonlocal returns in `op`. */ + def returning[T](op: given ReturnThrowable[T] => T): T = { + val returner = new ReturnThrowable[T] + try op given returner + catch { + case ex: ReturnThrowable[T] => + if (ex.eq(returner)) ex.result else throw ex + } + } +} diff --git a/tests/run/nonlocal-return.scala b/tests/run/nonlocal-return.scala new file mode 100644 index 000000000000..f52d86429620 --- /dev/null +++ b/tests/run/nonlocal-return.scala @@ -0,0 +1,16 @@ +import scala.util.control.NonLocalReturns._ + +object Test { + def has(xs: List[Int], elem: Int) = + returning { + for (x <- xs) + if (x == elem) throwReturn(true) + false + } + + def main(arg: Array[String]): Unit = { + assert(has(1 :: 2 :: Nil, 1)) + assert(has(1 :: 2 :: Nil, 2)) + assert(!has(1 :: 2 :: Nil, 3)) + } +} From 4c813c123f4da9cecccabd17596b92fabf141e97 Mon Sep 17 00:00:00 2001 From: Olivier Blanvillain Date: Thu, 6 Jun 2019 16:02:42 +0200 Subject: [PATCH 3/4] Fix #4240: Deprecate nonlocal returns :fire: --- .../src/dotty/tools/dotc/transform/NonLocalReturns.scala | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/NonLocalReturns.scala b/compiler/src/dotty/tools/dotc/transform/NonLocalReturns.scala index a1ecfb8b2986..4cafed59f8f6 100644 --- a/compiler/src/dotty/tools/dotc/transform/NonLocalReturns.scala +++ b/compiler/src/dotty/tools/dotc/transform/NonLocalReturns.scala @@ -86,6 +86,9 @@ class NonLocalReturns extends MiniPhase { } override def transformReturn(tree: Return)(implicit ctx: Context): Tree = - if (isNonLocalReturn(tree)) nonLocalReturnThrow(tree.expr, tree.from.symbol).withSpan(tree.span) - else tree + if (isNonLocalReturn(tree)) { + if (!ctx.scala2Mode) + ctx.strictWarning("Non local returns are deprecated; use scala.util.control.NonLocalReturns instead", tree.sourcePos) + nonLocalReturnThrow(tree.expr, tree.from.symbol).withSpan(tree.span) + } else tree } From 7f51a17ea6c53851409c0f7847e2abc5c4ff2188 Mon Sep 17 00:00:00 2001 From: Olivier Blanvillain Date: Fri, 7 Jun 2019 10:59:47 +0200 Subject: [PATCH 4/4] Add Dropped Feature doc page --- .../reference/dropped-features/nonlocal-returns.md | 14 ++++++++++++++ docs/sidebar.yml | 2 ++ 2 files changed, 16 insertions(+) create mode 100644 docs/docs/reference/dropped-features/nonlocal-returns.md diff --git a/docs/docs/reference/dropped-features/nonlocal-returns.md b/docs/docs/reference/dropped-features/nonlocal-returns.md new file mode 100644 index 000000000000..ef123cb090a1 --- /dev/null +++ b/docs/docs/reference/dropped-features/nonlocal-returns.md @@ -0,0 +1,14 @@ +--- +layout: doc-page +title: Deprecated: Nonlocal Returns +--- + +Returning from nested anonymous functions has been deprecated. Nonlocal returns are implemented by throwing and catching `scala.runtime.NonLocalReturnException`-s. This is rarely what is intendant by the programmer. It can be problematic because of the hidden performance cost of throwing and catching exceptions. Furthermore, it is a leaky implementation: a catch-all exception handler can intercept a `NonLocalReturnException`. + +A drop-in library replacement is provided in `scala.util.control.NonLocalReturns`: + +```scala +import scala.util.control.NonLocalReturns._ + +returning { ... throwReturn(x) ... } +``` diff --git a/docs/sidebar.yml b/docs/sidebar.yml index 8df98cfdf14f..879228ccfd70 100644 --- a/docs/sidebar.yml +++ b/docs/sidebar.yml @@ -159,6 +159,8 @@ sidebar: url: docs/reference/dropped-features/auto-apply.html - title: Weak Conformance url: docs/reference/dropped-features/weak-conformance.html + - title: Nonlocal Returns + url: docs/reference/dropped-features/nonlocal-returns.html - title: Contributing subsection: - title: Getting Started