using the Breaks singleton safely

I agree that Break as it is currently implemented is broken.

What you propose is marginally better, but I think it's still too easy to use incorrectly. This code (note 'break' instead of 'break()'):

  breakable { break =>
    for (x <- Products) {
      if (x > 5)
        break
      println(x)
    }
  }

compiles just fine but contains a subtle bug.

I don't think 'break' and 'continue' should be a part of the standard library. If they want to be included, they should be language constructs which are statically checked (can only break/continue out of a 'for' loop) and statically handled, like 'return'.

If it is needed for collections, it should be made private[collections].

--j

On Sat, Oct 31, 2009 at 8:43 AM, Lex Spoon <lex@lexspoon.org> wrote:

I believe the Breaks singleton is unsafe.  The problem is that any call to Breaks.break will break out of the *dynamically* enclosing call to Breaks.breakable, but what people really mean is to break out of the "statically" enclosing call.  It's the confusion of dynamic lookup all over again, only this time for control flow rather than for variables!

Here's an example code snippet showing what I mean.  It has a weirdo Products class that uses an internal breakable/break as part of computing its sequence.    Later, user code tries to iterate over the sequence, and uses its own break call to try and abort the iteration once it sees a number larger than 5.  This code should print out "1 2 4 3", but because the user's break call is bound to the Products class's breakable, it prints out a few more numbers than that.


import Breaks._

object Products {
 val list1 = List(1, 2, 3, 4, 5, 6, 7, 8, 9, 10)
 val list2 = List(1, 2, 3, 4, 5, 6, 7, 8, 9, 10)

 def foreach(f: Int=>Unit) {
   for (i <- list1) {
     breakable {
       for (j <- list2) {
         if (j > i)
           break
         f (i * j)
       }
     }
   }
 }
}



breakable {
 for (x <- Products) {
   if (x > 5)
     break
   println(x)
 }
}


This is a trap.  Any two users of the Breaks singleton have to make sure that their static nesting order matches their dynamic nesting order, but how can they reason about that?

One way to fix things up would be to remove the Breaks singleton and beef up the comment on the Breaks class to explain the trap.  Users would be required to instantiate a new Breaks instance for each time they want to use breakable, taking care to instantiate the new instance at the correct nesting depth so that the instance is not reused.  This is syntactically heavy, however.  Further, while the library has no built-in trap, it's still an easy mistake to instantiate the Breaks instances in the wrong places.

A better looking option is to encode break in terms of return.  Scala's return expression has the exact same issues as break, and the compiler already has a fine solution for it.  Here's a complete implementation of breakable using that strategy:

def breakable(f: (Unit=>Nothing) => Unit) {
 f(() => return)
}


Using this definition, callers are required to pass a one-argument rather than zero-argument function to break, like this:

breakable { break =>
 for (x <- Products) {
   if (x > 5)
     break()
   println(x)
 }
}

The new definition looks good to me.  What do others think?  The downside is that there are two extra tokens per call to breakable.  However, there is a lot of upside.  Everything is statically bound, the source code is simple, and users don't have to think about how to use it safely.  Further, there is one implementation of non-local, statically bound return: the compiler's implementation of "return".  Since this is a subtle beastie to implement -- especially once you consider performance -- localizing it looks helpful.

By the way, this general discussion appears for many languages.  In addition to Scala encountering it, Java has to face it if it adds function literals, JavaScript has encountered it in recent efforts to get more things statically scoped, and the whole situation was a FAQ on the old Smalltalk newsgroups I used to read.  In all of these cases, the one path to sanity I've seen is to think of break, continue, and return as being named in all cases; the non-named versions are syntactic sugar that is immediately expanded during parsing.

Lex Spoon

Actually, the real implementation iof Break is a bit more general in
that it allows to define your own Break modules, each iwth its
seperate break exceptions. I agree that break still does dynamic
resolution, which needs to be documented. But it's still useful enough
to be made made available, IMO.

Cheers

At least don't call it 'break' if it doesn't act at all like Java's 'break'.

--j

On Sat, Oct 31, 2009 at 2:59 PM, martin odersky <martin.odersky@epfl.ch> wrote:

Actually, the real implementation iof Break is a bit more general in
that it allows to define your own Break modules, each iwth its
seperate break exceptions. I agree that break still does dynamic
resolution, which needs to be documented. But it's still useful enough
to be made made available, IMO.

Cheers

On Oct 31, 2009, at 5:59 PM, martin odersky wrote:
> Actually, the real implementation iof Break is a bit more general in
> that it allows to define your own Break modules, each iwth its
> seperate break exceptions. I agree that break still does dynamic
> resolution, which needs to be documented. But it's still useful enough
> to be made made available, IMO.

In that case, a name change would help. Dynamically resolved break
isn't really a break at all; it's more like exception throwing.

Jorge, I agree about the trap in the version I posted.

Lex

Hi. The trap in the version you posted can be removed by the following
definition of breakable
(a call-by-name function type (=> Nothing) => Unit is used):

def breakable(f: (=> Nothing) => Unit) { f({ return }) }

This version cannot be used incorrectly:

breakable { break =>
for (x <- Products) {
if (x > 5)
break //() is unneeded
println(x)
}
}

2009/11/2 Lex Spoon :
> On Oct 31, 2009, at 5:59 PM, martin odersky wrote:
>>
>> Actually, the real implementation iof Break is a bit more general in
>> that it allows to define your own Break modules, each iwth its
>> seperate break exceptions. I agree that break still does dynamic
>> resolution, which needs to be documented. But it's still useful enough
>> to be made made available, IMO.
>
> In that case, a name change would help.  Dynamically resolved break isn't
> really a break at all; it's more like exception throwing.
>
> Jorge, I agree about the trap in the version I posted.
>
>
> Lex
>
>
>