Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

loop-break-value (issue #961) #1624

Merged
merged 14 commits into from
Oct 22, 2016
Merged
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
216 changes: 216 additions & 0 deletions text/0000-loop-break-value.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,216 @@
- Feature Name: loop_break_value
- Start Date: 2016-05-20
- RFC PR: (leave this empty)
- Rust Issue: (leave this empty)

# Summary
[summary]: #summary

(This is a result of discussion of
[issue #961](https://github.com/rust-lang/rfcs/issues/961) and related to RFCs
[352](https://github.com/rust-lang/rfcs/pull/352) and
[955](https://github.com/rust-lang/rfcs/pull/955).)

Let a `loop { ... }` expression return a value via `break my_value;`.

# Motivation
[motivation]: #motivation

This pattern is currently hard to implement without resorting to a function or
closure wrapping the loop:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not that hard:

fn f() {
    let outcome;
    loop {
        // get and process some input, e.g. from the user or from a list of
        // files
        let result = get_result();

        if successful() {
            outcome = result;
            break;
        }
        // otherwise keep trying
    };

    use_the_result(outcome);
}

Copy link
Contributor Author

@dhardy dhardy May 20, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, you're right. I was able to reduce my problem to this pattern:

let computation = {
    let mut result = None;
    loop {
        if let Some(r) = self.do_something() {
            result = Some(s);
            break;
        }
    }
    result.unwrap().do_computation()
};
self.use(computation);

Lifetimes make some of these problems horrible to think about.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Damn. The problem with being British is you expect people to spot the sarcasm.

  1. Having to mutable a variable just to return a value is inelegant
  2. Having to wrap with Option then unwrap just because there isn't an appropriate default value is clumsy
  3. Having to do this while considering the lifetimes of generated references is confusing
  4. It's a lot of extra code. Compare the above to:
let computation = loop {
        if let Some(r) = self.do_something() {
            break r;
        }
    }.do_computation();
self.use(computation);

(noting that due to reference lifetimes it may be possible to give the value fed to do_computation() a name in the outer scope).

Is that sufficient motivation?

The new lifetime rules under discussion may well simplify this problem, both with-and-without break-with-value, but I still think break-with-value is a neat pattern (or alternately, a generic break-from-block-with-value, but that opens another can of worms: the value if not broken out of).

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, @sfackler 's example works fine without a mutable variable or an Option. Am I missing something?

Copy link
Contributor Author

@dhardy dhardy May 21, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe, and maybe it's a good thing. Sounds like it's time I un-learned to never leave variables uninitialised (thanks to Rust's great flow & lifetime analysis).

Points 3 and 4 still hold (the example above can be four words shorter), but this weakens the argument.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Point 3 - please give an example where these confusing-to-consider lifetimes are involved.
Point 4 - "a lot of extra code" when talking about essentially one extra assignment?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The current pattern is not necessarily hard, but the main problem I think is that the new pattern expresses much better what is going on.
It is immediately clear that the loop is meant to set the value, while with the code of @sfackler this is not te case. Especially when considering loops with more content or with multiple exit points.


```rust
fn f() {
let outcome = loop {
// get and process some input, e.g. from the user or from a list of
// files
let result = get_result();

if successful() {
break result;
}
// otherwise keep trying
};

use_the_result(outcome);
}
```

In some cases, one can simply move `use_the_result(outcome)` into the loop, but
sometimes this is undesirable and sometimes impossible due to lifetimes.

# Detailed design
[design]: #detailed-design

This proposal does two things: let `break` take a value, and let `loop` have a
result type other than `()`.

### Break Syntax

Four forms of `break` will be supported:

1. `break;`
2. `break 'label;`
3. `break EXPR;`
4. `break 'label EXPR;`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the ; after break EXPR compulsory? Currently existing forms of break do not require a semicolon after if they are in expression position.

Could we get an example of what would the grammar look like with syntax changes? See this and this.

Copy link

@vitiral vitiral Oct 7, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would say that the ; is not necessary. It should map how return works, and the following compiles:

fn test() -> bool {
    if true {
        return true
    } else {
        return false
    }
}

fn test2() -> bool {
    return false
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes just like today

---------------
(break 'a e): !


where `'label` is the name of a loop and `EXPR` is an expression.

### Result type of loop

Currently the result-type of a 'loop' without 'break' is `!` (never returns),
which may be coerced to any type), and the result type of a 'loop' with 'break'

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

consistency nit: "result-type" vs "result type"

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/'/`/

I will not nit on any more of the formatting, but the issues with it probably should be fixed across the document. Namely, plain apostrophe (') does not begin/end an inline code snippet.

is `()`. This is important since a loop may appear as
the last expression of a function:

```rust
fn f() {
loop {
do_something();
// never breaks
}
}
fn g() -> () {
loop {
do_something();
if Q() { break; }
}
}
fn h() -> ! {
loop {
do_something();
// this loop is not allowed to break due to inferred `!` type
}
}
```

This proposal changes the result type of 'loop' to `T`, where:

* if a loop is "broken" via `break;` or `break 'label;`, the loop's result type must be `()`
* if a loop is "broken" via `break EXPR;` or `break 'label EXPR;`, `EXPR` must evaluate to type `T`
* as a special case, if a loop is "broken" via `break EXPR;` or `break 'label EXPR;` where `EXPR` evaluates to type `!` (does not return), this does not place a constraint on the type of the loop

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see the need for this special case: there's no reason to write break EXPR; instead of EXPR; when the expr is diverging.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, and I suppose this is why it's not legal to type let x: ! = ...;.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, that's because ! is not a real type (yet - there is an RFC open for that) and is only allowed in function return type notation.

Copy link
Member

@nagisa nagisa Oct 4, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now ! is a legal type everywhere, but this special case is still useful as a note, even though it follows just fine from rules applicable to ! (i.e. ! coerces to any other type).

(EDIT: Weird, this was a reply to an existing thread on the same line; explains why it has no the reply field though…)

* if external constaint on the loop's result type exist (e.g. `let x: S = loop { ... };`), then `T` must be coercible to this type
Copy link
Member

@nagisa nagisa Oct 4, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This list still has issues with handling of coercion. For example I imagine something like this ought to be valid:

loop {
     // ...
     break (expr: SomeT);
     // ...
     break (expr: OtherT);
}: RT

where SomeT and OtherT both coerce to RT. If this discrepancy was fixed, the list could avoid having to contain the last two notes/requirements. I also dislike those two extra requirements-exceptions-rules, because they have potential to diverge from type rules elsewhere in the language.

(Why this matters? Substitute SomeT = &[u8; 42], OtherT = &[u8; 32], RT = &[u8].)

Copy link
Contributor

@Ericson2314 Ericson2314 Oct 5, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the typing and coersions rules here can be exactly like those for match arms or if-else.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please write a diff against the RFC; I was not 100% sure how to handle coercions (same with the ! special case).


It is an error if these types do not agree or if the compiler's type deduction
rules do not yield a concrete type.

Examples of errors:

```rust
// error: loop type must be () and must be i32
let a: i32 = loop { break; };
// error: loop type must be i32 and must be &str
let b: i32 = loop { break "I am not an integer."; };
// error: loop type must be Option<_> and must be &str
let c = loop {
if Q() {
break "answer";
} else {
break None;
}
};
fn z() -> ! {
// function does not return
// error: loop may break (same behaviour as before)
loop {
if Q() { break; }
}
}
```

Examples involving `!`:

```rust
fn f() -> () {
// ! coerces to ()
loop {}
}
fn g() -> u32 {
// ! coerces to u32
loop {}
}
fn z() -> ! {
loop {
break panic!();
}
}
```

Example showing the equivalence of `break;` and `break ();`:

```rust
fn y() -> () {
loop {
if coin_flip() {
break;
} else {
break ();
}
}
}
```

### Result value

A loop only yields a value if broken via some form of `break ...;` statement,
in which case it yields the value resulting from the evaulation of the
statement's expression (`EXPR` above), or `()` if there is no `EXPR`
expression.

Examples:

```rust
assert_eq!(loop { break; }, ());
assert_eq!(loop { break 5; }, 5);
let x = 'a loop {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

syntactic nit: this should be 'a: loop { and 'b: loop { on the line below.

'b loop {
break 'a 1;
}
break 'a 2;
};
assert_eq!(x, 1);
```

# Drawbacks
[drawbacks]: #drawbacks

The proposal changes the syntax of `break` statements, requiring updates to
parsers and possibly syntax highlighters.

# Alternatives
[alternatives]: #alternatives

No alternatives to the design have been suggested. It has been suggested that
the feature itself is unnecessary, and indeed much Rust code already exists
without it, however the pattern solves some cases which are difficult to handle
otherwise and allows more flexibility in code layout.

# Unresolved questions
[unresolved]: #unresolved-questions

It would be possible to allow `for`, `while` and `while let` expressions return
values in a similar way; however, these expressions may also terminate
"naturally" (not via break), and no consensus has been reached on how the
result value should be determined in this case, or even the result type.
It is thus proposed not to change these expressions at this time.

It should be noted that `for`, `while` and `while let` can all be emulated via
`loop`, so perhaps allowing the former to return values is less important.
Alternatively, a new keyword such as `default` or `else` could be used to
specify the other exit value as in:

```rust
fn first<T: Copy>(list: Iterator<T>) -> Option<T> {
for x in list {
break Some(x);
} default {
None
}
}
```

The exact syntax is disputed. It is suggested that this RFC should not be
blocked on this issue since break-with-value can still be implemented in the
manner above after this RFC. See the
[discussion of #961](https://github.com/rust-lang/rfcs/issues/961)
for more on this topic.