Rust: Tracking issue for RFC 1624: `break` with values for `loop`

Created on 22 Oct 2016  Â·  44Comments  Â·  Source: rust-lang/rust

RFC.

Current status:

B-RFC-approved B-unstable E-easy E-mentor T-lang final-comment-period

Most helpful comment

A future RFC could extend this to labelled blocks without loops.

    let result : u64 = 'block: {
        if foo() { break 'block 1 }
        if bar() { break 'block 2 }
        3
    };

This would also give you a way to handle for and while

    let result : u64 = 'block: {
        for e in container.iter() {
            let v = foo(e);
            if v > 0 { break 'block v }
        }
        0
    };

The semantics would be exactly the same as

    let result : u64 = 'block: loop { break {
        for e in container.iter() {
            let v = foo(e);
            if v > 0 { break 'block v }
        }
        0
    }};

EDIT: I made this into a Pre-RFC.

All 44 comments

Currently awaiting implementation.

cc @dhardy

@aturon How would we go about advertising mentorship? If someone wants to implement this and they're not already familiar with the compiler internals, they can come to #rustc on IRC to discuss it.
This is the case for many such features with a relatively small footprint, we're glad to help.

(Although I may be spreading myself thin a bit too much instead of finishing my own refactors.)

@eddyb An important question we don't have a systematic answer for right now (cc @brson). We should probably move this to an internals thread or something, though.

I hear you @aturon but don't have the time just now (and maybe for a few weeks).

Out of curiosity is there anything holding this back from being stabilized?

@glaebhoerl Not that I'm aware of. More obscure features like this are hard to judge, since we naturally tend to get very little feedback on them.

In any case, it's certainly been around long enough that going to FCP to try to advertise for more feedback seems good:

@rfcbot fcp merge

Team member @aturon has proposed to merge this. The next step is review by the rest of the tagged teams:

  • [x] @aturon
  • [x] @eddyb
  • [x] @nikomatsakis
  • [x] @nrc
  • [x] @pnkfelix
  • [x] @withoutboats

No concerns currently listed.

Once these reviewers reach consensus, this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@rfcbot reviewed

I guess I don't have any technical reason against stabilising this (I was in favour of the RFC and nothing has changed my mind). But I kind of think that if a feature isn't being used at all then we should consider removing it rather than stabilising it. Is it worth posting in users.r-l.o to see if people are using it and whether there are any problems?

It’s not surprising that a feature isn’t used when it’s not available yet. Much of the ecosystem has moved to the stable channel.

@nrc I played with it, saw that it worked, and then promptly stopped using it because while it's nice (really!) it's not worth jumping off stable for some indeterminate period. So if my experience is a barometer you may not get that much feedback but maybe that's to be expected for this feature.

FWIW, I'm using it, e.g. for things like:

            let (filename, mut file) = loop {
                let uuid = Uuid::new_v4().simple().to_string();
                let filename = format!("{}.{}", uuid, ImageFormat::from_image_format(fmt).as_str());
                let fpath = ::helper::filename_to_local(&filename);
                if let Ok(f) = fs::OpenOptions::new().write(true).create_new(true).open(&fpath) {
                    break (filename, f);
                }
            };
            img.save(&mut file, fmt)?;

I also wouldn't depend on a nightly feature for this, but would use it if it were available. As it stands I'd tend to just use a temporary variable, but I'd rather not if this were stable.

(Likewise, I originally popped in here to ask about stabilization because I was going to suggest it to someone on twitter who was lamenting the necessity of the temporary variables, before I realized that it's only available on nightly.)

It should take more than just "apparent lack of usage" to remove a feature for which the RFC was accepted, with the motivation clearly understood.

People won't go out of their way to change their existing code just to use this feature, but it offers a better alternative for some cases, doesn't introduce any significant complexity, and makes the syntax more consistent with the expression oriented nature of Rust.

While I'm personally wildly in favor of this, it is important that features be evaluated before stabilization. There was a thread before on this connundrum, but I don't recall it concluding with a solution either :/.

To me the "experimental features are unavailable in stable & beta" decision seems wrong. There are a couple of advantages to using the stable branch and it would be nice to be able to use stable while also trying out probably-to-be-stabilised features with the only caveat being that these features might be removed or significantly altered in a future compiler version.

Being forced to use nightly is less attractive: more likely to find other bugs and possibly having to recommend usage of arbitrary nightly releases with your project instead of just the latest stable release.

None of which relates directly to this RFC.

it would be nice to be able to use stable while also trying out probably-to-be-stabilised features

While this would be nice for users in the short-term, there’s a very high risk that a popular "unstable" feature becomes used enough to put pressure on the Rust project not to change it, making it de-facto stable.

Look what happened with vendor prefixes in CSS: stuff got widespread adoption before it was ready (especially on mobile), and now each browser implement multiple versions of WebKit legacy gradient syntax (among other legacy stuff) to be more compatible with all these sites.

It does seem like "get wider experience with features before stabilizing them" versus "avoid unstable features being widely adopted to retain the freedom of changing them" is a zero-sum game.

Having unstable features depend on nightly introduces a stability guarantee for the rust ecosystem. If it compiles on stable you get some stability guarantee that you wouldn't get otherwise. One of the reasons that rust has such a good backwards compatibility is exactly due to this. IMO it's one of the most clever choices the rust team has made. ;-)

There's an inherent contradiction between stability and progress, same as there is between security and convenience.

I wasn't proposing that experimental features be usable by accident but that they be usable via a command-line switch or something, though maybe that's insignificant.

@SimonSapin I hope that if we're really clear up front, we can avoid that. The fact that the Rust community is smaller and more centralized than the web helps.

Also, vendor prefixes are awkward because stabilization is necessarily a breaking change, whereas unused/unknown features are ignored.

@Ericson2314 Today if I write a library for other people, there is an incentive to avoid unstable features: so that people who use the stable channel can use it. So opting into unstable has immediate and concrete consequences. If however #![feature(…)] could be used on the stable channel, I could use it in my library with no immediate consequence. The only reason to avoid it would be for the theoretical long-term health of the Rust project. And when the unstable feature does change (break) eventually it’s very easy to deflect the blame onto whoever broke it, claim it worked fine before, etc.

I think it’s well known how humans tend to behave when it comes to balancing immediate gratification against vague long-term “health” concerns.

@SimonSapin I think one solution is to force downstream to use the same feature, whether or not it affects upstream's interface. Then everyone needs to opt in---the pain and guilty is immediately broadcasted. That plus angry warnings I think is enough immediate consequence to make humanity exceed expectations :).

Oh, and if we actually change long-lived, slow-evolving features like const fn, the threat is henceforth credible :D.

This is a good conversation but this is a very poor venue for it. Someone should start (or possibly bump) an internals thread

If however #![feature(…)] could be used on the stable channel, I could use it in my library with no immediate consequence. The only reason to avoid it would be for the theoretical long-term health of the Rust project.

Yep, this is part of the big problem with allowing unstable features on stable. Keep in mind this scenario:

  1. An author of a small library opts into an unstable feature that makes their life easier and more convenient.
  2. This library is depended on by a large number of other libraries.
  3. Those libraries are depended on by further libraries, and so on and so on.
  4. Applications transitively depend on the original library, and the application authors aren't even aware of it.
  5. A Rust team decides not to stabilize the feature and removes support, breaking every transitive reverse dependency of that original small library. Unaware application authors are affected, to their surprise.

I think one solution is to force downstream to use the same feature, whether or not it affects upstream's interface.

@Ericson2314 You beat me to it here. I think this is actually a realistic solution to the above scenario, unlike the typical ask to "just allow features on stable".

That said, I really like how things have been working so far. I don't see a strong need to change things, and this discussion has been had many times before.

And @withoutboats is right, but I already typed out this whole reply. :P

Now, getting back on track. It seems this disagreement was provoked by @nrc's comment here:

But I kind of think that if a feature isn't being used at all then we should consider removing it rather than stabilising it.

I usually agree with this but I disagree with applying it to this feature. See @aturon's earlier comment:

More obscure features like this are hard to judge, since we naturally tend to get very little feedback on them.

I think it's okay to make a judgement call for a small convenience feature like this that is "nice to have" but isn't going overcome the activation energy for someone to enable the feature (let alone switch to nightly).

It may also be useful to spread the message more and ask for opinions, but we did already accept the RFC and I don't think we expected to get a ton of feedback to change the decision later.

A couple further thoughts.

  1. I'm not sure that there's no usage of this feature. But as many have pointed out, it's not surprising that people aren't going to jump to nightly just for this.

  2. Remember that going to FCP does not necessarily mean stabilizing the feature. In particular, going to FCP highlights the feature significantly, which gives us a chance at gathering more feedback than we could otherwise.

  3. I think there are definitely some steps we could take to improve this part of the feature pipeline. As @withoutboats said, that should probably go on an internals thread. I'll try to do that soon.

I started an internals thread on the topic of gathering feedback on unstable features. Please join the thread!

Approved for FCP - it seems from comments here that people are using this, so that satisfies my desire for some usage, and this reminder from @aturon is useful too:

Remember that going to FCP does not necessarily mean stabilizing the feature ...

it seems from comments here that people are using this

Am definitely using it.

:bell: This is now entering its final comment period, as per the review above. :bell:

The final comment period is now complete.

So, we've decided to stabilize this feature. That means we would like some PRs. I'm going to mark this bug as E-easy and E-mentor. The PRs we need are a documentation PR (first) and the stabilization PR (second). I've added some links to instructions into the head comment.

Pull requests for the documentation sent to the various repositories. Should I wait for the stabilization PR?

I noticed a small inconsistency between documentation and implementation: the RFC claims

break and break 'label become equivalent to break () and break 'label () respectively

but the impl doesn't support break () in while or for loops.

Perhaps this is for the best anyway until "break with value" is supported by for and while.

It might make sense to allow break to take a value of type () in while and for loops too, for consistency.

A future RFC could extend this to labelled blocks without loops.

    let result : u64 = 'block: {
        if foo() { break 'block 1 }
        if bar() { break 'block 2 }
        3
    };

This would also give you a way to handle for and while

    let result : u64 = 'block: {
        for e in container.iter() {
            let v = foo(e);
            if v > 0 { break 'block v }
        }
        0
    };

The semantics would be exactly the same as

    let result : u64 = 'block: loop { break {
        for e in container.iter() {
            let v = foo(e);
            if v > 0 { break 'block v }
        }
        0
    }};

EDIT: I made this into a Pre-RFC.

We specifically decided to limit this to loop loops. In fact, there is a (closed) rfc issue on this topic (https://github.com/rust-lang/rfcs/issues/1767).

@nikomatsakis The proposal here is different: it's talking about jumping out to a labeled block, rather than simply breaking out of a loop. It just happens to combine reasonably well with loops.

Hmm, one extra level of indentation, but way less controversy! That's a pretty neat trick @ciphergoth.

Can this issue be closed? The stabilisation PR #42016 has been merged since.

I think so, closing.

Was this page helpful?
0 / 5 - 0 ratings