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

Preliminary support for labeled break/continue for loops #3807

Closed
wants to merge 1 commit into from

Conversation

catamorphism
Copy link
Contributor

This patch adds preliminary middle-end support (liveness and trans)
for breaks and loops to loop constructs that have labels.

while and for loops can't have labels yet.

Progress on #2216

r? @pcwalton r+ from @nikomatsakis so no need for other review.

This patch adds preliminary middle-end support (liveness and trans)
for breaks and `loop`s to `loop` constructs that have labels.

while and for loops can't have labels yet.

Progress on rust-lang#2216
@nikomatsakis
Copy link
Contributor

@catamorphism @pcwalton hope you don't mind, I was scanning the pull request queue and saw that this touched on liveness, so I wanted to take a look. Overall it looks pretty reasonable (modulo some nits regarding indentation).

I did have one question: Can you add a test that breaks out of a for loop, like this?

fn main() {
    let v = ~[1, 2, 3];
    loop foo: {
        for v.each |e| { break foo; }
        io::println("Hello, world!\n");
        break;
    }
    io::println("Goodbye, world!\n");
}

Based on my testing, this test will not execute correctly (that is, "Hello, World!" and "Goodye, World!" will both print). I think it's perfectly fine if we do not support using named breaks out of for loops (at least for the moment), but we should either report an error or Do The Right Thing. Of course I could be misreading the code, but I believe that resolve accepts this (at least in my local testing) and I also believe that if it gets all the way to trans, it will simply ignore the label and break out of the for loop. Actually though what I think will happen is that it will ICE in liveness, because the hashmap mapping ids to nodes will not contain anything for foo inside the closure.

@catamorphism
Copy link
Contributor Author

I merged this.

@catamorphism
Copy link
Contributor Author

(Oh, and in response to @nikomatsakis 's last comment, I'm working on this -- it's almost done.)

tesuji pushed a commit to tesuji/rustc that referenced this pull request Jun 4, 2020
len_zero: skip ranges if feature `range_is_empty` is not enabled

If the feature is not enabled, calling `is_empty()` on a range is ambiguous. Moreover, the two possible resolutions are unstable methods, one inherent to the range and the other being part of the `ExactSizeIterator` trait.

Since `len_zero` only checks for existing `is_empty()` inherent methods, we only take into account the `range_is_empty` feature.

Related: rust-lang#48111 (comment)

changelog: len_zero: avoid linting ranges without #![feature(range_is_empty)]

Fixes: rust-lang#3807
RalfJung pushed a commit to RalfJung/rust that referenced this pull request Aug 18, 2024
add 'project' process guidlines for larger contributions

Fixes rust-lang/miri#3443

I am honestly not entirely sure what the consensus from what issue was. I feel like the epoll PR worked reasonably well, and not having been closely involved I am not sure which process `@oli-obk` followed there. Compared to the first draft in rust-lang#3443 I tried to make this less formal and framed more as guidelines than hard rules.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants