From 94111e9b34b554f123ea2dfeff46001833738300 Mon Sep 17 00:00:00 2001 From: odersky Date: Mon, 2 Jan 2023 11:48:19 +0100 Subject: [PATCH 01/33] Introduce `boundary/break` control abstraction. The abstractions are intended to replace the `scala.util.control.Breaks` and `scala.uitl.control.NonLocalReturns`. They are simpler, safer, and more performant, since there is a new MiniPhase `DropBreaks` that rewrites local breaks to labeled returns, i.e. jumps. The abstractions are not experimental. This break from usual procedure is because we need to roll them out fast. Non local returns were just deprecated in 3.2, and we proposed `NonLocalReturns.{returning,throwReturn}` as an alternative. But these APIs were a mistake and should be deprecated. So rolling out boundary/break now counts as a bugfix. --- compiler/src/dotty/tools/dotc/Compiler.scala | 3 +- .../src/dotty/tools/dotc/ast/TreeInfo.scala | 6 + .../dotty/tools/dotc/core/Definitions.scala | 3 + .../src/dotty/tools/dotc/core/NameKinds.scala | 2 + .../src/dotty/tools/dotc/core/StdNames.scala | 3 + .../tools/dotc/transform/DropBreaks.scala | 213 ++++++++++++++++++ library/src/scala/util/boundary.scala | 40 ++++ library/src/scala/util/break.scala | 20 ++ .../scala/util/control/ControlException.scala | 18 ++ tests/run/break-opt.check | 2 + tests/run/break-opt.scala | 72 ++++++ tests/run/breaks.scala | 38 ++++ tests/run/rescue-boundary.scala | 76 +++++++ 13 files changed, 495 insertions(+), 1 deletion(-) create mode 100644 compiler/src/dotty/tools/dotc/transform/DropBreaks.scala create mode 100644 library/src/scala/util/boundary.scala create mode 100644 library/src/scala/util/break.scala create mode 100644 library/src/scala/util/control/ControlException.scala create mode 100644 tests/run/break-opt.check create mode 100644 tests/run/break-opt.scala create mode 100644 tests/run/breaks.scala create mode 100644 tests/run/rescue-boundary.scala diff --git a/compiler/src/dotty/tools/dotc/Compiler.scala b/compiler/src/dotty/tools/dotc/Compiler.scala index 375cdaaa2e94..b03953afb37c 100644 --- a/compiler/src/dotty/tools/dotc/Compiler.scala +++ b/compiler/src/dotty/tools/dotc/Compiler.scala @@ -88,7 +88,8 @@ class Compiler { new sjs.ExplicitJSClasses, // Make all JS classes explicit (Scala.js only) new ExplicitOuter, // Add accessors to outer classes from nested ones. new ExplicitSelf, // Make references to non-trivial self types explicit as casts - new StringInterpolatorOpt) :: // Optimizes raw and s and f string interpolators by rewriting them to string concatenations or formats + new StringInterpolatorOpt, // Optimizes raw and s and f string interpolators by rewriting them to string concatenations or formats + new DropBreaks) :: // Optimize local Break throws by rewriting them List(new PruneErasedDefs, // Drop erased definitions from scopes and simplify erased expressions new UninitializedDefs, // Replaces `compiletime.uninitialized` by `_` new InlinePatterns, // Remove placeholders of inlined patterns diff --git a/compiler/src/dotty/tools/dotc/ast/TreeInfo.scala b/compiler/src/dotty/tools/dotc/ast/TreeInfo.scala index ccc6e99737d4..eec32598de1a 100644 --- a/compiler/src/dotty/tools/dotc/ast/TreeInfo.scala +++ b/compiler/src/dotty/tools/dotc/ast/TreeInfo.scala @@ -102,6 +102,12 @@ trait TreeInfo[T <: Untyped] { self: Trees.Instance[T] => case _ => tree } + def stripTyped(tree: Tree): Tree = unsplice(tree) match + case Typed(expr, _) => + stripTyped(expr) + case _ => + tree + /** The number of arguments in an application */ def numArgs(tree: Tree): Int = unsplice(tree) match { case Apply(fn, args) => numArgs(fn) + args.length diff --git a/compiler/src/dotty/tools/dotc/core/Definitions.scala b/compiler/src/dotty/tools/dotc/core/Definitions.scala index 81a9d4db2e40..5503e9186ada 100644 --- a/compiler/src/dotty/tools/dotc/core/Definitions.scala +++ b/compiler/src/dotty/tools/dotc/core/Definitions.scala @@ -968,6 +968,9 @@ class Definitions { def TupledFunctionClass(using Context): ClassSymbol = TupledFunctionTypeRef.symbol.asClass def RuntimeTupleFunctionsModule(using Context): Symbol = requiredModule("scala.runtime.TupledFunctions") + @tu lazy val LabelClass: Symbol = requiredClass("scala.util.boundary.Label") + @tu lazy val BreakClass: Symbol = requiredClass("scala.util.boundary.Break") + @tu lazy val CapsModule: Symbol = requiredModule("scala.caps") @tu lazy val captureRoot: TermSymbol = CapsModule.requiredValue("*") @tu lazy val CapsUnsafeModule: Symbol = requiredModule("scala.caps.unsafe") diff --git a/compiler/src/dotty/tools/dotc/core/NameKinds.scala b/compiler/src/dotty/tools/dotc/core/NameKinds.scala index d121288a9cd8..2c968ab9446c 100644 --- a/compiler/src/dotty/tools/dotc/core/NameKinds.scala +++ b/compiler/src/dotty/tools/dotc/core/NameKinds.scala @@ -325,6 +325,8 @@ object NameKinds { val LocalOptInlineLocalObj: UniqueNameKind = new UniqueNameKind("ilo") + val BoundaryName: UniqueNameKind = new UniqueNameKind("boundary") + /** The kind of names of default argument getters */ val DefaultGetterName: NumberedNameKind = new NumberedNameKind(DEFAULTGETTER, "DefaultGetter") { def mkString(underlying: TermName, info: ThisInfo) = { diff --git a/compiler/src/dotty/tools/dotc/core/StdNames.scala b/compiler/src/dotty/tools/dotc/core/StdNames.scala index dff423fd0bb4..578852a06839 100644 --- a/compiler/src/dotty/tools/dotc/core/StdNames.scala +++ b/compiler/src/dotty/tools/dotc/core/StdNames.scala @@ -357,6 +357,7 @@ object StdNames { val Flag : N = "Flag" val Ident: N = "Ident" val Import: N = "Import" + val Label_this: N = "Label_this" val Literal: N = "Literal" val LiteralAnnotArg: N = "LiteralAnnotArg" val Matchable: N = "Matchable" @@ -511,10 +512,12 @@ object StdNames { val isInstanceOfPM: N = "$isInstanceOf$" val java: N = "java" val key: N = "key" + val label: N = "label" val lang: N = "lang" val language: N = "language" val length: N = "length" val lengthCompare: N = "lengthCompare" + val local: N = "local" val longHash: N = "longHash" val macroThis : N = "_this" val macroContext : N = "c" diff --git a/compiler/src/dotty/tools/dotc/transform/DropBreaks.scala b/compiler/src/dotty/tools/dotc/transform/DropBreaks.scala new file mode 100644 index 000000000000..f2ad70d0011e --- /dev/null +++ b/compiler/src/dotty/tools/dotc/transform/DropBreaks.scala @@ -0,0 +1,213 @@ +package dotty.tools +package dotc +package transform + +import ast.{Trees, tpd} +import core.* +import Decorators.* +import NameKinds.BoundaryName +import MegaPhase._ +import Types._, Contexts._, Flags._, DenotTransformers._ +import Symbols._, StdNames._, Trees._ +import util.Property +import Flags.MethodOrLazy + +object DropBreaks: + val name: String = "dropBreaks" + val description: String = "replace local Break throws by labeled returns" + + /** Usage data and other info associated with a Label symbol. + * @param goto the return-label to use for a labeled return. + * @param enclMeth the enclosing method + */ + class LabelUsage(val goto: TermSymbol, val enclMeth: Symbol): + /** The number of references to associated label that come from labeled returns */ + var returnRefs: Int = 0 + /** The number of other references to assocated label */ + var otherRefs: Int = 0 + + private val LabelUsages = new Property.Key[Map[Symbol, LabelUsage]] + private val LabelsShadowedByTry = new Property.Key[Set[Symbol]] + +/** Rewrites local Break throws to labeled returns. + * Drops `try` statements on breaks if no other uses of its label remain. + * A Break throw with a `Label` created by some enclosing boundary is replaced + * with a labeled return if + * + * - the throw and the boundary are in the same method, and + * - there is no try expression inside the boundary that encloses the throw. + */ +class DropBreaks extends MiniPhase: + import DropBreaks.* + + import tpd._ + + override def phaseName: String = DropBreaks.name + + override def description: String = DropBreaks.description + + override def runsAfterGroupsOf: Set[String] = Set(ElimByName.name) + // we want by-name parameters to be converted to closures + + private object LabelTry: + + object GuardedThrow: + + /** `(ex, local)` provided `expr` matches + * + * if ex.label.eq(local) then ex.value else throw ex + */ + def unapply(expr: Tree)(using Context): Option[(Symbol, Symbol)] = stripTyped(expr) match + case If( + Apply(Select(Select(ex: Ident, label), eq), (lbl @ Ident(local)) :: Nil), + Select(ex2: Ident, value), + Apply(throww, (ex3: Ident) :: Nil)) + if label == nme.label && eq == nme.eq && local == nme.local && value == nme.value + && throww.symbol == defn.throwMethod + && ex.symbol == ex2.symbol && ex.symbol == ex3.symbol => + Some((ex.symbol, lbl.symbol)) + case _ => + None + end GuardedThrow + + /** `(local, body)` provided `tree` matches + * + * try body + * catch case ex: Break => + * if ex.label.eq(local) then ex.value else throw ex + */ + def unapply(tree: Tree)(using Context): Option[(Symbol, Tree)] = stripTyped(tree) match + case Try(body, CaseDef(pat @ Bind(_, Typed(_, tpt)), EmptyTree, GuardedThrow(exc, local)) :: Nil, EmptyTree) + if tpt.tpe.isRef(defn.BreakClass) && exc == pat.symbol => + Some((local, body)) + case _ => + None + end LabelTry + + private object BreakBoundary: + + /** `(local, body)` provided `tree` matches + * + * { val local: Label[...] = ...; } + */ + def unapply(tree: Tree)(using Context): Option[(Symbol, Tree)] = tree match + case Block((vd @ ValDef(nme.local, _, _)) :: Nil, LabelTry(caughtAndRhs)) + if vd.symbol.info.isRef(defn.LabelClass) && vd.symbol == caughtAndRhs._1 => + Some(caughtAndRhs) + case _ => + None + end BreakBoundary + + private object BreakThrow: + + /** `(local, arg)` provided `tree` matches inlined + * + * val Label_this: ... = local + * throw new Break[...](Label_this, arg) + */ + def unapply(tree: Tree)(using Context): Option[(Symbol, Tree)] = tree match + case Inlined(_, + (vd @ ValDef(label_this1, _, id: Ident)):: Nil, + Apply(throww, Apply(constr, Inlined(_, _, Ident(label_this2)) :: arg :: Nil) :: Nil)) + if throww.symbol == defn.throwMethod + && label_this1 == nme.Label_this && label_this2 == nme.Label_this + && id.symbol.name == nme.local + && constr.symbol.isClassConstructor && constr.symbol.owner == defn.BreakClass => + Some((id.symbol, arg)) + case _ => + None + end BreakThrow + + /** The LabelUsage data associated with `lbl` in the current context */ + private def labelUsage(lbl: Symbol)(using Context): Option[LabelUsage] = + for + usesMap <- ctx.property(LabelUsages) + uses <- usesMap.get(lbl) + yield + uses + + /** If `tree` is a BreakBoundary, associate a fresh `LabelUsage` with its label. */ + override def prepareForBlock(tree: Block)(using Context): Context = tree match + case BreakBoundary(label, _) => + val mapSoFar = ctx.property(LabelUsages).getOrElse(Map.empty) + val goto = newSymbol(ctx.owner, BoundaryName.fresh(), Synthetic | Label, tree.tpe) + ctx.fresh.setProperty(LabelUsages, + mapSoFar.updated(label, LabelUsage(goto, ctx.owner.enclosingMethod))) + case _ => + ctx + + /** If `tree` is not a LabeledTry, include all enclosing labels in the + * `LabelsShadowedByTry` context property. This means that breaks to these + * labels will not be translated to labeled returns in the body of the try. + */ + override def prepareForTry(tree: Try)(using Context): Context = tree match + case LabelTry(_, _) => ctx + case _ => ctx.property(LabelUsages) match + case Some(usesMap) => + val setSoFar = ctx.property(LabelsShadowedByTry).getOrElse(Set.empty) + ctx.fresh.setProperty(LabelsShadowedByTry, setSoFar ++ usesMap.keysIterator) + case _ => ctx + + /** If `tree` is a BreakBoundary, transform it as follows: + * - Wrap it in a labeled block if its label has local uses + * - Drop the try/catch if its label has no other uses + */ + override def transformBlock(tree: Block)(using Context): Tree = tree match + case BreakBoundary(label, expr) => + val uses = ctx.property(LabelUsages).get(label) + val tree1 = + if uses.otherRefs > 1 then + // one non-local ref is always in the catch clause; this one does not count + tree + else + expr + report.log(i"trans boundary block $label // ${uses.returnRefs}, ${uses.otherRefs}") + if uses.returnRefs > 0 then Labeled(uses.goto, tree1) else tree1 + case _ => + tree + + /** Rewrite a BreakThrow + * + * val Label_this: ... = local + * throw new Break[...](Label_this, arg) + * + * where `local` is defined in the current method and is not included in + * LabeldShowedByTry to + * + * return[target] arg + * + * where `target` is the `goto` return label associated with `local`. + * Adjust associated ref counts accordingly. The local refcount is increased + * and the non-local refcount is decreased, since `local` the `Label_this` + * binding containing `local` is dropped. + */ + override def transformInlined(tree: Inlined)(using Context): Tree = tree match + case BreakThrow(lbl, arg) => + report.log(i"trans inlined $arg, ${arg.source}, ${ctx.outer.source}, ${tree.source}") + labelUsage(lbl) match + case Some(uses: LabelUsage) + if uses.enclMeth == ctx.owner.enclosingMethod + && !ctx.property(LabelsShadowedByTry).getOrElse(Set.empty).contains(lbl) + => + uses.otherRefs -= 1 + uses.returnRefs += 1 + cpy.Inlined(tree)(tree.call, Nil, + inContext(ctx.withSource(tree.expansion.source)) { + Return(arg, ref(uses.goto)).withSpan(arg.span) + }) + case _ => + tree + case _ => + tree + + /** If `tree` refers to an enclosing label, increase its non local recount. + * This increase is corrected in `transformInlined` if the reference turns + * out to be part of a BreakThrow to a local, non-shadowed label. + */ + override def transformIdent(tree: Ident)(using Context): Tree = + if tree.symbol.name == nme.local then + for uses <- labelUsage(tree.symbol) do + uses.otherRefs += 1 + tree + +end DropBreaks diff --git a/library/src/scala/util/boundary.scala b/library/src/scala/util/boundary.scala new file mode 100644 index 000000000000..a053ca522242 --- /dev/null +++ b/library/src/scala/util/boundary.scala @@ -0,0 +1,40 @@ +package scala.util +import control.ControlException + +/** A boundary that can be exited by `break` calls. + * `boundary` and `break` represent a unified and superior alternative for the + * `scala.util.control.NonLocalReturns` and `scala.util.control.Breaks` APIs. + * The main differences are: + * + * - Unified names: `boundary` to establish a scope, `break` to leave it. + * `break` can optionally return a value. + * - Integration with exceptions. `break`s are logically non-fatal exceptions. + * The `Break` exception class extends `ControlException` which is a regular + * `RuntimeException`, optimized so that stack trace generation is suppressed. + * - Better performance: breaks to enclosing scopes in the same method can + * be rwritten to jumps. + */ +object boundary: + + /** User code should call `break.apply` instead of throwing this exception + * directly. + */ + class Break[T](val label: Label[T], val value: T) extends ControlException + + /** Labels are targets indicating which boundary will be exited by a `break`. + */ + class Label[T]: + transparent inline def break(value: T): Nothing = throw Break(this, value) + + /** Run `body` with freshly generated label as implicit argument. Catch any + * breaks associated with that label and return their results instead of + * `body`'s result. + */ + transparent inline def apply[T <: R, R](inline body: Label[T] ?=> R): R = + val local = Label[T]() + try body(using local) + catch case ex: Break[T] @unchecked => + if ex.label eq local then ex.value + else throw ex + +end boundary diff --git a/library/src/scala/util/break.scala b/library/src/scala/util/break.scala new file mode 100644 index 000000000000..fe547bb15536 --- /dev/null +++ b/library/src/scala/util/break.scala @@ -0,0 +1,20 @@ +package scala.util + +/** This object has two apply methods that abort the current computation + * up to an enclosing `boundary` call. + */ +object break: + + /** Abort current computation and instead return `value` as the value of + * the enclosing `boundary` call that created `label`. + */ + transparent inline def apply[T](value: T)(using l: boundary.Label[T]): Nothing = + l.break(value) + + /** Abort current computation and instead continue after the `boundary` call that + * created `label`. + */ + transparent inline def apply()(using l: boundary.Label[Unit]): Nothing = + apply(()) + +end break \ No newline at end of file diff --git a/library/src/scala/util/control/ControlException.scala b/library/src/scala/util/control/ControlException.scala new file mode 100644 index 000000000000..c042a3757814 --- /dev/null +++ b/library/src/scala/util/control/ControlException.scala @@ -0,0 +1,18 @@ +package scala.util.control + +/** A parent class for throwable objects intended for flow control. + * + * Instances of ControlException don't record a stacktrace and are therefore + * much more efficient to throw than normal exceptions. + * + * Unlike `ControlThrowable`, `ControlException` is a regular `RuntimeException` + * that is supposed to be handled like any other exception. + * + * Instances of `ControlException` should not normally have a cause. + * Legacy subclasses may set a cause using `initCause`. + */ +abstract class ControlException(message: String | Null) extends Throwable( + message, /*cause*/ null, /*enableSuppression=*/ false, /*writableStackTrace*/ false): + + def this() = this(message = null) + diff --git a/tests/run/break-opt.check b/tests/run/break-opt.check new file mode 100644 index 000000000000..ac1be39bbe0a --- /dev/null +++ b/tests/run/break-opt.check @@ -0,0 +1,2 @@ +done +done diff --git a/tests/run/break-opt.scala b/tests/run/break-opt.scala new file mode 100644 index 000000000000..397a27e131fc --- /dev/null +++ b/tests/run/break-opt.scala @@ -0,0 +1,72 @@ +import scala.util.* + +object breakOpt: + + var zero = 0 + + def test1(x: Int): Int = + boundary: + if x < 0 then break(zero) + x + + def test2(xx: Int, xs: List[Int]): Int = + boundary: + if xx < 0 then break(zero) + xs.map: y => + if y < 0 then break(zero) + y + xx + xs.sum + + def test3(xx: Int, xs: List[Int]): Int = + def cond[T](p: Boolean, x: => T, y: => T): T = + if p then x else y + boundary: + cond(true, { if xx < 0 then break(zero); xx }, xx) + + def test3a(xx: Int, xs: List[Int]): Int = + inline def cond[T](p: Boolean, inline x: T, y: => T): T = + if p then x else y + boundary: + cond(true, { if xx < 0 then break(zero); xx }, xx) + + def test4(x: Int): Int = + boundary: + try + if x < 0 then break(zero) + boundary: + if x == 0 then break(-1) + x + finally + println("done") + + def test5(x: Int): Int = + boundary: lab1 ?=> + if x < 0 then break(zero) + boundary: + if x == 0 then break(-1) + if x > 0 then break(+1)(using lab1) + x + + def test6(x0: Int): Int = + var x = x0 + var y = x + boundary: + while true do + y = y * x + x -= 1 + if x == 0 then break() + y + +@main def Test = + import breakOpt.* + assert(test1(0) == 0) + assert(test1(-1) == 0) + assert(test2(1, List(1, 2, 3)) == 7) + assert(test2(-1, List(1, 2, 3)) == 0) + assert(test2(1, List(1, -2, 3)) == 0) + test4(1) + test4(-1) + assert(test5(2) == 1) + assert(test6(3) == 18) + + diff --git a/tests/run/breaks.scala b/tests/run/breaks.scala new file mode 100644 index 000000000000..d6bddd881281 --- /dev/null +++ b/tests/run/breaks.scala @@ -0,0 +1,38 @@ +import scala.util.* +import collection.mutable.ListBuffer + +object Test { + def has(xs: List[Int], elem: Int) = + boundary: + for x <- xs do + if x == elem then break(true) + false + + def takeUntil(xs: List[Int], elem: Int) = + boundary: + var buf = new ListBuffer[Int] + for x <- xs yield + if x == elem then break(buf.toList) + buf += x + x + + trait Animal + object Dog extends Animal + object Cat extends Animal + + def animal(arg: Int): Animal = + boundary: + if arg < 0 then break(Dog) + Cat + + def main(arg: Array[String]): Unit = { + assert(has(1 :: 2 :: Nil, 1)) + assert(has(1 :: 2 :: Nil, 2)) + assert(!has(1 :: 2 :: Nil, 3)) + assert(animal(1) == Cat) + assert(animal(-1) == Dog) + + assert(has(List(1, 2, 3), 2)) + assert(takeUntil(List(1, 2, 3), 3) == List(1, 2)) + } +} \ No newline at end of file diff --git a/tests/run/rescue-boundary.scala b/tests/run/rescue-boundary.scala new file mode 100644 index 000000000000..7d472aacaf1d --- /dev/null +++ b/tests/run/rescue-boundary.scala @@ -0,0 +1,76 @@ +import scala.util.control.{ControlException, NonFatal} +import scala.util.* + +object lib: + extension [T](op: => T) inline def rescue (fallback: => T) = + try op + catch + case ex: ControlException => throw ex + case NonFatal(_) => fallback + + extension [T, E <: Throwable](op: => T) inline def rescue (fallback: PartialFunction[E, T]) = + try op + catch + case ex: E => + // user should never match `ReturnThrowable`, which breaks semantics of non-local return + if fallback.isDefinedAt(ex) && !ex.isInstanceOf[boundary.Break[_]] then fallback(ex) else throw ex +end lib + +import lib.* + +@main def Test = { + assert((9 / 1 rescue 1) == 9) + assert((9 / 0 rescue 1) == 1) + assert(((9 / 0 rescue { case ex: NullPointerException => 5 }) rescue 10) == 10) + assert(((9 / 0 rescue { case ex: ArithmeticException => 5 }) rescue 10) == 5) + + assert( + { + 9 / 0 rescue { + case ex: NullPointerException => 4 + case ex: ArithmeticException => 3 + } + } == 3 + ) + + (9 / 0) rescue { case ex: ArithmeticException => 4 } + + assert( + { + { + val a = 9 / 0 rescue { + case ex: NullPointerException => 4 + } + a * a + } rescue { + case ex: ArithmeticException => 3 + } + } == 3 + ) + + assert(foo(10) == 40) + assert(bar(10) == 40) + + // should not catch fatal errors + assert( + try { { throw new OutOfMemoryError(); true } rescue false } + catch { case _: OutOfMemoryError => true } + ) + + // should catch any errors specified, including fatal errors + assert( + try { { throw new OutOfMemoryError(); true } rescue { case _: OutOfMemoryError => true } } + catch { case _: OutOfMemoryError => false } + ) + + // should not catch NonLocalReturns + def foo(x: Int): Int = boundary { + { break[Int](4 * x) : Int } rescue 10 + } + + // should catch specified exceptions, but not NonLocalReturn + def bar(x: Int): Int = boundary { + { break[Int](4 * x) : Int } rescue { case _ => 10 } + } + +} From a7dc51ffbf7ecb4b0224e86b7213b0c30f95b08b Mon Sep 17 00:00:00 2001 From: odersky Date: Mon, 2 Jan 2023 20:28:00 +0100 Subject: [PATCH 02/33] Make `boundary.apply` a regular inline Drop the `transparent` in order to curcumvent #16609 --- compiler/src/dotty/tools/dotc/transform/DropBreaks.scala | 2 +- library/src/scala/util/boundary.scala | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/DropBreaks.scala b/compiler/src/dotty/tools/dotc/transform/DropBreaks.scala index f2ad70d0011e..fcd3febbf8c3 100644 --- a/compiler/src/dotty/tools/dotc/transform/DropBreaks.scala +++ b/compiler/src/dotty/tools/dotc/transform/DropBreaks.scala @@ -90,7 +90,7 @@ class DropBreaks extends MiniPhase: * * { val local: Label[...] = ...; } */ - def unapply(tree: Tree)(using Context): Option[(Symbol, Tree)] = tree match + def unapply(tree: Tree)(using Context): Option[(Symbol, Tree)] = stripTyped(tree) match case Block((vd @ ValDef(nme.local, _, _)) :: Nil, LabelTry(caughtAndRhs)) if vd.symbol.info.isRef(defn.LabelClass) && vd.symbol == caughtAndRhs._1 => Some(caughtAndRhs) diff --git a/library/src/scala/util/boundary.scala b/library/src/scala/util/boundary.scala index a053ca522242..bb2cc1a96b60 100644 --- a/library/src/scala/util/boundary.scala +++ b/library/src/scala/util/boundary.scala @@ -30,7 +30,7 @@ object boundary: * breaks associated with that label and return their results instead of * `body`'s result. */ - transparent inline def apply[T <: R, R](inline body: Label[T] ?=> R): R = + inline def apply[T <: R, R](inline body: Label[T] ?=> R): R = val local = Label[T]() try body(using local) catch case ex: Break[T] @unchecked => From 5aa4b9078c39fc71d3a25c540a5c392afa23b1a9 Mon Sep 17 00:00:00 2001 From: odersky Date: Wed, 4 Jan 2023 09:13:24 +0100 Subject: [PATCH 03/33] Fix parent of ControlException --- library/src/scala/util/control/ControlException.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/src/scala/util/control/ControlException.scala b/library/src/scala/util/control/ControlException.scala index c042a3757814..eb0cfa63e96b 100644 --- a/library/src/scala/util/control/ControlException.scala +++ b/library/src/scala/util/control/ControlException.scala @@ -11,7 +11,7 @@ package scala.util.control * Instances of `ControlException` should not normally have a cause. * Legacy subclasses may set a cause using `initCause`. */ -abstract class ControlException(message: String | Null) extends Throwable( +abstract class ControlException(message: String | Null) extends RuntimeException( message, /*cause*/ null, /*enableSuppression=*/ false, /*writableStackTrace*/ false): def this() = this(message = null) From 01ac5e03178845c025a2b15d3ba45899b7dcd6e8 Mon Sep 17 00:00:00 2001 From: odersky Date: Wed, 4 Jan 2023 09:43:21 +0100 Subject: [PATCH 04/33] Deprecate `NonLocalReturns` API Change the recommendation in the warning about non-local returns accordingly. Still to do: A PR against scala/scala that deprecates scala.util.control.Breaks. --- .../dotty/tools/dotc/transform/NonLocalReturns.scala | 2 +- library/src/scala/util/control/NonLocalReturns.scala | 12 ++++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/transform/NonLocalReturns.scala b/compiler/src/dotty/tools/dotc/transform/NonLocalReturns.scala index ddf858994220..9fad2289da54 100644 --- a/compiler/src/dotty/tools/dotc/transform/NonLocalReturns.scala +++ b/compiler/src/dotty/tools/dotc/transform/NonLocalReturns.scala @@ -97,7 +97,7 @@ class NonLocalReturns extends MiniPhase { override def transformReturn(tree: Return)(using Context): Tree = if isNonLocalReturn(tree) then report.gradualErrorOrMigrationWarning( - em"Non local returns are no longer supported; use scala.util.control.NonLocalReturns instead", + em"Non local returns are no longer supported; use `boundary` and `break` in `scala.util` instead", tree.srcPos, warnFrom = `3.2`, errorFrom = future) diff --git a/library/src/scala/util/control/NonLocalReturns.scala b/library/src/scala/util/control/NonLocalReturns.scala index c32e0ff16457..7c2c6584c489 100644 --- a/library/src/scala/util/control/NonLocalReturns.scala +++ b/library/src/scala/util/control/NonLocalReturns.scala @@ -7,8 +7,18 @@ package scala.util.control * import scala.util.control.NonLocalReturns.* * * returning { ... throwReturn(x) ... } + * + * This API has been deprecated. Its functionality is better served by + * + * - `scala.util.boundary` in place of `returning` + * - `scala.util.break` in place of `throwReturn` + * + * The new abstractions work with plain `RuntimeExceptions` and are more + * performant, since returns within the scope of the same method can be + * rewritten by the compiler to jumps. */ object NonLocalReturns { + @deprecated("Use scala.util.boundary.Break instead", "3.3") class ReturnThrowable[T] extends ControlThrowable { private var myResult: T = _ def throwReturn(result: T): Nothing = { @@ -19,10 +29,12 @@ object NonLocalReturns { } /** Performs a nonlocal return by throwing an exception. */ + @deprecated("Use scala.util.break and scala.util.boundary instead", "3.3") def throwReturn[T](result: T)(using returner: ReturnThrowable[? >: T]): Nothing = returner.throwReturn(result) /** Enable nonlocal returns in `op`. */ + @deprecated("Use scala.util.boundary and scala.util.break instead", "3.3") def returning[T](op: ReturnThrowable[T] ?=> T): T = { val returner = new ReturnThrowable[T] try op(using returner) From d1d530153ff9d0a880e18acaf75d220fab985ece Mon Sep 17 00:00:00 2001 From: odersky Date: Tue, 10 Jan 2023 10:34:47 +0100 Subject: [PATCH 05/33] Add loop test with exit and continue --- tests/run/loops.scala | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) create mode 100644 tests/run/loops.scala diff --git a/tests/run/loops.scala b/tests/run/loops.scala new file mode 100644 index 000000000000..d1474d06f3a1 --- /dev/null +++ b/tests/run/loops.scala @@ -0,0 +1,37 @@ +import scala.util.{boundary, break} + +object loop: + + // We could use a boolean instead, but with `Ctrl` label types in using clauses are + // more specific. + enum Ctrl: + case Exit, Continue + + inline def apply(inline op: boundary.Label[Ctrl] ?=> Unit) = + while boundary { op; Ctrl.Continue } == Ctrl.Continue do () + + inline def exit()(using boundary.Label[Ctrl]): Unit = + break(Ctrl.Exit) + + inline def continue()(using boundary.Label[Ctrl]): Unit = + break(Ctrl.Continue) +end loop + +def testLoop(xs: List[Int]) = + var current = xs + var sum = 0 + loop: + if current.isEmpty then loop.exit() + val hd = current.head + current = current.tail + if hd == 0 then loop.exit() + if hd < 0 then loop.continue() + sum += hd + sum + +@main def Test = + assert(testLoop(List(1, 2, 3, -2, 4, -3, 0, 1, 2, 3)) == 10) + assert(testLoop(List()) == 0) + assert(testLoop(List(-2, -3, 0, 1)) == 0) + + From 46102015dd31c7c75cb9ccb631182b61ae179345 Mon Sep 17 00:00:00 2001 From: odersky Date: Wed, 11 Jan 2023 15:02:11 +0100 Subject: [PATCH 06/33] Make `break` non-inlined functions `DropBreaks` now detects the calls to `break` instead of their inline expansion. Also: Make `Break`'s constructor private. --- .../dotty/tools/dotc/core/Definitions.scala | 1 + .../tools/dotc/transform/DropBreaks.scala | 69 ++++++++----------- library/src/scala/util/boundary.scala | 4 +- library/src/scala/util/break.scala | 4 +- 4 files changed, 33 insertions(+), 45 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/Definitions.scala b/compiler/src/dotty/tools/dotc/core/Definitions.scala index 5503e9186ada..db982dc3fd88 100644 --- a/compiler/src/dotty/tools/dotc/core/Definitions.scala +++ b/compiler/src/dotty/tools/dotc/core/Definitions.scala @@ -970,6 +970,7 @@ class Definitions { @tu lazy val LabelClass: Symbol = requiredClass("scala.util.boundary.Label") @tu lazy val BreakClass: Symbol = requiredClass("scala.util.boundary.Break") + @tu lazy val breakModule: Symbol = requiredModule("scala.util.break") @tu lazy val CapsModule: Symbol = requiredModule("scala.caps") @tu lazy val captureRoot: TermSymbol = CapsModule.requiredValue("*") diff --git a/compiler/src/dotty/tools/dotc/transform/DropBreaks.scala b/compiler/src/dotty/tools/dotc/transform/DropBreaks.scala index fcd3febbf8c3..585f33efc7ab 100644 --- a/compiler/src/dotty/tools/dotc/transform/DropBreaks.scala +++ b/compiler/src/dotty/tools/dotc/transform/DropBreaks.scala @@ -10,6 +10,7 @@ import MegaPhase._ import Types._, Contexts._, Flags._, DenotTransformers._ import Symbols._, StdNames._, Trees._ import util.Property +import Constants.Constant import Flags.MethodOrLazy object DropBreaks: @@ -98,26 +99,6 @@ class DropBreaks extends MiniPhase: None end BreakBoundary - private object BreakThrow: - - /** `(local, arg)` provided `tree` matches inlined - * - * val Label_this: ... = local - * throw new Break[...](Label_this, arg) - */ - def unapply(tree: Tree)(using Context): Option[(Symbol, Tree)] = tree match - case Inlined(_, - (vd @ ValDef(label_this1, _, id: Ident)):: Nil, - Apply(throww, Apply(constr, Inlined(_, _, Ident(label_this2)) :: arg :: Nil) :: Nil)) - if throww.symbol == defn.throwMethod - && label_this1 == nme.Label_this && label_this2 == nme.Label_this - && id.symbol.name == nme.local - && constr.symbol.isClassConstructor && constr.symbol.owner == defn.BreakClass => - Some((id.symbol, arg)) - case _ => - None - end BreakThrow - /** The LabelUsage data associated with `lbl` in the current context */ private def labelUsage(lbl: Symbol)(using Context): Option[LabelUsage] = for @@ -166,13 +147,29 @@ class DropBreaks extends MiniPhase: case _ => tree - /** Rewrite a BreakThrow + private def isBreak(sym: Symbol)(using Context): Boolean = + sym.name == nme.apply && sym.owner == defn.breakModule.moduleClass + + private def transformBreak(tree: Tree, arg: Tree, lbl: Symbol)(using Context): Tree = + report.log(i"transform break $tree/$arg/$lbl") + labelUsage(lbl) match + case Some(uses: LabelUsage) + if uses.enclMeth == ctx.owner.enclosingMethod + && !ctx.property(LabelsShadowedByTry).getOrElse(Set.empty).contains(lbl) + => + uses.otherRefs -= 1 + uses.returnRefs += 1 + Return(arg, ref(uses.goto)).withSpan(arg.span) + case _ => + tree + + + /** Rewrite a break call * - * val Label_this: ... = local - * throw new Break[...](Label_this, arg) + * break.apply[...](value)(using lbl) * - * where `local` is defined in the current method and is not included in - * LabeldShowedByTry to + * where `lbl` is a label defined in the current method and is not included in + * LabelsShadowedByTry to * * return[target] arg * @@ -181,22 +178,12 @@ class DropBreaks extends MiniPhase: * and the non-local refcount is decreased, since `local` the `Label_this` * binding containing `local` is dropped. */ - override def transformInlined(tree: Inlined)(using Context): Tree = tree match - case BreakThrow(lbl, arg) => - report.log(i"trans inlined $arg, ${arg.source}, ${ctx.outer.source}, ${tree.source}") - labelUsage(lbl) match - case Some(uses: LabelUsage) - if uses.enclMeth == ctx.owner.enclosingMethod - && !ctx.property(LabelsShadowedByTry).getOrElse(Set.empty).contains(lbl) - => - uses.otherRefs -= 1 - uses.returnRefs += 1 - cpy.Inlined(tree)(tree.call, Nil, - inContext(ctx.withSource(tree.expansion.source)) { - Return(arg, ref(uses.goto)).withSpan(arg.span) - }) - case _ => - tree + override def transformApply(tree: Apply)(using Context): Tree = tree match + case Apply(Apply(fn, args), (id: Ident) :: Nil) if isBreak(fn.symbol) => + val arg = (args: @unchecked) match + case arg :: Nil => arg + case Nil => Literal(Constant(())).withSpan(tree.span) + transformBreak(tree, arg, id.symbol) case _ => tree diff --git a/library/src/scala/util/boundary.scala b/library/src/scala/util/boundary.scala index bb2cc1a96b60..e9a20fbaa50c 100644 --- a/library/src/scala/util/boundary.scala +++ b/library/src/scala/util/boundary.scala @@ -19,12 +19,12 @@ object boundary: /** User code should call `break.apply` instead of throwing this exception * directly. */ - class Break[T](val label: Label[T], val value: T) extends ControlException + class Break[T] private[boundary](val label: Label[T], val value: T) extends ControlException /** Labels are targets indicating which boundary will be exited by a `break`. */ class Label[T]: - transparent inline def break(value: T): Nothing = throw Break(this, value) + def break(value: T): Nothing = throw Break(this, value) /** Run `body` with freshly generated label as implicit argument. Catch any * breaks associated with that label and return their results instead of diff --git a/library/src/scala/util/break.scala b/library/src/scala/util/break.scala index fe547bb15536..5c3bccfa93d7 100644 --- a/library/src/scala/util/break.scala +++ b/library/src/scala/util/break.scala @@ -8,13 +8,13 @@ object break: /** Abort current computation and instead return `value` as the value of * the enclosing `boundary` call that created `label`. */ - transparent inline def apply[T](value: T)(using l: boundary.Label[T]): Nothing = + def apply[T](value: T)(using l: boundary.Label[T]): Nothing = l.break(value) /** Abort current computation and instead continue after the `boundary` call that * created `label`. */ - transparent inline def apply()(using l: boundary.Label[Unit]): Nothing = + def apply()(using l: boundary.Label[Unit]): Nothing = apply(()) end break \ No newline at end of file From 04e7bc1ff5ec5b12cc1f5dcaf6b6696369dbccff Mon Sep 17 00:00:00 2001 From: odersky Date: Wed, 11 Jan 2023 19:36:03 +0100 Subject: [PATCH 07/33] Suppress returns that cross different stack sizes. This is needed to avoid verify errors --- .../tools/dotc/transform/DropBreaks.scala | 112 +++++++++++------- .../dotty/tools/dotc/transform/LiftTry.scala | 29 +---- .../dotc/transform/RecordStackChange.scala | 41 +++++++ tests/run/break-opt.scala | 17 +++ 4 files changed, 132 insertions(+), 67 deletions(-) create mode 100644 compiler/src/dotty/tools/dotc/transform/RecordStackChange.scala diff --git a/compiler/src/dotty/tools/dotc/transform/DropBreaks.scala b/compiler/src/dotty/tools/dotc/transform/DropBreaks.scala index 585f33efc7ab..ae3412305638 100644 --- a/compiler/src/dotty/tools/dotc/transform/DropBreaks.scala +++ b/compiler/src/dotty/tools/dotc/transform/DropBreaks.scala @@ -28,7 +28,7 @@ object DropBreaks: var otherRefs: Int = 0 private val LabelUsages = new Property.Key[Map[Symbol, LabelUsage]] - private val LabelsShadowedByTry = new Property.Key[Set[Symbol]] + private val ShadowedLabels = new Property.Key[Set[Symbol]] /** Rewrites local Break throws to labeled returns. * Drops `try` statements on breaks if no other uses of its label remain. @@ -38,7 +38,7 @@ object DropBreaks: * - the throw and the boundary are in the same method, and * - there is no try expression inside the boundary that encloses the throw. */ -class DropBreaks extends MiniPhase: +class DropBreaks extends MiniPhase, RecordStackChange: import DropBreaks.* import tpd._ @@ -99,6 +99,31 @@ class DropBreaks extends MiniPhase: None end BreakBoundary + private object Break: + + private def isBreak(sym: Symbol)(using Context): Boolean = + sym.name == nme.apply && sym.owner == defn.breakModule.moduleClass + + /** `(local, arg)` provided `tree` matches + * + * break[...](arg)(local) + * + * or `(local, ())` provided `tree` matches + * + * break()(local) + */ + def unapply(tree: Tree)(using Context): Option[(Symbol, Tree)] = tree match + case Apply(Apply(fn, args), id :: Nil) + if isBreak(fn.symbol) => + stripInlined(id) match + case id: Ident => + val arg = (args: @unchecked) match + case arg :: Nil => arg + case Nil => Literal(Constant(())).withSpan(tree.span) + Some((id.symbol, arg)) + case _ => None + case _ => None + /** The LabelUsage data associated with `lbl` in the current context */ private def labelUsage(lbl: Symbol)(using Context): Option[LabelUsage] = for @@ -117,18 +142,34 @@ class DropBreaks extends MiniPhase: case _ => ctx - /** If `tree` is not a LabeledTry, include all enclosing labels in the - * `LabelsShadowedByTry` context property. This means that breaks to these - * labels will not be translated to labeled returns in the body of the try. + /** Include all enclosing labels in the `ShadowedLabels` context property. + * This means that breaks to these labels will not be translated to labeled + * returns while this context is valid. */ - override def prepareForTry(tree: Try)(using Context): Context = tree match - case LabelTry(_, _) => ctx - case _ => ctx.property(LabelUsages) match + private def shadowLabels(using Context): Context = + ctx.property(LabelUsages) match case Some(usesMap) => - val setSoFar = ctx.property(LabelsShadowedByTry).getOrElse(Set.empty) - ctx.fresh.setProperty(LabelsShadowedByTry, setSoFar ++ usesMap.keysIterator) + val setSoFar = ctx.property(ShadowedLabels).getOrElse(Set.empty) + ctx.fresh.setProperty(ShadowedLabels, setSoFar ++ usesMap.keysIterator) case _ => ctx + /** Need to suppress labeled returns if the stack can change between + * source and target of the jump + */ + protected def stackChange(using Context) = shadowLabels + + /** Need to suppress labeled returns if there is an intervening try + */ + override def prepareForTry(tree: Try)(using Context): Context = tree match + case LabelTry(_, _) => ctx + case _ => shadowLabels + + override def prepareForApply(tree: Apply)(using Context): Context = tree match + case Break(_, _) => ctx + case _ => stackChange + + // other stack changing operations are handled in RecordStackChange + /** If `tree` is a BreakBoundary, transform it as follows: * - Wrap it in a labeled block if its label has local uses * - Drop the try/catch if its label has no other uses @@ -147,29 +188,9 @@ class DropBreaks extends MiniPhase: case _ => tree - private def isBreak(sym: Symbol)(using Context): Boolean = - sym.name == nme.apply && sym.owner == defn.breakModule.moduleClass - - private def transformBreak(tree: Tree, arg: Tree, lbl: Symbol)(using Context): Tree = - report.log(i"transform break $tree/$arg/$lbl") - labelUsage(lbl) match - case Some(uses: LabelUsage) - if uses.enclMeth == ctx.owner.enclosingMethod - && !ctx.property(LabelsShadowedByTry).getOrElse(Set.empty).contains(lbl) - => - uses.otherRefs -= 1 - uses.returnRefs += 1 - Return(arg, ref(uses.goto)).withSpan(arg.span) - case _ => - tree - - - /** Rewrite a break call - * - * break.apply[...](value)(using lbl) - * + /** Rewrite a break with argument `arg` and label `lbl` * where `lbl` is a label defined in the current method and is not included in - * LabelsShadowedByTry to + * ShadowedLabels to * * return[target] arg * @@ -179,22 +200,29 @@ class DropBreaks extends MiniPhase: * binding containing `local` is dropped. */ override def transformApply(tree: Apply)(using Context): Tree = tree match - case Apply(Apply(fn, args), (id: Ident) :: Nil) if isBreak(fn.symbol) => - val arg = (args: @unchecked) match - case arg :: Nil => arg - case Nil => Literal(Constant(())).withSpan(tree.span) - transformBreak(tree, arg, id.symbol) - case _ => - tree + case Break(lbl, arg) => + labelUsage(lbl) match + case Some(uses: LabelUsage) + if uses.enclMeth == ctx.owner.enclosingMethod + && !ctx.property(ShadowedLabels).getOrElse(Set.empty).contains(lbl) + => + uses.otherRefs -= 1 + uses.returnRefs += 1 + Return(arg, ref(uses.goto)).withSpan(arg.span) + case _ => tree + case _ => tree /** If `tree` refers to an enclosing label, increase its non local recount. * This increase is corrected in `transformInlined` if the reference turns * out to be part of a BreakThrow to a local, non-shadowed label. */ override def transformIdent(tree: Ident)(using Context): Tree = - if tree.symbol.name == nme.local then - for uses <- labelUsage(tree.symbol) do - uses.otherRefs += 1 + for uses <- labelUsage(tree.symbol) do + uses.otherRefs += 1 tree + //override def transformReturn(tree: Return)(using Context): Tree = + // if !tree.from.isEmpty && tree.expr.tpe.isExactlyNothing then tree.expr + // else tree + end DropBreaks diff --git a/compiler/src/dotty/tools/dotc/transform/LiftTry.scala b/compiler/src/dotty/tools/dotc/transform/LiftTry.scala index 6acb1013d509..2c31d981d2fc 100644 --- a/compiler/src/dotty/tools/dotc/transform/LiftTry.scala +++ b/compiler/src/dotty/tools/dotc/transform/LiftTry.scala @@ -6,8 +6,8 @@ import core.DenotTransformers._ import core.Symbols._ import core.Contexts._ import core.Types._ -import core.Flags._ import core.Decorators._ +import core.Flags._ import core.NameKinds.LiftedTreeName import NonLocalReturns._ import util.Store @@ -27,7 +27,7 @@ import util.Store * after an exception, so the fact that values on the stack are 'lost' does not matter * (copied from https://github.com/scala/scala/pull/922). */ -class LiftTry extends MiniPhase with IdentityDenotTransformer { thisPhase => +class LiftTry extends MiniPhase, IdentityDenotTransformer, RecordStackChange { thisPhase => import ast.tpd._ override def phaseName: String = LiftTry.name @@ -40,35 +40,14 @@ class LiftTry extends MiniPhase with IdentityDenotTransformer { thisPhase => override def initContext(ctx: FreshContext): Unit = NeedLift = ctx.addLocation(false) - private def liftingCtx(p: Boolean)(using Context) = + private def liftingCtx(p: Boolean)(using Context): Context = if (needLift == p) ctx else ctx.fresh.updateStore(NeedLift, p) - override def prepareForApply(tree: Apply)(using Context): Context = - liftingCtx(true) + protected def stackChange(using Context): Context = liftingCtx(true) override def prepareForDefDef(tree: DefDef)(using Context): Context = liftingCtx(false) - override def prepareForValDef(tree: ValDef)(using Context): Context = - if !tree.symbol.exists - || tree.symbol.isSelfSym - || tree.symbol.owner == ctx.owner.enclosingMethod - && !tree.symbol.is(Lazy) - // The current implementation wraps initializers of lazy vals in - // calls to an initialize method, which means that a `try` in the - // initializer needs to be lifted. Note that the new scheme proposed - // in #6979 would avoid this. - then ctx - else liftingCtx(true) - - override def prepareForAssign(tree: Assign)(using Context): Context = - if (tree.lhs.symbol.maybeOwner == ctx.owner.enclosingMethod) ctx - else liftingCtx(true) - - override def prepareForReturn(tree: Return)(using Context): Context = - if (!isNonLocalReturn(tree)) ctx - else liftingCtx(true) - override def prepareForTemplate(tree: Template)(using Context): Context = liftingCtx(false) diff --git a/compiler/src/dotty/tools/dotc/transform/RecordStackChange.scala b/compiler/src/dotty/tools/dotc/transform/RecordStackChange.scala new file mode 100644 index 000000000000..bb24cabea018 --- /dev/null +++ b/compiler/src/dotty/tools/dotc/transform/RecordStackChange.scala @@ -0,0 +1,41 @@ +package dotty.tools.dotc +package transform + +import MegaPhase.MiniPhase +import core.* +import Contexts.* +import Flags.Lazy +import NonLocalReturns.isNonLocalReturn + +/** A trait shared between LiftTry and DropBreaks. + * Overrides prepare methods for trees that push to the stack before some of their elements + * are evaluated. + */ +trait RecordStackChange extends MiniPhase: + import ast.tpd.* + + /** The method to call to record a stack change */ + protected def stackChange(using Context): Context + + override def prepareForApply(tree: Apply)(using Context): Context = + stackChange + + override def prepareForValDef(tree: ValDef)(using Context): Context = + if !tree.symbol.exists + || tree.symbol.isSelfSym + || tree.symbol.owner == ctx.owner.enclosingMethod + && !tree.symbol.is(Lazy) + // The current implementation wraps initializers of lazy vals in + // calls to an initialize method, which means that a `try` in the + // initializer needs to be lifted. Note that the new scheme proposed + // in #6979 would avoid this. + then ctx + else stackChange + + override def prepareForAssign(tree: Assign)(using Context): Context = + if (tree.lhs.symbol.maybeOwner == ctx.owner.enclosingMethod) ctx + else stackChange + + override def prepareForReturn(tree: Return)(using Context): Context = + if (!isNonLocalReturn(tree)) ctx + else stackChange \ No newline at end of file diff --git a/tests/run/break-opt.scala b/tests/run/break-opt.scala index 397a27e131fc..5dd12b0a237f 100644 --- a/tests/run/break-opt.scala +++ b/tests/run/break-opt.scala @@ -57,6 +57,21 @@ object breakOpt: if x == 0 then break() y + def test7(x0: Int): Option[Int] = + boundary: + Some( + if x0 < 0 then break(None) // no jump possible, since stacksize changes + else x0 + 1 + ) + + + def test8(x0: Int): Option[Int] = + boundary: + lazy val x = + if x0 < 0 then break(None) // no jump possible, since stacksize changes + else x0 + 1 + Some(x) + @main def Test = import breakOpt.* assert(test1(0) == 0) @@ -68,5 +83,7 @@ object breakOpt: test4(-1) assert(test5(2) == 1) assert(test6(3) == 18) + assert(test7(3) == Some(4)) + assert(test7(-3) == None) From 5620e3e580fc1a83de79bc0eb7cf22dddeb68b5a Mon Sep 17 00:00:00 2001 From: odersky Date: Wed, 11 Jan 2023 19:36:19 +0100 Subject: [PATCH 08/33] Add test from errorhandling strawman --- tests/run/errorhandling/Result.scala | 45 +++++++++++++++++ tests/run/errorhandling/Test.scala | 67 ++++++++++++++++++++++++++ tests/run/errorhandling/optional.scala | 20 ++++++++ 3 files changed, 132 insertions(+) create mode 100644 tests/run/errorhandling/Result.scala create mode 100644 tests/run/errorhandling/Test.scala create mode 100644 tests/run/errorhandling/optional.scala diff --git a/tests/run/errorhandling/Result.scala b/tests/run/errorhandling/Result.scala new file mode 100644 index 000000000000..6a9d9027311b --- /dev/null +++ b/tests/run/errorhandling/Result.scala @@ -0,0 +1,45 @@ +package scala.util +import boundary.Label + +abstract class Result[+T, +E] +case class Ok[+T](value: T) extends Result[T, Nothing] +case class Err[+E](value: E) extends Result[Nothing, E] + +object Result: + extension [T, E](r: Result[T, E]) + + /** `_.?` propagates Err to current Label */ + transparent inline def ? (using Label[Err[E]]): T = r match + case r: Ok[_] => r.value + case err => break(err.asInstanceOf[Err[E]]) + + /** If this is an `Err`, map its value */ + def mapErr[E1](f: E => E1): Result[T, E1] = r match + case err: Err[_] => Err(f(err.value)) + case ok: Ok[_] => ok + + /** Map Ok values, propagate Errs */ + def map[U](f: T => U): Result[U, E] = r match + case Ok(x) => Ok(f(x)) + case err: Err[_] => err + + /** Flatmap Ok values, propagate Errs */ + def flatMap[U](f: T => Result[U, E]): Result[U, E] = r match + case Ok(x) => f(x) + case err: Err[_] => err + + /** Similar to `Try`: Convert exceptions raised by `body` to `Err`s. + */ + def apply[T](body: => T): Result[T, Exception] = + try Ok(body) + catch case ex: Exception => Err(ex) +end Result + +/** A prompt for `_.?`. It establishes a boundary to which `_.?` returns */ +object respond: + inline def apply[T, E](inline body: Label[Err[E]] ?=> T): Result[T, E] = + boundary: + val result = body + Ok(result) + + diff --git a/tests/run/errorhandling/Test.scala b/tests/run/errorhandling/Test.scala new file mode 100644 index 000000000000..4c88c1f044dd --- /dev/null +++ b/tests/run/errorhandling/Test.scala @@ -0,0 +1,67 @@ +import scala.util.* + +/** boundary/break as a replacement for non-local returns */ +def indexOf[T](xs: List[T], elem: T): Int = + boundary: + for (x, i) <- xs.zipWithIndex do + if x == elem then break(i) + -1 + +def breakTest() = + println("breakTest") + assert(indexOf(List(1, 2, 3), 2) == 1) + assert(indexOf(List(1, 2, 3), 0) == -1) + +/** traverse becomes trivial to write */ +def traverse[T](xs: List[Option[T]]): Option[List[T]] = + optional(xs.map(_.?)) + +def optTest() = + println("optTest") + assert(traverse(List(Some(1), Some(2), Some(3))) == Some(List(1, 2, 3))) + assert(traverse(List(Some(1), None, Some(3))) == None) + +/** A check function returning a Result[Unit, _] */ +inline def check[E](p: Boolean, err: E): Result[Unit, E] = + if p then Ok(()) else Err(err) + +/** Another variant of a check function that returns directly to the given + * label in case of error. + */ +inline def check_![E](p: Boolean, err: E)(using l: boundary.Label[Err[E]]): Unit = + if p then () else break(Err(err)) + +/** Use `Result` to convert exceptions to `Err` values */ +def parseDouble(s: String): Result[Double, Exception] = + Result(s.toDouble) + +def parseDoubles(ss: List[String]): Result[List[Double], Exception] = + respond: + ss.map(parseDouble(_).?) + +/** Demonstrate combination of `check` and `.?`. */ +def trySqrt(x: Double) = // inferred: Result[Double, String] + respond: + check(x >= 0, s"cannot take sqrt of negative $x").? // direct jump + math.sqrt(x) + +/** Instead of `check(...).?` one can also use `check_!(...)`. + * Note use of `mapErr` to convert Exception errors to String errors. + */ +def sumRoots(xs: List[String]) = // inferred: Result[Double, String] + respond: + check_!(xs.nonEmpty, "list is empty") // direct jump + val ys = parseDoubles(xs).mapErr(_.toString).? // direct jump + ys.reduce((x, y) => x + trySqrt(y).?) // need exception to propagate `Err` + +def resultTest() = + println("resultTest") + assert(sumRoots(List("1", "4", "9")) == Ok(6)) + assert(sumRoots(List("1", "-2", "4")) == Err(s"cannot take sqrt of negative -2.0")) + assert(sumRoots(List()) == Err("list is empty")) + assert(sumRoots(List("1", "3ab")) == Err("java.lang.NumberFormatException: For input string: \"3ab\"")) + +@main def Test = + breakTest() + optTest() + resultTest() \ No newline at end of file diff --git a/tests/run/errorhandling/optional.scala b/tests/run/errorhandling/optional.scala new file mode 100644 index 000000000000..6eeac76108b0 --- /dev/null +++ b/tests/run/errorhandling/optional.scala @@ -0,0 +1,20 @@ +package scala.util +import boundary.Label + +/** A mockup of scala.Option */ +abstract class Option[+T] +case class Some[+T](x: T) extends Option[T] +case object None extends Option[Nothing] + +object Option: + /** This extension should be added to the companion object of scala.Option */ + extension [T](r: Option[T]) + inline def ? (using label: Label[None.type]): T = r match + case Some(x) => x + case None => break(None) + +/** A prompt for `Option`, which establishes a boundary which `_.?` on `Option` can return */ +object optional: + inline def apply[T](inline body: Label[None.type] ?=> T): Option[T] = + boundary(Some(body)) + From 395f2b60ee22abfa8fec0861d6209f4fa3835931 Mon Sep 17 00:00:00 2001 From: odersky Date: Mon, 26 Dec 2022 19:05:31 +0100 Subject: [PATCH 09/33] Make test Scala.js-compliant --- tests/run/errorhandling/Test.scala | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/tests/run/errorhandling/Test.scala b/tests/run/errorhandling/Test.scala index 4c88c1f044dd..36c44c04a19e 100644 --- a/tests/run/errorhandling/Test.scala +++ b/tests/run/errorhandling/Test.scala @@ -56,10 +56,12 @@ def sumRoots(xs: List[String]) = // inferred: Result[Double, String] def resultTest() = println("resultTest") + def assertFail(value: Any, s: String) = value match + case Err(msg: String) => assert(msg.contains(s)) assert(sumRoots(List("1", "4", "9")) == Ok(6)) - assert(sumRoots(List("1", "-2", "4")) == Err(s"cannot take sqrt of negative -2.0")) - assert(sumRoots(List()) == Err("list is empty")) - assert(sumRoots(List("1", "3ab")) == Err("java.lang.NumberFormatException: For input string: \"3ab\"")) + assertFail(sumRoots(List("1", "-2", "4")), "cannot take sqrt of negative") + assertFail(sumRoots(List()), "list is empty") + assertFail(sumRoots(List("1", "3ab")), "NumberFormatException") @main def Test = breakTest() From 5447a498950fac012e8db526868e65acd55cc85a Mon Sep 17 00:00:00 2001 From: odersky Date: Wed, 28 Dec 2022 19:38:42 +0100 Subject: [PATCH 10/33] Another test # Conflicts: # tests/run/errorhandling/break.scala --- tests/run/errorhandling.check | 4 ++++ tests/run/errorhandling/Test.scala | 3 ++- tests/run/errorhandling/kostas.scala | 35 ++++++++++++++++++++++++++++ 3 files changed, 41 insertions(+), 1 deletion(-) create mode 100644 tests/run/errorhandling.check create mode 100644 tests/run/errorhandling/kostas.scala diff --git a/tests/run/errorhandling.check b/tests/run/errorhandling.check new file mode 100644 index 000000000000..882ca57f5022 --- /dev/null +++ b/tests/run/errorhandling.check @@ -0,0 +1,4 @@ +breakTest +optTest +resultTest +Person(Kostas,5) diff --git a/tests/run/errorhandling/Test.scala b/tests/run/errorhandling/Test.scala index 36c44c04a19e..fed824917c0b 100644 --- a/tests/run/errorhandling/Test.scala +++ b/tests/run/errorhandling/Test.scala @@ -66,4 +66,5 @@ def resultTest() = @main def Test = breakTest() optTest() - resultTest() \ No newline at end of file + resultTest() + parseCsvIgnoreErrors() \ No newline at end of file diff --git a/tests/run/errorhandling/kostas.scala b/tests/run/errorhandling/kostas.scala new file mode 100644 index 000000000000..8caa878ee9d2 --- /dev/null +++ b/tests/run/errorhandling/kostas.scala @@ -0,0 +1,35 @@ +package optionMockup: + import dotty.util.boundary + object optional: + transparent inline def apply[T](inline body: boundary.Label[None.type] ?=> T): Option[T] = + boundary(Some(body)) + + extension [T](r: Option[T]) + transparent inline def ? (using label: boundary.Label[None.type]): T = r match + case Some(x) => x + case None => label.break(None) + +import optionMockup.* + +case class Person(name: String, age: Int) + +object PersonCsvParserIgnoreErrors: + def parse(csv: Seq[String]): Seq[Person] = + for + line <- csv + columns = line.split(",")x + parsed <- parseColumns(columns) + yield + parsed + + private def parseColumns(columns: Seq[String]): Option[Person] = + columns match + case Seq(name, age) => parsePerson(name, age) + case _ => None + + private def parsePerson(name: String, age: String): Option[Person] = + optional: + Person(name, age.toIntOption.?) + +def parseCsvIgnoreErrors() = + println(PersonCsvParserIgnoreErrors.parse(Seq("Kostas,5", "George,invalid", "too,many,columns")).mkString("\n")) \ No newline at end of file From 3e5a678941760615c0e5d22366eab0b68e2bcfbb Mon Sep 17 00:00:00 2001 From: odersky Date: Sat, 31 Dec 2022 19:45:50 +0100 Subject: [PATCH 11/33] Add validation to the strawman # Conflicts: # tests/run/errorhandling/Result.scala --- tests/run/errorhandling/Result.scala | 22 ++++++++++++++++++++++ tests/run/errorhandling/Test.scala | 8 ++++++++ tests/run/errorhandling/kostas.scala | 2 +- 3 files changed, 31 insertions(+), 1 deletion(-) diff --git a/tests/run/errorhandling/Result.scala b/tests/run/errorhandling/Result.scala index 6a9d9027311b..c44f5ff7bf64 100644 --- a/tests/run/errorhandling/Result.scala +++ b/tests/run/errorhandling/Result.scala @@ -28,11 +28,33 @@ object Result: case Ok(x) => f(x) case err: Err[_] => err + /** Validate both `r` and `other`; return a pair of successes or a list of failures. */ + def * [U](other: Result[U, E]): Result[(T, U), List[E]] = (r, other) match + case (Ok(x), Ok(y)) => Ok((x, y)) + case (Ok(_), Err(e)) => Err(e :: Nil) + case (Err(e), Ok(_)) => Err(e :: Nil) + case (Err(e1), Err(e2)) => Err(e1 :: e2 :: Nil) + + /** Validate both `r` and `other`; return a tuple of successes or a list of failures. + * Unlike with `*`, the right hand side `other` must be a `Result` returning a `Tuple`, + * and the left hand side is added to it. See `Result.empty` for a convenient + * right unit of chains of `*:`s. + */ + def *: [U <: Tuple](other: Result[U, List[E]]): Result[T *: U, List[E]] = (r, other) match + case (Ok(x), Ok(ys)) => Ok(x *: ys) + case (Ok(_), es: Err[?]) => es + case (Err(e), Ok(_)) => Err(e :: Nil) + case (Err(e), Err(es)) => Err(e :: es) + end extension + /** Similar to `Try`: Convert exceptions raised by `body` to `Err`s. */ def apply[T](body: => T): Result[T, Exception] = try Ok(body) catch case ex: Exception => Err(ex) + + /** Right unit for chains of `*:`s. Returns an `Ok` with an `EmotyTuple` value. */ + def empty: Result[EmptyTuple, Nothing] = Ok(EmptyTuple) end Result /** A prompt for `_.?`. It establishes a boundary to which `_.?` returns */ diff --git a/tests/run/errorhandling/Test.scala b/tests/run/errorhandling/Test.scala index fed824917c0b..7f510bb74e64 100644 --- a/tests/run/errorhandling/Test.scala +++ b/tests/run/errorhandling/Test.scala @@ -62,6 +62,14 @@ def resultTest() = assertFail(sumRoots(List("1", "-2", "4")), "cannot take sqrt of negative") assertFail(sumRoots(List()), "list is empty") assertFail(sumRoots(List("1", "3ab")), "NumberFormatException") + val xs = sumRoots(List("1", "-2", "4")) *: sumRoots(List()) *: sumRoots(List("1", "3ab")) *: Result.empty + xs match + case Err(msgs) => assert(msgs.length == 3) + case _ => assert(false) + val ys = sumRoots(List("1", "2", "4")) *: sumRoots(List("1")) *: sumRoots(List("2")) *: Result.empty + ys match + case Ok((a, b, c)) => // ok + case _ => assert(false) @main def Test = breakTest() diff --git a/tests/run/errorhandling/kostas.scala b/tests/run/errorhandling/kostas.scala index 8caa878ee9d2..ec32750f4bc4 100644 --- a/tests/run/errorhandling/kostas.scala +++ b/tests/run/errorhandling/kostas.scala @@ -17,7 +17,7 @@ object PersonCsvParserIgnoreErrors: def parse(csv: Seq[String]): Seq[Person] = for line <- csv - columns = line.split(",")x + columns = line.split(",") parsed <- parseColumns(columns) yield parsed From 39e63f3aa18bf09943c26dbc7278cce45dbb85bf Mon Sep 17 00:00:00 2001 From: odersky Date: Thu, 12 Jan 2023 10:43:21 +0100 Subject: [PATCH 12/33] Adress some review comments and other tweaks --- .../src/dotty/tools/dotc/transform/DropBreaks.scala | 8 ++------ library/src/scala/util/break.scala | 10 +++++----- tests/run/errorhandling/kostas.scala | 2 +- 3 files changed, 8 insertions(+), 12 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/DropBreaks.scala b/compiler/src/dotty/tools/dotc/transform/DropBreaks.scala index ae3412305638..e3f24ec85702 100644 --- a/compiler/src/dotty/tools/dotc/transform/DropBreaks.scala +++ b/compiler/src/dotty/tools/dotc/transform/DropBreaks.scala @@ -24,7 +24,7 @@ object DropBreaks: class LabelUsage(val goto: TermSymbol, val enclMeth: Symbol): /** The number of references to associated label that come from labeled returns */ var returnRefs: Int = 0 - /** The number of other references to assocated label */ + /** The number of other references to associated label */ var otherRefs: Int = 0 private val LabelUsages = new Property.Key[Map[Symbol, LabelUsage]] @@ -167,7 +167,7 @@ class DropBreaks extends MiniPhase, RecordStackChange: override def prepareForApply(tree: Apply)(using Context): Context = tree match case Break(_, _) => ctx case _ => stackChange - + // other stack changing operations are handled in RecordStackChange /** If `tree` is a BreakBoundary, transform it as follows: @@ -221,8 +221,4 @@ class DropBreaks extends MiniPhase, RecordStackChange: uses.otherRefs += 1 tree - //override def transformReturn(tree: Return)(using Context): Tree = - // if !tree.from.isEmpty && tree.expr.tpe.isExactlyNothing then tree.expr - // else tree - end DropBreaks diff --git a/library/src/scala/util/break.scala b/library/src/scala/util/break.scala index 5c3bccfa93d7..3bf13c1e8072 100644 --- a/library/src/scala/util/break.scala +++ b/library/src/scala/util/break.scala @@ -8,13 +8,13 @@ object break: /** Abort current computation and instead return `value` as the value of * the enclosing `boundary` call that created `label`. */ - def apply[T](value: T)(using l: boundary.Label[T]): Nothing = - l.break(value) + def apply[T](value: T)(using label: boundary.Label[T]): Nothing = + label.break(value) /** Abort current computation and instead continue after the `boundary` call that * created `label`. */ - def apply()(using l: boundary.Label[Unit]): Nothing = - apply(()) + def apply()(using label: boundary.Label[Unit]): Nothing = + label.break(()) -end break \ No newline at end of file +end break diff --git a/tests/run/errorhandling/kostas.scala b/tests/run/errorhandling/kostas.scala index ec32750f4bc4..d76349530277 100644 --- a/tests/run/errorhandling/kostas.scala +++ b/tests/run/errorhandling/kostas.scala @@ -1,5 +1,5 @@ package optionMockup: - import dotty.util.boundary + import scala.util.boundary object optional: transparent inline def apply[T](inline body: boundary.Label[None.type] ?=> T): Option[T] = boundary(Some(body)) From 8fb3c38d66c939f8a2f67d3a0937b9f9d6fdfc65 Mon Sep 17 00:00:00 2001 From: odersky Date: Thu, 12 Jan 2023 10:51:48 +0100 Subject: [PATCH 13/33] Make Label contravariant Reduces number of type parameters in boundary from 2 to 1 --- library/src/scala/util/boundary.scala | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/library/src/scala/util/boundary.scala b/library/src/scala/util/boundary.scala index e9a20fbaa50c..13c17093c7e6 100644 --- a/library/src/scala/util/boundary.scala +++ b/library/src/scala/util/boundary.scala @@ -23,14 +23,14 @@ object boundary: /** Labels are targets indicating which boundary will be exited by a `break`. */ - class Label[T]: + class Label[-T]: def break(value: T): Nothing = throw Break(this, value) /** Run `body` with freshly generated label as implicit argument. Catch any * breaks associated with that label and return their results instead of * `body`'s result. */ - inline def apply[T <: R, R](inline body: Label[T] ?=> R): R = + inline def apply[T](inline body: Label[T] ?=> T): T = val local = Label[T]() try body(using local) catch case ex: Break[T] @unchecked => From fbc80775312cd55153677ed9821ad8a2502846a4 Mon Sep 17 00:00:00 2001 From: odersky Date: Thu, 12 Jan 2023 11:31:22 +0100 Subject: [PATCH 14/33] Make Label a final class --- compiler/src/dotty/tools/dotc/transform/DropBreaks.scala | 1 + library/src/scala/util/boundary.scala | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/transform/DropBreaks.scala b/compiler/src/dotty/tools/dotc/transform/DropBreaks.scala index e3f24ec85702..833f25e0ceb7 100644 --- a/compiler/src/dotty/tools/dotc/transform/DropBreaks.scala +++ b/compiler/src/dotty/tools/dotc/transform/DropBreaks.scala @@ -123,6 +123,7 @@ class DropBreaks extends MiniPhase, RecordStackChange: Some((id.symbol, arg)) case _ => None case _ => None + end Break /** The LabelUsage data associated with `lbl` in the current context */ private def labelUsage(lbl: Symbol)(using Context): Option[LabelUsage] = diff --git a/library/src/scala/util/boundary.scala b/library/src/scala/util/boundary.scala index 13c17093c7e6..c7a9ed0e0ab5 100644 --- a/library/src/scala/util/boundary.scala +++ b/library/src/scala/util/boundary.scala @@ -23,7 +23,7 @@ object boundary: /** Labels are targets indicating which boundary will be exited by a `break`. */ - class Label[-T]: + final class Label[-T]: def break(value: T): Nothing = throw Break(this, value) /** Run `body` with freshly generated label as implicit argument. Catch any From a51a58657c0a47eef61f9d6b456e1ab5eedc7190 Mon Sep 17 00:00:00 2001 From: odersky Date: Thu, 12 Jan 2023 11:32:06 +0100 Subject: [PATCH 15/33] Add loops test with separate exit and continue boundaries Note that this does not rewrite to labeled returns yet because of some inlining limitations. --- tests/run/loops-alt.scala | 46 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) create mode 100644 tests/run/loops-alt.scala diff --git a/tests/run/loops-alt.scala b/tests/run/loops-alt.scala new file mode 100644 index 000000000000..69f5594bcdc1 --- /dev/null +++ b/tests/run/loops-alt.scala @@ -0,0 +1,46 @@ +import scala.util.{boundary, break} +import boundary.Label +import java.util.concurrent.TimeUnit + +object loop: + opaque type ExitLabel = Label[Unit] + opaque type ContinueLabel = Label[Unit] + + inline def apply(inline op: (ExitLabel, ContinueLabel) ?=> Unit): Unit = + boundary { exitLabel ?=> + while true do + boundary { continueLabel ?=> + op(using exitLabel, continueLabel) + } + } + end apply + + inline def exit()(using ExitLabel): Unit = + break() + + inline def continue()(using ContinueLabel): Unit = + break() +end loop + +def testLoop(xs: List[Int]) = + var current = xs + var sum = 0 + loop: + // This should be convertible to labeled returns but isn't, since + // the following code is still passed as a closure to `boundary`. + // That's probably due to the additional facade operations necessary + // for opaque types. + if current.isEmpty then loop.exit() + val hd = current.head + current = current.tail + if hd == 0 then loop.exit() + if hd < 0 then loop.continue() + sum += hd + sum + +@main def Test = + assert(testLoop(List(1, 2, 3, -2, 4, -3, 0, 1, 2, 3)) == 10) + assert(testLoop(List()) == 0) + assert(testLoop(List(-2, -3, 0, 1)) == 0) + + From 38923f9bd16cf4d338ea90bdd323f7c9a508036e Mon Sep 17 00:00:00 2001 From: odersky Date: Thu, 12 Jan 2023 11:44:05 +0100 Subject: [PATCH 16/33] Another test --- tests/pos/simple-boundary.scala | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 tests/pos/simple-boundary.scala diff --git a/tests/pos/simple-boundary.scala b/tests/pos/simple-boundary.scala new file mode 100644 index 000000000000..8ec097706bc3 --- /dev/null +++ b/tests/pos/simple-boundary.scala @@ -0,0 +1,4 @@ +import scala.util.* +def test: Unit = + boundary: label ?=> + while true do break() From 861f5da6c7efc431a2d408e38a4a72cc0655e673 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Doeraene?= Date: Thu, 12 Jan 2023 14:03:51 +0100 Subject: [PATCH 17/33] Actually test the stack size problem. The test was minimized too far and did not actually have a stack size issue. Also added a test when in a local `def` and made the comment for the `lazy val` case more accurate. --- tests/run/break-opt.scala | 32 ++++++++++++++++++++++---------- 1 file changed, 22 insertions(+), 10 deletions(-) diff --git a/tests/run/break-opt.scala b/tests/run/break-opt.scala index 5dd12b0a237f..c59a57d4bffc 100644 --- a/tests/run/break-opt.scala +++ b/tests/run/break-opt.scala @@ -58,17 +58,27 @@ object breakOpt: y def test7(x0: Int): Option[Int] = - boundary: - Some( - if x0 < 0 then break(None) // no jump possible, since stacksize changes - else x0 + 1 - ) - + val result = + boundary: + Some( + 1 + ( + if x0 < 0 then break(None) // no jump possible, since stacksize changes and no direct RETURN + else x0 + ) + ) + result.map(_ + 10) def test8(x0: Int): Option[Int] = boundary: lazy val x = - if x0 < 0 then break(None) // no jump possible, since stacksize changes + if x0 < 0 then break(None) // no jump possible, since ultimately in a different method + else x0 + 1 + Some(x) + + def test9(x0: Int): Option[Int] = + boundary: + def x = + if x0 < 0 then break(None) // no jump possible, since in a different method else x0 + 1 Some(x) @@ -83,7 +93,9 @@ object breakOpt: test4(-1) assert(test5(2) == 1) assert(test6(3) == 18) - assert(test7(3) == Some(4)) + assert(test7(3) == Some(14)) assert(test7(-3) == None) - - + assert(test8(3) == Some(4)) + assert(test8(-3) == None) + assert(test9(3) == Some(4)) + assert(test9(-3) == None) From 63f6e97e00e9b3af5e4479d9d65b5775baf55150 Mon Sep 17 00:00:00 2001 From: odersky Date: Thu, 12 Jan 2023 13:07:23 +0100 Subject: [PATCH 18/33] Optimize extractors Only check trees if there is an enclosing boundary --- .../tools/dotc/transform/DropBreaks.scala | 57 ++++++++++++------- 1 file changed, 35 insertions(+), 22 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/DropBreaks.scala b/compiler/src/dotty/tools/dotc/transform/DropBreaks.scala index 833f25e0ceb7..4ce5d53ffc2e 100644 --- a/compiler/src/dotty/tools/dotc/transform/DropBreaks.scala +++ b/compiler/src/dotty/tools/dotc/transform/DropBreaks.scala @@ -50,6 +50,9 @@ class DropBreaks extends MiniPhase, RecordStackChange: override def runsAfterGroupsOf: Set[String] = Set(ElimByName.name) // we want by-name parameters to be converted to closures + /** The number of boundary nodes enclosing the currently analized tree. */ + var enclosingBoundaries: Int = 0 + private object LabelTry: object GuardedThrow: @@ -136,6 +139,7 @@ class DropBreaks extends MiniPhase, RecordStackChange: /** If `tree` is a BreakBoundary, associate a fresh `LabelUsage` with its label. */ override def prepareForBlock(tree: Block)(using Context): Context = tree match case BreakBoundary(label, _) => + enclosingBoundaries += 1 val mapSoFar = ctx.property(LabelUsages).getOrElse(Map.empty) val goto = newSymbol(ctx.owner, BoundaryName.fresh(), Synthetic | Label, tree.tpe) ctx.fresh.setProperty(LabelUsages, @@ -157,17 +161,22 @@ class DropBreaks extends MiniPhase, RecordStackChange: /** Need to suppress labeled returns if the stack can change between * source and target of the jump */ - protected def stackChange(using Context) = shadowLabels + protected def stackChange(using Context) = + if enclosingBoundaries == 0 then ctx else shadowLabels /** Need to suppress labeled returns if there is an intervening try */ - override def prepareForTry(tree: Try)(using Context): Context = tree match - case LabelTry(_, _) => ctx - case _ => shadowLabels - - override def prepareForApply(tree: Apply)(using Context): Context = tree match - case Break(_, _) => ctx - case _ => stackChange + override def prepareForTry(tree: Try)(using Context): Context = + if enclosingBoundaries == 0 then ctx + else tree match + case LabelTry(_, _) => ctx + case _ => shadowLabels + + override def prepareForApply(tree: Apply)(using Context): Context = + if enclosingBoundaries == 0 then ctx + else tree match + case Break(_, _) => ctx + case _ => stackChange // other stack changing operations are handled in RecordStackChange @@ -177,6 +186,7 @@ class DropBreaks extends MiniPhase, RecordStackChange: */ override def transformBlock(tree: Block)(using Context): Tree = tree match case BreakBoundary(label, expr) => + enclosingBoundaries -= 1 val uses = ctx.property(LabelUsages).get(label) val tree1 = if uses.otherRefs > 1 then @@ -200,26 +210,29 @@ class DropBreaks extends MiniPhase, RecordStackChange: * and the non-local refcount is decreased, since `local` the `Label_this` * binding containing `local` is dropped. */ - override def transformApply(tree: Apply)(using Context): Tree = tree match - case Break(lbl, arg) => - labelUsage(lbl) match - case Some(uses: LabelUsage) - if uses.enclMeth == ctx.owner.enclosingMethod - && !ctx.property(ShadowedLabels).getOrElse(Set.empty).contains(lbl) - => - uses.otherRefs -= 1 - uses.returnRefs += 1 - Return(arg, ref(uses.goto)).withSpan(arg.span) - case _ => tree - case _ => tree + override def transformApply(tree: Apply)(using Context): Tree = + if enclosingBoundaries == 0 then tree + else tree match + case Break(lbl, arg) => + labelUsage(lbl) match + case Some(uses: LabelUsage) + if uses.enclMeth == ctx.owner.enclosingMethod + && !ctx.property(ShadowedLabels).getOrElse(Set.empty).contains(lbl) + => + uses.otherRefs -= 1 + uses.returnRefs += 1 + Return(arg, ref(uses.goto)).withSpan(arg.span) + case _ => tree + case _ => tree /** If `tree` refers to an enclosing label, increase its non local recount. * This increase is corrected in `transformInlined` if the reference turns * out to be part of a BreakThrow to a local, non-shadowed label. */ override def transformIdent(tree: Ident)(using Context): Tree = - for uses <- labelUsage(tree.symbol) do - uses.otherRefs += 1 + if enclosingBoundaries != 0 then + for uses <- labelUsage(tree.symbol) do + uses.otherRefs += 1 tree end DropBreaks From da42cf7f2455309c28d04f606c246199ca52de48 Mon Sep 17 00:00:00 2001 From: odersky Date: Thu, 12 Jan 2023 15:07:29 +0100 Subject: [PATCH 19/33] Drop ControlException --- library/src/scala/util/boundary.scala | 9 +++++---- .../scala/util/control/ControlException.scala | 18 ------------------ 2 files changed, 5 insertions(+), 22 deletions(-) delete mode 100644 library/src/scala/util/control/ControlException.scala diff --git a/library/src/scala/util/boundary.scala b/library/src/scala/util/boundary.scala index c7a9ed0e0ab5..563a068b3a8c 100644 --- a/library/src/scala/util/boundary.scala +++ b/library/src/scala/util/boundary.scala @@ -1,5 +1,4 @@ package scala.util -import control.ControlException /** A boundary that can be exited by `break` calls. * `boundary` and `break` represent a unified and superior alternative for the @@ -9,8 +8,8 @@ import control.ControlException * - Unified names: `boundary` to establish a scope, `break` to leave it. * `break` can optionally return a value. * - Integration with exceptions. `break`s are logically non-fatal exceptions. - * The `Break` exception class extends `ControlException` which is a regular - * `RuntimeException`, optimized so that stack trace generation is suppressed. + * The `Break` exception class extends `RuntimeException` and is optimized so + * that stack trace generation is suppressed. * - Better performance: breaks to enclosing scopes in the same method can * be rwritten to jumps. */ @@ -19,7 +18,9 @@ object boundary: /** User code should call `break.apply` instead of throwing this exception * directly. */ - class Break[T] private[boundary](val label: Label[T], val value: T) extends ControlException + class Break[T] private[boundary](val label: Label[T], val value: T) + extends RuntimeException( + /*message*/ null, /*cause*/ null, /*enableSuppression=*/ false, /*writableStackTrace*/ false) /** Labels are targets indicating which boundary will be exited by a `break`. */ diff --git a/library/src/scala/util/control/ControlException.scala b/library/src/scala/util/control/ControlException.scala deleted file mode 100644 index eb0cfa63e96b..000000000000 --- a/library/src/scala/util/control/ControlException.scala +++ /dev/null @@ -1,18 +0,0 @@ -package scala.util.control - -/** A parent class for throwable objects intended for flow control. - * - * Instances of ControlException don't record a stacktrace and are therefore - * much more efficient to throw than normal exceptions. - * - * Unlike `ControlThrowable`, `ControlException` is a regular `RuntimeException` - * that is supposed to be handled like any other exception. - * - * Instances of `ControlException` should not normally have a cause. - * Legacy subclasses may set a cause using `initCause`. - */ -abstract class ControlException(message: String | Null) extends RuntimeException( - message, /*cause*/ null, /*enableSuppression=*/ false, /*writableStackTrace*/ false): - - def this() = this(message = null) - From 2d67e6b21470de53a9a7c809cf562dda4e507807 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Doeraene?= Date: Thu, 12 Jan 2023 16:58:19 +0100 Subject: [PATCH 20/33] Track stack height differences in the back-end for Labeled Returns. We track in a new variable `stackHeight` the number of stack slots that are "on hold" while loading some other tree. When we immediately consume stack elements without using `genLoad` on top, we do not need to update `stackHeight`. We store the target `stackHeight` of labels in their `LoadDestination.Jump`. This way, when we generate a jump, we can compute the difference and drop the excess stack. --- .../tools/backend/jvm/BCodeBodyBuilder.scala | 91 +++++++++++++++---- .../tools/backend/jvm/BCodeIdiomatic.scala | 10 ++ .../tools/backend/jvm/BCodeSkelBuilder.scala | 13 ++- 3 files changed, 96 insertions(+), 18 deletions(-) diff --git a/compiler/src/dotty/tools/backend/jvm/BCodeBodyBuilder.scala b/compiler/src/dotty/tools/backend/jvm/BCodeBodyBuilder.scala index bf10e37943a8..1d559c9950f1 100644 --- a/compiler/src/dotty/tools/backend/jvm/BCodeBodyBuilder.scala +++ b/compiler/src/dotty/tools/backend/jvm/BCodeBodyBuilder.scala @@ -4,7 +4,7 @@ package jvm import scala.language.unsafeNulls -import scala.annotation.switch +import scala.annotation.{switch, tailrec} import scala.collection.mutable.SortedMap import scala.tools.asm @@ -79,9 +79,14 @@ trait BCodeBodyBuilder extends BCodeSkelBuilder { tree match { case Assign(lhs @ DesugaredSelect(qual, _), rhs) => + val savedStackHeight = stackHeight val isStatic = lhs.symbol.isStaticMember - if (!isStatic) { genLoadQualifier(lhs) } + if (!isStatic) { + genLoadQualifier(lhs) + stackHeight += 1 + } genLoad(rhs, symInfoTK(lhs.symbol)) + stackHeight = savedStackHeight lineNumber(tree) // receiverClass is used in the bytecode to access the field. using sym.owner may lead to IllegalAccessError val receiverClass = qual.tpe.typeSymbol @@ -145,7 +150,9 @@ trait BCodeBodyBuilder extends BCodeSkelBuilder { } genLoad(larg, resKind) + stackHeight += resKind.size genLoad(rarg, if (isShift) INT else resKind) + stackHeight -= resKind.size (code: @switch) match { case ADD => bc add resKind @@ -182,14 +189,19 @@ trait BCodeBodyBuilder extends BCodeSkelBuilder { if (isArrayGet(code)) { // load argument on stack assert(args.length == 1, s"Too many arguments for array get operation: $tree"); + stackHeight += 1 genLoad(args.head, INT) + stackHeight -= 1 generatedType = k.asArrayBType.componentType bc.aload(elementType) } else if (isArraySet(code)) { val List(a1, a2) = args + stackHeight += 1 genLoad(a1, INT) + stackHeight += 1 genLoad(a2) + stackHeight -= 2 generatedType = UNIT bc.astore(elementType) } else { @@ -223,7 +235,7 @@ trait BCodeBodyBuilder extends BCodeSkelBuilder { val resKind = if (hasUnitBranch) UNIT else tpeTK(tree) val postIf = new asm.Label - genLoadTo(thenp, resKind, LoadDestination.Jump(postIf)) + genLoadTo(thenp, resKind, LoadDestination.Jump(postIf, stackHeight)) markProgramPoint(failure) genLoadTo(elsep, resKind, LoadDestination.FallThrough) markProgramPoint(postIf) @@ -482,7 +494,17 @@ trait BCodeBodyBuilder extends BCodeSkelBuilder { dest match case LoadDestination.FallThrough => () - case LoadDestination.Jump(label) => + case LoadDestination.Jump(label, targetStackHeight) => + if targetStackHeight < stackHeight then + val stackDiff = stackHeight - targetStackHeight + if expectedType == UNIT then + bc dropMany stackDiff + else + val loc = locals.makeTempLocal(expectedType) + bc.store(loc.idx, expectedType) + bc dropMany stackDiff + bc.load(loc.idx, expectedType) + end if bc goTo label case LoadDestination.Return => bc emitRETURN returnType @@ -577,7 +599,7 @@ trait BCodeBodyBuilder extends BCodeSkelBuilder { if dest == LoadDestination.FallThrough then val resKind = tpeTK(tree) val jumpTarget = new asm.Label - registerJumpDest(labelSym, resKind, LoadDestination.Jump(jumpTarget)) + registerJumpDest(labelSym, resKind, LoadDestination.Jump(jumpTarget, stackHeight)) genLoad(expr, resKind) markProgramPoint(jumpTarget) resKind @@ -635,7 +657,7 @@ trait BCodeBodyBuilder extends BCodeSkelBuilder { markProgramPoint(loop) if isInfinite then - val dest = LoadDestination.Jump(loop) + val dest = LoadDestination.Jump(loop, stackHeight) genLoadTo(body, UNIT, dest) dest else @@ -650,7 +672,7 @@ trait BCodeBodyBuilder extends BCodeSkelBuilder { val failure = new asm.Label genCond(cond, success, failure, targetIfNoJump = success) markProgramPoint(success) - genLoadTo(body, UNIT, LoadDestination.Jump(loop)) + genLoadTo(body, UNIT, LoadDestination.Jump(loop, stackHeight)) markProgramPoint(failure) end match LoadDestination.FallThrough @@ -744,7 +766,9 @@ trait BCodeBodyBuilder extends BCodeSkelBuilder { // scala/bug#10290: qual can be `this.$outer()` (not just `this`), so we call genLoad (not just ALOAD_0) genLoad(superQual) + stackHeight += 1 genLoadArguments(args, paramTKs(app)) + stackHeight -= 1 generatedType = genCallMethod(fun.symbol, InvokeStyle.Super, app.span) // 'new' constructor call: Note: since constructors are @@ -766,7 +790,9 @@ trait BCodeBodyBuilder extends BCodeSkelBuilder { assert(classBTypeFromSymbol(ctor.owner) == rt, s"Symbol ${ctor.owner.showFullName} is different from $rt") mnode.visitTypeInsn(asm.Opcodes.NEW, rt.internalName) bc dup generatedType + stackHeight += 2 genLoadArguments(args, paramTKs(app)) + stackHeight -= 2 genCallMethod(ctor, InvokeStyle.Special, app.span) case _ => @@ -799,8 +825,12 @@ trait BCodeBodyBuilder extends BCodeSkelBuilder { else if (app.hasAttachment(BCodeHelpers.UseInvokeSpecial)) InvokeStyle.Special else InvokeStyle.Virtual - if (invokeStyle.hasInstance) genLoadQualifier(fun) + val savedStackHeight = stackHeight + if invokeStyle.hasInstance then + genLoadQualifier(fun) + stackHeight += 1 genLoadArguments(args, paramTKs(app)) + stackHeight = savedStackHeight val DesugaredSelect(qual, name) = fun: @unchecked // fun is a Select, also checked in genLoadQualifier val isArrayClone = name == nme.clone_ && qual.tpe.widen.isInstanceOf[JavaArrayType] @@ -858,6 +888,8 @@ trait BCodeBodyBuilder extends BCodeSkelBuilder { bc iconst elems.length bc newarray elmKind + stackHeight += 3 // during the genLoad below, there is the result, its dup, and the index + var i = 0 var rest = elems while (!rest.isEmpty) { @@ -869,6 +901,8 @@ trait BCodeBodyBuilder extends BCodeSkelBuilder { i = i + 1 } + stackHeight -= 3 + generatedType } @@ -883,7 +917,7 @@ trait BCodeBodyBuilder extends BCodeSkelBuilder { val (generatedType, postMatch, postMatchDest) = if dest == LoadDestination.FallThrough then val postMatch = new asm.Label - (tpeTK(tree), postMatch, LoadDestination.Jump(postMatch)) + (tpeTK(tree), postMatch, LoadDestination.Jump(postMatch, stackHeight)) else (expectedType, null, dest) @@ -1160,14 +1194,21 @@ trait BCodeBodyBuilder extends BCodeSkelBuilder { } def genLoadArguments(args: List[Tree], btpes: List[BType]): Unit = - args match - case arg :: args1 => - btpes match - case btpe :: btpes1 => - genLoad(arg, btpe) - genLoadArguments(args1, btpes1) - case _ => - case _ => + @tailrec def loop(args: List[Tree], btpes: List[BType]): Unit = + args match + case arg :: args1 => + btpes match + case btpe :: btpes1 => + genLoad(arg, btpe) + stackHeight += btpe.size + loop(args1, btpes1) + case _ => + case _ => + + val savedStackHeight = stackHeight + loop(args, btpes) + stackHeight = savedStackHeight + end genLoadArguments def genLoadModule(tree: Tree): BType = { val module = ( @@ -1266,11 +1307,14 @@ trait BCodeBodyBuilder extends BCodeSkelBuilder { }.sum bc.genNewStringBuilder(approxBuilderSize) + stackHeight += 1 // during the genLoad below, there is a reference to the StringBuilder on the stack for (elem <- concatArguments) { val elemType = tpeTK(elem) genLoad(elem, elemType) bc.genStringBuilderAppend(elemType) } + stackHeight -= 1 + bc.genStringBuilderEnd } else { @@ -1287,12 +1331,15 @@ trait BCodeBodyBuilder extends BCodeSkelBuilder { var totalArgSlots = 0 var countConcats = 1 // ie. 1 + how many times we spilled + val savedStackHeight = stackHeight + for (elem <- concatArguments) { val tpe = tpeTK(elem) val elemSlots = tpe.size // Unlikely spill case if (totalArgSlots + elemSlots >= MaxIndySlots) { + stackHeight = savedStackHeight + countConcats bc.genIndyStringConcat(recipe.toString, argTypes.result(), constVals.result()) countConcats += 1 totalArgSlots = 0 @@ -1317,8 +1364,10 @@ trait BCodeBodyBuilder extends BCodeSkelBuilder { val tpe = tpeTK(elem) argTypes += tpe.toASMType genLoad(elem, tpe) + stackHeight += 1 } } + stackHeight = savedStackHeight bc.genIndyStringConcat(recipe.toString, argTypes.result(), constVals.result()) // If we spilled, generate one final concat @@ -1513,7 +1562,9 @@ trait BCodeBodyBuilder extends BCodeSkelBuilder { } else { val tk = tpeTK(l).maxType(tpeTK(r)) genLoad(l, tk) + stackHeight += tk.size genLoad(r, tk) + stackHeight -= tk.size genCJUMP(success, failure, op, tk, targetIfNoJump) } } @@ -1628,7 +1679,9 @@ trait BCodeBodyBuilder extends BCodeSkelBuilder { } genLoad(l, ObjectRef) + stackHeight += 1 genLoad(r, ObjectRef) + stackHeight -= 1 genCallMethod(equalsMethod, InvokeStyle.Static) genCZJUMP(success, failure, Primitives.NE, BOOL, targetIfNoJump) } @@ -1644,7 +1697,9 @@ trait BCodeBodyBuilder extends BCodeSkelBuilder { } else if (isNonNullExpr(l)) { // SI-7852 Avoid null check if L is statically non-null. genLoad(l, ObjectRef) + stackHeight += 1 genLoad(r, ObjectRef) + stackHeight -= 1 genCallMethod(defn.Any_equals, InvokeStyle.Virtual) genCZJUMP(success, failure, Primitives.NE, BOOL, targetIfNoJump) } else { @@ -1654,7 +1709,9 @@ trait BCodeBodyBuilder extends BCodeSkelBuilder { val lNonNull = new asm.Label genLoad(l, ObjectRef) + stackHeight += 1 genLoad(r, ObjectRef) + stackHeight -= 1 locals.store(eqEqTempLocal) bc dup ObjectRef genCZJUMP(lNull, lNonNull, Primitives.EQ, ObjectRef, targetIfNoJump = lNull) diff --git a/compiler/src/dotty/tools/backend/jvm/BCodeIdiomatic.scala b/compiler/src/dotty/tools/backend/jvm/BCodeIdiomatic.scala index 2d4b22a10527..b86efb7cacb1 100644 --- a/compiler/src/dotty/tools/backend/jvm/BCodeIdiomatic.scala +++ b/compiler/src/dotty/tools/backend/jvm/BCodeIdiomatic.scala @@ -620,6 +620,16 @@ trait BCodeIdiomatic { // can-multi-thread final def drop(tk: BType): Unit = { emit(if (tk.isWideType) Opcodes.POP2 else Opcodes.POP) } + // can-multi-thread + final def dropMany(size: Int): Unit = { + var s = size + while s >= 2 do + emit(Opcodes.POP2) + s -= 2 + if s > 0 then + emit(Opcodes.POP) + } + // can-multi-thread final def dup(tk: BType): Unit = { emit(if (tk.isWideType) Opcodes.DUP2 else Opcodes.DUP) } diff --git a/compiler/src/dotty/tools/backend/jvm/BCodeSkelBuilder.scala b/compiler/src/dotty/tools/backend/jvm/BCodeSkelBuilder.scala index 1d8a9c579cb9..1885210a6687 100644 --- a/compiler/src/dotty/tools/backend/jvm/BCodeSkelBuilder.scala +++ b/compiler/src/dotty/tools/backend/jvm/BCodeSkelBuilder.scala @@ -45,7 +45,7 @@ trait BCodeSkelBuilder extends BCodeHelpers { /** The value is put on the stack, and control flows through to the next opcode. */ case FallThrough /** The value is put on the stack, and control flow is transferred to the given `label`. */ - case Jump(label: asm.Label) + case Jump(label: asm.Label, targetStackHeight: Int) /** The value is RETURN'ed from the enclosing method. */ case Return /** The value is ATHROW'n. */ @@ -368,6 +368,8 @@ trait BCodeSkelBuilder extends BCodeHelpers { // used by genLoadTry() and genSynchronized() var earlyReturnVar: Symbol = null var shouldEmitCleanup = false + // stack tracking + var stackHeight = 0 // line numbers var lastEmittedLineNr = -1 @@ -504,6 +506,13 @@ trait BCodeSkelBuilder extends BCodeHelpers { loc } + def makeTempLocal(tk: BType): Local = + assert(nxtIdx != -1, "not a valid start index") + assert(tk.size > 0, "makeLocal called for a symbol whose type is Unit.") + val loc = Local(tk, "temp", nxtIdx, isSynth = true) + nxtIdx += tk.size + loc + // not to be confused with `fieldStore` and `fieldLoad` which also take a symbol but a field-symbol. def store(locSym: Symbol): Unit = { val Local(tk, _, idx, _) = slots(locSym) @@ -574,6 +583,8 @@ trait BCodeSkelBuilder extends BCodeHelpers { earlyReturnVar = null shouldEmitCleanup = false + stackHeight = 0 + lastEmittedLineNr = -1 } From 009d623bf96174f1c138a528f0f3794242f76091 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Doeraene?= Date: Thu, 12 Jan 2023 17:02:37 +0100 Subject: [PATCH 21/33] DropBreaks does not track stack changes anymore. The back-end can now handle labeled-returns across stack differences, so we can emit optimized blocks even in those situations. --- compiler/src/dotty/tools/dotc/transform/DropBreaks.scala | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/transform/DropBreaks.scala b/compiler/src/dotty/tools/dotc/transform/DropBreaks.scala index 4ce5d53ffc2e..ac9232f3170b 100644 --- a/compiler/src/dotty/tools/dotc/transform/DropBreaks.scala +++ b/compiler/src/dotty/tools/dotc/transform/DropBreaks.scala @@ -162,7 +162,7 @@ class DropBreaks extends MiniPhase, RecordStackChange: * source and target of the jump */ protected def stackChange(using Context) = - if enclosingBoundaries == 0 then ctx else shadowLabels + ctx // if enclosingBoundaries == 0 then ctx else shadowLabels /** Need to suppress labeled returns if there is an intervening try */ @@ -178,6 +178,10 @@ class DropBreaks extends MiniPhase, RecordStackChange: case Break(_, _) => ctx case _ => stackChange + override def prepareForValDef(tree: ValDef)(using Context): Context = + if tree.symbol.is(Lazy) && tree.symbol.owner == ctx.owner.enclosingMethod then shadowLabels + else ctx + // other stack changing operations are handled in RecordStackChange /** If `tree` is a BreakBoundary, transform it as follows: From 0c1b9b5bbebcca9cee271413d845f58bdaf0468b Mon Sep 17 00:00:00 2001 From: odersky Date: Thu, 12 Jan 2023 21:11:01 +0100 Subject: [PATCH 22/33] Fix test --- tests/run/rescue-boundary.scala | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/run/rescue-boundary.scala b/tests/run/rescue-boundary.scala index 7d472aacaf1d..59f68bbd7c4e 100644 --- a/tests/run/rescue-boundary.scala +++ b/tests/run/rescue-boundary.scala @@ -1,11 +1,11 @@ -import scala.util.control.{ControlException, NonFatal} +import scala.util.control.NonFatal import scala.util.* object lib: extension [T](op: => T) inline def rescue (fallback: => T) = try op catch - case ex: ControlException => throw ex + case ex: boundary.Break => throw ex case NonFatal(_) => fallback extension [T, E <: Throwable](op: => T) inline def rescue (fallback: PartialFunction[E, T]) = From 2e3c4cfeb2e4d059500043af121724d059e518ae Mon Sep 17 00:00:00 2001 From: odersky Date: Thu, 12 Jan 2023 21:18:21 +0100 Subject: [PATCH 23/33] Revert "Suppress returns that cross different stack sizes." This reverts commit 04e7bc1ff5ec5b12cc1f5dcaf6b6696369dbccff. --- .../tools/dotc/transform/DropBreaks.scala | 43 +++++++++++-------- .../dotty/tools/dotc/transform/LiftTry.scala | 29 +++++++++++-- .../dotc/transform/RecordStackChange.scala | 41 ------------------ tests/run/rescue-boundary.scala | 2 +- 4 files changed, 52 insertions(+), 63 deletions(-) delete mode 100644 compiler/src/dotty/tools/dotc/transform/RecordStackChange.scala diff --git a/compiler/src/dotty/tools/dotc/transform/DropBreaks.scala b/compiler/src/dotty/tools/dotc/transform/DropBreaks.scala index ac9232f3170b..017d3db4a6ef 100644 --- a/compiler/src/dotty/tools/dotc/transform/DropBreaks.scala +++ b/compiler/src/dotty/tools/dotc/transform/DropBreaks.scala @@ -38,7 +38,7 @@ object DropBreaks: * - the throw and the boundary are in the same method, and * - there is no try expression inside the boundary that encloses the throw. */ -class DropBreaks extends MiniPhase, RecordStackChange: +class DropBreaks extends MiniPhase: import DropBreaks.* import tpd._ @@ -158,12 +158,6 @@ class DropBreaks extends MiniPhase, RecordStackChange: ctx.fresh.setProperty(ShadowedLabels, setSoFar ++ usesMap.keysIterator) case _ => ctx - /** Need to suppress labeled returns if the stack can change between - * source and target of the jump - */ - protected def stackChange(using Context) = - ctx // if enclosingBoundaries == 0 then ctx else shadowLabels - /** Need to suppress labeled returns if there is an intervening try */ override def prepareForTry(tree: Try)(using Context): Context = @@ -172,18 +166,13 @@ class DropBreaks extends MiniPhase, RecordStackChange: case LabelTry(_, _) => ctx case _ => shadowLabels - override def prepareForApply(tree: Apply)(using Context): Context = - if enclosingBoundaries == 0 then ctx - else tree match - case Break(_, _) => ctx - case _ => stackChange - override def prepareForValDef(tree: ValDef)(using Context): Context = - if tree.symbol.is(Lazy) && tree.symbol.owner == ctx.owner.enclosingMethod then shadowLabels + if enclosingBoundaries != 0 + && tree.symbol.is(Lazy) + && tree.symbol.owner == ctx.owner.enclosingMethod + then shadowLabels // RHS be converted to a lambda else ctx - // other stack changing operations are handled in RecordStackChange - /** If `tree` is a BreakBoundary, transform it as follows: * - Wrap it in a labeled block if its label has local uses * - Drop the try/catch if its label has no other uses @@ -203,7 +192,27 @@ class DropBreaks extends MiniPhase, RecordStackChange: case _ => tree - /** Rewrite a break with argument `arg` and label `lbl` + private def isBreak(sym: Symbol)(using Context): Boolean = + sym.name == nme.apply && sym.owner == defn.breakModule.moduleClass + + private def transformBreak(tree: Tree, arg: Tree, lbl: Symbol)(using Context): Tree = + report.log(i"transform break $tree/$arg/$lbl") + labelUsage(lbl) match + case Some(uses: LabelUsage) + if uses.enclMeth == ctx.owner.enclosingMethod + && !ctx.property(ShadowedLabels).getOrElse(Set.empty).contains(lbl) + => + uses.otherRefs -= 1 + uses.returnRefs += 1 + Return(arg, ref(uses.goto)).withSpan(arg.span) + case _ => + tree + + + /** Rewrite a break call + * + * break.apply[...](value)(using lbl) + * * where `lbl` is a label defined in the current method and is not included in * ShadowedLabels to * diff --git a/compiler/src/dotty/tools/dotc/transform/LiftTry.scala b/compiler/src/dotty/tools/dotc/transform/LiftTry.scala index 2c31d981d2fc..6acb1013d509 100644 --- a/compiler/src/dotty/tools/dotc/transform/LiftTry.scala +++ b/compiler/src/dotty/tools/dotc/transform/LiftTry.scala @@ -6,8 +6,8 @@ import core.DenotTransformers._ import core.Symbols._ import core.Contexts._ import core.Types._ -import core.Decorators._ import core.Flags._ +import core.Decorators._ import core.NameKinds.LiftedTreeName import NonLocalReturns._ import util.Store @@ -27,7 +27,7 @@ import util.Store * after an exception, so the fact that values on the stack are 'lost' does not matter * (copied from https://github.com/scala/scala/pull/922). */ -class LiftTry extends MiniPhase, IdentityDenotTransformer, RecordStackChange { thisPhase => +class LiftTry extends MiniPhase with IdentityDenotTransformer { thisPhase => import ast.tpd._ override def phaseName: String = LiftTry.name @@ -40,14 +40,35 @@ class LiftTry extends MiniPhase, IdentityDenotTransformer, RecordStackChange { t override def initContext(ctx: FreshContext): Unit = NeedLift = ctx.addLocation(false) - private def liftingCtx(p: Boolean)(using Context): Context = + private def liftingCtx(p: Boolean)(using Context) = if (needLift == p) ctx else ctx.fresh.updateStore(NeedLift, p) - protected def stackChange(using Context): Context = liftingCtx(true) + override def prepareForApply(tree: Apply)(using Context): Context = + liftingCtx(true) override def prepareForDefDef(tree: DefDef)(using Context): Context = liftingCtx(false) + override def prepareForValDef(tree: ValDef)(using Context): Context = + if !tree.symbol.exists + || tree.symbol.isSelfSym + || tree.symbol.owner == ctx.owner.enclosingMethod + && !tree.symbol.is(Lazy) + // The current implementation wraps initializers of lazy vals in + // calls to an initialize method, which means that a `try` in the + // initializer needs to be lifted. Note that the new scheme proposed + // in #6979 would avoid this. + then ctx + else liftingCtx(true) + + override def prepareForAssign(tree: Assign)(using Context): Context = + if (tree.lhs.symbol.maybeOwner == ctx.owner.enclosingMethod) ctx + else liftingCtx(true) + + override def prepareForReturn(tree: Return)(using Context): Context = + if (!isNonLocalReturn(tree)) ctx + else liftingCtx(true) + override def prepareForTemplate(tree: Template)(using Context): Context = liftingCtx(false) diff --git a/compiler/src/dotty/tools/dotc/transform/RecordStackChange.scala b/compiler/src/dotty/tools/dotc/transform/RecordStackChange.scala deleted file mode 100644 index bb24cabea018..000000000000 --- a/compiler/src/dotty/tools/dotc/transform/RecordStackChange.scala +++ /dev/null @@ -1,41 +0,0 @@ -package dotty.tools.dotc -package transform - -import MegaPhase.MiniPhase -import core.* -import Contexts.* -import Flags.Lazy -import NonLocalReturns.isNonLocalReturn - -/** A trait shared between LiftTry and DropBreaks. - * Overrides prepare methods for trees that push to the stack before some of their elements - * are evaluated. - */ -trait RecordStackChange extends MiniPhase: - import ast.tpd.* - - /** The method to call to record a stack change */ - protected def stackChange(using Context): Context - - override def prepareForApply(tree: Apply)(using Context): Context = - stackChange - - override def prepareForValDef(tree: ValDef)(using Context): Context = - if !tree.symbol.exists - || tree.symbol.isSelfSym - || tree.symbol.owner == ctx.owner.enclosingMethod - && !tree.symbol.is(Lazy) - // The current implementation wraps initializers of lazy vals in - // calls to an initialize method, which means that a `try` in the - // initializer needs to be lifted. Note that the new scheme proposed - // in #6979 would avoid this. - then ctx - else stackChange - - override def prepareForAssign(tree: Assign)(using Context): Context = - if (tree.lhs.symbol.maybeOwner == ctx.owner.enclosingMethod) ctx - else stackChange - - override def prepareForReturn(tree: Return)(using Context): Context = - if (!isNonLocalReturn(tree)) ctx - else stackChange \ No newline at end of file diff --git a/tests/run/rescue-boundary.scala b/tests/run/rescue-boundary.scala index 59f68bbd7c4e..81b868eadc7d 100644 --- a/tests/run/rescue-boundary.scala +++ b/tests/run/rescue-boundary.scala @@ -5,7 +5,7 @@ object lib: extension [T](op: => T) inline def rescue (fallback: => T) = try op catch - case ex: boundary.Break => throw ex + case ex: boundary.Break[_] => throw ex case NonFatal(_) => fallback extension [T, E <: Throwable](op: => T) inline def rescue (fallback: PartialFunction[E, T]) = From cf8daf0a9752042919c63b8758a15c84268ee008 Mon Sep 17 00:00:00 2001 From: odersky Date: Mon, 16 Jan 2023 16:48:08 +0100 Subject: [PATCH 24/33] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Sébastien Doeraene --- library/src/scala/util/boundary.scala | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/library/src/scala/util/boundary.scala b/library/src/scala/util/boundary.scala index 563a068b3a8c..08c26bdb2b1e 100644 --- a/library/src/scala/util/boundary.scala +++ b/library/src/scala/util/boundary.scala @@ -18,14 +18,14 @@ object boundary: /** User code should call `break.apply` instead of throwing this exception * directly. */ - class Break[T] private[boundary](val label: Label[T], val value: T) + final class Break[T] private[boundary](val label: Label[T], val value: T) extends RuntimeException( /*message*/ null, /*cause*/ null, /*enableSuppression=*/ false, /*writableStackTrace*/ false) /** Labels are targets indicating which boundary will be exited by a `break`. */ final class Label[-T]: - def break(value: T): Nothing = throw Break(this, value) + private[util] def break(value: T): Nothing = throw Break(this, value) /** Run `body` with freshly generated label as implicit argument. Catch any * breaks associated with that label and return their results instead of From 211ddb7adc763cbc0ffd2ae4c07a093ed1ed1038 Mon Sep 17 00:00:00 2001 From: odersky Date: Mon, 16 Jan 2023 16:53:56 +0100 Subject: [PATCH 25/33] Deprecate NonLocalReturns object --- library/src/scala/util/control/NonLocalReturns.scala | 1 + 1 file changed, 1 insertion(+) diff --git a/library/src/scala/util/control/NonLocalReturns.scala b/library/src/scala/util/control/NonLocalReturns.scala index 7c2c6584c489..3acb22b74ccf 100644 --- a/library/src/scala/util/control/NonLocalReturns.scala +++ b/library/src/scala/util/control/NonLocalReturns.scala @@ -17,6 +17,7 @@ package scala.util.control * performant, since returns within the scope of the same method can be * rewritten by the compiler to jumps. */ +@deprecated("Use scala.util.boundary and scala.util.break instead", "3.3") object NonLocalReturns { @deprecated("Use scala.util.boundary.Break instead", "3.3") class ReturnThrowable[T] extends ControlThrowable { From a8f81ddf3c9e69b46c1bcb01ef4d0b8433869a83 Mon Sep 17 00:00:00 2001 From: odersky Date: Wed, 18 Jan 2023 14:53:26 +0100 Subject: [PATCH 26/33] Fix test --- tests/run/errorhandling/kostas.scala | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/run/errorhandling/kostas.scala b/tests/run/errorhandling/kostas.scala index d76349530277..675c4afeb837 100644 --- a/tests/run/errorhandling/kostas.scala +++ b/tests/run/errorhandling/kostas.scala @@ -1,5 +1,5 @@ package optionMockup: - import scala.util.boundary + import scala.util.{boundary, break} object optional: transparent inline def apply[T](inline body: boundary.Label[None.type] ?=> T): Option[T] = boundary(Some(body)) @@ -7,7 +7,7 @@ package optionMockup: extension [T](r: Option[T]) transparent inline def ? (using label: boundary.Label[None.type]): T = r match case Some(x) => x - case None => label.break(None) + case None => break(None) import optionMockup.* From 335d18d9477d48cf694fb875124cbfe9ea2cfea6 Mon Sep 17 00:00:00 2001 From: Nicolas Stucki Date: Tue, 10 Jan 2023 15:08:41 +0100 Subject: [PATCH 27/33] Add boundary break optimization tests --- .../backend/jvm/LabelBytecodeTests.scala | 166 ++++++++++++++++++ 1 file changed, 166 insertions(+) create mode 100644 compiler/test/dotty/tools/backend/jvm/LabelBytecodeTests.scala diff --git a/compiler/test/dotty/tools/backend/jvm/LabelBytecodeTests.scala b/compiler/test/dotty/tools/backend/jvm/LabelBytecodeTests.scala new file mode 100644 index 000000000000..779098893360 --- /dev/null +++ b/compiler/test/dotty/tools/backend/jvm/LabelBytecodeTests.scala @@ -0,0 +1,166 @@ +package dotty.tools.backend.jvm + +import scala.language.unsafeNulls + +import org.junit.Assert._ +import org.junit.Test + +import scala.tools.asm +import asm._ +import asm.tree._ + +import scala.tools.asm.Opcodes +import scala.jdk.CollectionConverters._ +import Opcodes._ + +class LabelBytecodeTests extends DottyBytecodeTest { + import ASMConverters._ + + @Test def localLabelBreak = { + testLabelBytecodeEquals( + """val local = boundary.Label[Long]() + |try break(5L)(using local) + |catch case ex: boundary.Break[Long] @unchecked => + | if ex.label eq local then ex.value + | else throw ex + """.stripMargin, + "Long", + Ldc(LDC, 5), + Op(LRETURN) + ) + } + + @Test def simpleBoundaryBreak = { + testLabelBytecodeEquals( + """boundary: l ?=> + | break(2)(using l) + """.stripMargin, + "Int", + Op(ICONST_2), + Op(IRETURN) + ) + + testLabelBytecodeEquals( + """boundary: + | break(3) + """.stripMargin, + "Int", + Op(ICONST_3), + Op(IRETURN) + ) + + testLabelBytecodeEquals( + """boundary: + | break() + """.stripMargin, + "Unit", + Op(RETURN) + ) + } + + @Test def labelExtraction = { + // Test extra Inlined around the label + testLabelBytecodeEquals( + """boundary: + | break(2)(using summon[boundary.Label[Int]]) + """.stripMargin, + "Int", + Op(ICONST_2), + Op(IRETURN) + ) + + // Test extra Block around the label + testLabelBytecodeEquals( + """boundary: l ?=> + | break(2)(using { l }) + """.stripMargin, + "Int", + Op(ICONST_2), + Op(IRETURN) + ) + } + + @Test def boundaryLocalBreak = { + testLabelBytecodeExpect( + """val x: Boolean = true + |boundary[Unit]: + | var i = 0 + | while true do + | i += 1 + | if i > 10 then break() + """.stripMargin, + "Unit", + !throws(_) + ) + } + + @Test def boundaryNonLocalBreak = { + testLabelBytecodeExpect( + """boundary[Unit]: + | nonLocalBreak() + """.stripMargin, + "Unit", + throws + ) + + testLabelBytecodeExpect( + """boundary[Unit]: + | def f() = break() + | f() + """.stripMargin, + "Unit", + throws + ) + } + + @Test def boundaryLocalAndNonLocalBreak = { + testLabelBytecodeExpect( + """boundary[Unit]: l ?=> + | break() + | nonLocalBreak() + """.stripMargin, + "Unit", + throws + ) + } + + private def throws(instructions: List[Instruction]): Boolean = + instructions.exists { + case Op(ATHROW) => true + case _ => false + } + + private def testLabelBytecodeEquals(code: String, tpe: String, expected: Instruction*): Unit = + checkLabelBytecodeInstructions(code, tpe) { instructions => + val expectedList = expected.toList + assert(instructions == expectedList, + "`test` was not properly generated\n" + diffInstructions(instructions, expectedList)) + } + + private def testLabelBytecodeExpect(code: String, tpe: String, expected: List[Instruction] => Boolean): Unit = + checkLabelBytecodeInstructions(code, tpe) { instructions => + assert(expected(instructions), + "`test` was not properly generated\n" + instructions) + } + + private def checkLabelBytecodeInstructions(code: String, tpe: String)(checkOutput: List[Instruction] => Unit): Unit = { + val source = + s"""import scala.util.* + |class Test: + | def test: $tpe = { + | ${code.lines().toList().asScala.mkString("", "\n ", "")} + | } + | def nonLocalBreak[T](value: T)(using boundary.Label[T]): Nothing = break(value) + | def nonLocalBreak()(using boundary.Label[Unit]): Nothing = break(()) + """.stripMargin + + checkBCode(source) { dir => + val clsIn = dir.lookupName("Test.class", directory = false).input + val clsNode = loadClassNode(clsIn) + val method = getMethod(clsNode, "test") + + checkOutput(instructionsFromMethod(method)) + } + } + +} From 6dded2c557fc28a6e0a399adb1670234061c5179 Mon Sep 17 00:00:00 2001 From: odersky Date: Wed, 18 Jan 2023 17:30:56 +0100 Subject: [PATCH 28/33] Refactoring of break methiod calls Move the detected `break` methods into the `boundary` object and keep inline methods in object `scala.util.break` as facades. --- .../src/dotty/tools/dotc/core/Definitions.scala | 2 +- .../src/dotty/tools/dotc/core/StdNames.scala | 1 + .../dotty/tools/dotc/transform/DropBreaks.scala | 4 ++-- library/src/scala/util/boundary.scala | 15 +++++++++++++-- library/src/scala/util/break.scala | 16 ++++++++++------ 5 files changed, 27 insertions(+), 11 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/Definitions.scala b/compiler/src/dotty/tools/dotc/core/Definitions.scala index db982dc3fd88..13a2e1559b06 100644 --- a/compiler/src/dotty/tools/dotc/core/Definitions.scala +++ b/compiler/src/dotty/tools/dotc/core/Definitions.scala @@ -968,9 +968,9 @@ class Definitions { def TupledFunctionClass(using Context): ClassSymbol = TupledFunctionTypeRef.symbol.asClass def RuntimeTupleFunctionsModule(using Context): Symbol = requiredModule("scala.runtime.TupledFunctions") + @tu lazy val boundaryModule: Symbol = requiredModule("scala.util.boundary") @tu lazy val LabelClass: Symbol = requiredClass("scala.util.boundary.Label") @tu lazy val BreakClass: Symbol = requiredClass("scala.util.boundary.Break") - @tu lazy val breakModule: Symbol = requiredModule("scala.util.break") @tu lazy val CapsModule: Symbol = requiredModule("scala.caps") @tu lazy val captureRoot: TermSymbol = CapsModule.requiredValue("*") diff --git a/compiler/src/dotty/tools/dotc/core/StdNames.scala b/compiler/src/dotty/tools/dotc/core/StdNames.scala index 578852a06839..d45901829dd2 100644 --- a/compiler/src/dotty/tools/dotc/core/StdNames.scala +++ b/compiler/src/dotty/tools/dotc/core/StdNames.scala @@ -421,6 +421,7 @@ object StdNames { val assert_ : N = "assert" val assume_ : N = "assume" val box: N = "box" + val break: N = "break" val build : N = "build" val bundle: N = "bundle" val bytes: N = "bytes" diff --git a/compiler/src/dotty/tools/dotc/transform/DropBreaks.scala b/compiler/src/dotty/tools/dotc/transform/DropBreaks.scala index 017d3db4a6ef..8c4cfab58b3c 100644 --- a/compiler/src/dotty/tools/dotc/transform/DropBreaks.scala +++ b/compiler/src/dotty/tools/dotc/transform/DropBreaks.scala @@ -105,7 +105,7 @@ class DropBreaks extends MiniPhase: private object Break: private def isBreak(sym: Symbol)(using Context): Boolean = - sym.name == nme.apply && sym.owner == defn.breakModule.moduleClass + sym.name == nme.break && sym.owner == defn.boundaryModule.moduleClass /** `(local, arg)` provided `tree` matches * @@ -193,7 +193,7 @@ class DropBreaks extends MiniPhase: tree private def isBreak(sym: Symbol)(using Context): Boolean = - sym.name == nme.apply && sym.owner == defn.breakModule.moduleClass + sym.name == nme.break && sym.owner == defn.boundaryModule.moduleClass private def transformBreak(tree: Tree, arg: Tree, lbl: Symbol)(using Context): Tree = report.log(i"transform break $tree/$arg/$lbl") diff --git a/library/src/scala/util/boundary.scala b/library/src/scala/util/boundary.scala index 08c26bdb2b1e..66a93da60600 100644 --- a/library/src/scala/util/boundary.scala +++ b/library/src/scala/util/boundary.scala @@ -24,8 +24,19 @@ object boundary: /** Labels are targets indicating which boundary will be exited by a `break`. */ - final class Label[-T]: - private[util] def break(value: T): Nothing = throw Break(this, value) + final class Label[-T] + + /** Abort current computation and instead return `value` as the value of + * the enclosing `boundary` call that created `label`. + */ + def break[T](value: T)(using label: Label[T]): Nothing = + throw Break(label, value) + + /** Abort current computation and instead continue after the `boundary` call that + * created `label`. + */ + def break()(using label: Label[Unit]): Nothing = + throw Break(label, ()) /** Run `body` with freshly generated label as implicit argument. Catch any * breaks associated with that label and return their results instead of diff --git a/library/src/scala/util/break.scala b/library/src/scala/util/break.scala index 3bf13c1e8072..54abbc6bdba2 100644 --- a/library/src/scala/util/break.scala +++ b/library/src/scala/util/break.scala @@ -6,15 +6,19 @@ package scala.util object break: /** Abort current computation and instead return `value` as the value of - * the enclosing `boundary` call that created `label`. + * the enclosing `boundary` call that created `label`. Expands to + * + * boundary.break(value)(using label) */ - def apply[T](value: T)(using label: boundary.Label[T]): Nothing = - label.break(value) + inline def apply[T](value: T)(using label: boundary.Label[T]): Nothing = + boundary.break(value) /** Abort current computation and instead continue after the `boundary` call that - * created `label`. + * created `label`. Expands to + * + * boundary.break()(using label) */ - def apply()(using label: boundary.Label[Unit]): Nothing = - label.break(()) + inline def apply()(using label: boundary.Label[Unit]): Nothing = + boundary.break() end break From fcef230e6f7f96063f157e1ad3ad57532216b0b2 Mon Sep 17 00:00:00 2001 From: odersky Date: Wed, 18 Jan 2023 23:12:20 +0100 Subject: [PATCH 29/33] Fix bytecode test We can't use `String.lines()` since that only exists on Java 11 and the CI still runs on Java 8. Use `linesIterator` in `StringOps` instead. --- compiler/test/dotty/tools/backend/jvm/LabelBytecodeTests.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/test/dotty/tools/backend/jvm/LabelBytecodeTests.scala b/compiler/test/dotty/tools/backend/jvm/LabelBytecodeTests.scala index 779098893360..50e8a69ff0fc 100644 --- a/compiler/test/dotty/tools/backend/jvm/LabelBytecodeTests.scala +++ b/compiler/test/dotty/tools/backend/jvm/LabelBytecodeTests.scala @@ -148,7 +148,7 @@ class LabelBytecodeTests extends DottyBytecodeTest { s"""import scala.util.* |class Test: | def test: $tpe = { - | ${code.lines().toList().asScala.mkString("", "\n ", "")} + | ${code.linesIterator.toList.mkString("", "\n ", "")} | } | def nonLocalBreak[T](value: T)(using boundary.Label[T]): Nothing = break(value) | def nonLocalBreak()(using boundary.Label[Unit]): Nothing = break(()) From a17a6df11a0053fdd0455c1d9e39e905465d9bad Mon Sep 17 00:00:00 2001 From: odersky Date: Fri, 20 Jan 2023 14:20:16 +0100 Subject: [PATCH 30/33] Drop `break` object in `scala.util` --- .../backend/jvm/LabelBytecodeTests.scala | 2 +- library/src/scala/util/boundary.scala | 12 +++++++++- library/src/scala/util/break.scala | 24 ------------------- .../scala/util/control/NonLocalReturns.scala | 6 ++--- tests/pos/simple-boundary.scala | 2 +- tests/run/break-opt.scala | 2 +- tests/run/breaks.scala | 2 +- tests/run/errorhandling/Result.scala | 2 +- tests/run/errorhandling/Test.scala | 2 +- tests/run/errorhandling/kostas.scala | 2 +- tests/run/errorhandling/optional.scala | 2 +- tests/run/loops-alt.scala | 4 ++-- tests/run/loops.scala | 2 +- tests/run/rescue-boundary.scala | 2 +- 14 files changed, 26 insertions(+), 40 deletions(-) delete mode 100644 library/src/scala/util/break.scala diff --git a/compiler/test/dotty/tools/backend/jvm/LabelBytecodeTests.scala b/compiler/test/dotty/tools/backend/jvm/LabelBytecodeTests.scala index 50e8a69ff0fc..aea567b87f91 100644 --- a/compiler/test/dotty/tools/backend/jvm/LabelBytecodeTests.scala +++ b/compiler/test/dotty/tools/backend/jvm/LabelBytecodeTests.scala @@ -145,7 +145,7 @@ class LabelBytecodeTests extends DottyBytecodeTest { private def checkLabelBytecodeInstructions(code: String, tpe: String)(checkOutput: List[Instruction] => Unit): Unit = { val source = - s"""import scala.util.* + s"""import scala.util.boundary, boundary.break |class Test: | def test: $tpe = { | ${code.linesIterator.toList.mkString("", "\n ", "")} diff --git a/library/src/scala/util/boundary.scala b/library/src/scala/util/boundary.scala index 66a93da60600..3c6c6982c7ee 100644 --- a/library/src/scala/util/boundary.scala +++ b/library/src/scala/util/boundary.scala @@ -11,7 +11,17 @@ package scala.util * The `Break` exception class extends `RuntimeException` and is optimized so * that stack trace generation is suppressed. * - Better performance: breaks to enclosing scopes in the same method can - * be rwritten to jumps. + * be rewritten to jumps. + * + * Example usage: + * + * import scala.util.boundary, boundary.break + * + * def firstIndex[T](xs: List[T], elem: T): Int = + * boundary: + * for (x, i) <- xs.zipWithIndex do + * if x == elem then break(i) + * -1 */ object boundary: diff --git a/library/src/scala/util/break.scala b/library/src/scala/util/break.scala deleted file mode 100644 index 54abbc6bdba2..000000000000 --- a/library/src/scala/util/break.scala +++ /dev/null @@ -1,24 +0,0 @@ -package scala.util - -/** This object has two apply methods that abort the current computation - * up to an enclosing `boundary` call. - */ -object break: - - /** Abort current computation and instead return `value` as the value of - * the enclosing `boundary` call that created `label`. Expands to - * - * boundary.break(value)(using label) - */ - inline def apply[T](value: T)(using label: boundary.Label[T]): Nothing = - boundary.break(value) - - /** Abort current computation and instead continue after the `boundary` call that - * created `label`. Expands to - * - * boundary.break()(using label) - */ - inline def apply()(using label: boundary.Label[Unit]): Nothing = - boundary.break() - -end break diff --git a/library/src/scala/util/control/NonLocalReturns.scala b/library/src/scala/util/control/NonLocalReturns.scala index 3acb22b74ccf..ad4dc05f36ac 100644 --- a/library/src/scala/util/control/NonLocalReturns.scala +++ b/library/src/scala/util/control/NonLocalReturns.scala @@ -17,7 +17,7 @@ package scala.util.control * performant, since returns within the scope of the same method can be * rewritten by the compiler to jumps. */ -@deprecated("Use scala.util.boundary and scala.util.break instead", "3.3") +@deprecated("Use scala.util.boundary instead", "3.3") object NonLocalReturns { @deprecated("Use scala.util.boundary.Break instead", "3.3") class ReturnThrowable[T] extends ControlThrowable { @@ -30,12 +30,12 @@ object NonLocalReturns { } /** Performs a nonlocal return by throwing an exception. */ - @deprecated("Use scala.util.break and scala.util.boundary instead", "3.3") + @deprecated("Use scala.util.boundary.break instead", "3.3") def throwReturn[T](result: T)(using returner: ReturnThrowable[? >: T]): Nothing = returner.throwReturn(result) /** Enable nonlocal returns in `op`. */ - @deprecated("Use scala.util.boundary and scala.util.break instead", "3.3") + @deprecated("Use scala.util.boundary instead", "3.3") def returning[T](op: ReturnThrowable[T] ?=> T): T = { val returner = new ReturnThrowable[T] try op(using returner) diff --git a/tests/pos/simple-boundary.scala b/tests/pos/simple-boundary.scala index 8ec097706bc3..b7adaf4e11cf 100644 --- a/tests/pos/simple-boundary.scala +++ b/tests/pos/simple-boundary.scala @@ -1,4 +1,4 @@ -import scala.util.* +import scala.util.boundary, boundary.break def test: Unit = boundary: label ?=> while true do break() diff --git a/tests/run/break-opt.scala b/tests/run/break-opt.scala index c59a57d4bffc..ec979ff0e8ad 100644 --- a/tests/run/break-opt.scala +++ b/tests/run/break-opt.scala @@ -1,4 +1,4 @@ -import scala.util.* +import scala.util.boundary, boundary.break object breakOpt: diff --git a/tests/run/breaks.scala b/tests/run/breaks.scala index d6bddd881281..3036bc6c9124 100644 --- a/tests/run/breaks.scala +++ b/tests/run/breaks.scala @@ -1,4 +1,4 @@ -import scala.util.* +import scala.util.boundary, boundary.break import collection.mutable.ListBuffer object Test { diff --git a/tests/run/errorhandling/Result.scala b/tests/run/errorhandling/Result.scala index c44f5ff7bf64..027c07c86769 100644 --- a/tests/run/errorhandling/Result.scala +++ b/tests/run/errorhandling/Result.scala @@ -1,5 +1,5 @@ package scala.util -import boundary.Label +import boundary.{Label, break} abstract class Result[+T, +E] case class Ok[+T](value: T) extends Result[T, Nothing] diff --git a/tests/run/errorhandling/Test.scala b/tests/run/errorhandling/Test.scala index 7f510bb74e64..4aa1cd28c5aa 100644 --- a/tests/run/errorhandling/Test.scala +++ b/tests/run/errorhandling/Test.scala @@ -1,4 +1,4 @@ -import scala.util.* +import scala.util.*, boundary.break /** boundary/break as a replacement for non-local returns */ def indexOf[T](xs: List[T], elem: T): Int = diff --git a/tests/run/errorhandling/kostas.scala b/tests/run/errorhandling/kostas.scala index 675c4afeb837..085275d5cd82 100644 --- a/tests/run/errorhandling/kostas.scala +++ b/tests/run/errorhandling/kostas.scala @@ -1,5 +1,5 @@ package optionMockup: - import scala.util.{boundary, break} + import scala.util.boundary, boundary.break object optional: transparent inline def apply[T](inline body: boundary.Label[None.type] ?=> T): Option[T] = boundary(Some(body)) diff --git a/tests/run/errorhandling/optional.scala b/tests/run/errorhandling/optional.scala index 6eeac76108b0..e041834b825c 100644 --- a/tests/run/errorhandling/optional.scala +++ b/tests/run/errorhandling/optional.scala @@ -1,5 +1,5 @@ package scala.util -import boundary.Label +import boundary.{break, Label} /** A mockup of scala.Option */ abstract class Option[+T] diff --git a/tests/run/loops-alt.scala b/tests/run/loops-alt.scala index 69f5594bcdc1..19c3312bff50 100644 --- a/tests/run/loops-alt.scala +++ b/tests/run/loops-alt.scala @@ -1,5 +1,5 @@ -import scala.util.{boundary, break} -import boundary.Label +import scala.util.boundary +import boundary.{break, Label} import java.util.concurrent.TimeUnit object loop: diff --git a/tests/run/loops.scala b/tests/run/loops.scala index d1474d06f3a1..e6a9d3c98dbe 100644 --- a/tests/run/loops.scala +++ b/tests/run/loops.scala @@ -1,4 +1,4 @@ -import scala.util.{boundary, break} +import scala.util.boundary, boundary.break object loop: diff --git a/tests/run/rescue-boundary.scala b/tests/run/rescue-boundary.scala index 81b868eadc7d..3e7fe8508686 100644 --- a/tests/run/rescue-boundary.scala +++ b/tests/run/rescue-boundary.scala @@ -1,5 +1,5 @@ import scala.util.control.NonFatal -import scala.util.* +import scala.util.boundary, boundary.break object lib: extension [T](op: => T) inline def rescue (fallback: => T) = From 91bd5df79416c2315f2247db44df3952bc377b49 Mon Sep 17 00:00:00 2001 From: odersky Date: Fri, 20 Jan 2023 14:45:14 +0100 Subject: [PATCH 31/33] Drop `Label_this` and fix comments referring to it --- compiler/src/dotty/tools/dotc/core/StdNames.scala | 1 - compiler/src/dotty/tools/dotc/transform/DropBreaks.scala | 6 +++--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/StdNames.scala b/compiler/src/dotty/tools/dotc/core/StdNames.scala index d45901829dd2..7755b7877ca8 100644 --- a/compiler/src/dotty/tools/dotc/core/StdNames.scala +++ b/compiler/src/dotty/tools/dotc/core/StdNames.scala @@ -357,7 +357,6 @@ object StdNames { val Flag : N = "Flag" val Ident: N = "Ident" val Import: N = "Import" - val Label_this: N = "Label_this" val Literal: N = "Literal" val LiteralAnnotArg: N = "LiteralAnnotArg" val Matchable: N = "Matchable" diff --git a/compiler/src/dotty/tools/dotc/transform/DropBreaks.scala b/compiler/src/dotty/tools/dotc/transform/DropBreaks.scala index 8c4cfab58b3c..562dd967747c 100644 --- a/compiler/src/dotty/tools/dotc/transform/DropBreaks.scala +++ b/compiler/src/dotty/tools/dotc/transform/DropBreaks.scala @@ -218,10 +218,10 @@ class DropBreaks extends MiniPhase: * * return[target] arg * - * where `target` is the `goto` return label associated with `local`. + * where `target` is the `goto` return label associated with `lbl`. * Adjust associated ref counts accordingly. The local refcount is increased - * and the non-local refcount is decreased, since `local` the `Label_this` - * binding containing `local` is dropped. + * and the non-local refcount is decreased, since the `lbl` implicit argument + * to `break` is dropped. */ override def transformApply(tree: Apply)(using Context): Tree = if enclosingBoundaries == 0 then tree From c3cf035d7d619f3b1204e911d79fee0a2cccbc0d Mon Sep 17 00:00:00 2001 From: odersky Date: Fri, 20 Jan 2023 15:38:24 +0100 Subject: [PATCH 32/33] Make var private --- compiler/src/dotty/tools/dotc/transform/DropBreaks.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/transform/DropBreaks.scala b/compiler/src/dotty/tools/dotc/transform/DropBreaks.scala index 562dd967747c..3081bd5c2b20 100644 --- a/compiler/src/dotty/tools/dotc/transform/DropBreaks.scala +++ b/compiler/src/dotty/tools/dotc/transform/DropBreaks.scala @@ -51,7 +51,7 @@ class DropBreaks extends MiniPhase: // we want by-name parameters to be converted to closures /** The number of boundary nodes enclosing the currently analized tree. */ - var enclosingBoundaries: Int = 0 + private var enclosingBoundaries: Int = 0 private object LabelTry: From 69b7a4835d0373ad3ada998446acaf9084a05e98 Mon Sep 17 00:00:00 2001 From: odersky Date: Fri, 20 Jan 2023 15:48:36 +0100 Subject: [PATCH 33/33] Update migration warning message --- compiler/src/dotty/tools/dotc/transform/NonLocalReturns.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/transform/NonLocalReturns.scala b/compiler/src/dotty/tools/dotc/transform/NonLocalReturns.scala index 9fad2289da54..a75d6da9dd6a 100644 --- a/compiler/src/dotty/tools/dotc/transform/NonLocalReturns.scala +++ b/compiler/src/dotty/tools/dotc/transform/NonLocalReturns.scala @@ -97,7 +97,7 @@ class NonLocalReturns extends MiniPhase { override def transformReturn(tree: Return)(using Context): Tree = if isNonLocalReturn(tree) then report.gradualErrorOrMigrationWarning( - em"Non local returns are no longer supported; use `boundary` and `break` in `scala.util` instead", + em"Non local returns are no longer supported; use `boundary` and `boundary.break` in `scala.util` instead", tree.srcPos, warnFrom = `3.2`, errorFrom = future)