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

CheckLoopVisitor: also visit closure arguments #50849

Merged
merged 1 commit into from
May 22, 2018

Conversation

est31
Copy link
Member

@est31 est31 commented May 17, 2018

This turns the ICE #50581 in this code:

fn main() {
    |_: [u8; break]| ();
}

from

    'assertion failed: self.tcx.sess.err_count() > 0', librustc_typeck/check/mod.rs

to

    librustc_mir/hair/cx/expr.rs:543: invalid loop id for break: not inside loop scope

which is an ICE as well but at a later stage during compilation and most importantly
fixes of bug #50576 will fix this as well.

As this "only" moves an ICE to a later stage, I didn't add any tests.

Now I have manually verified the default impls of the visitor trait to check whether we have missed any other opportunity to visit more stuff and coudln't find anything (except the missing break visit I've fixed in #50829 but that one was already r+'d so I didn't want to push more commits).

This turns an ICE on this code:

fn main() {
    |_: [u8; break]| ();
}

from
    'assertion failed: self.tcx.sess.err_count() > 0', librustc_typeck/check/mod.rs
to
    librustc_mir/hair/cx/expr.rs:543: invalid loop id for break: not inside loop scope

which is at a later stage during compilation and most importantly
fixes of bug rust-lang#50576 will fix this as well.
@rust-highfive
Copy link
Collaborator

r? @michaelwoerister

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 17, 2018
@vegard
Copy link

vegard commented May 17, 2018

Is that the same as #50581? ;-)

@est31
Copy link
Member Author

est31 commented May 17, 2018

@vegard lol indeed it is. Note that this PR doesn't fix that ICE, it only merges it into #50576: the ICE message will become the same and fixes of the bug will most likely affect all example cases.

@pietroalbini
Copy link
Member

Ping from triage @michaelwoerister! This PR needs your review.

@michaelwoerister
Copy link
Member

Seems like an improvement. Thanks, @est31!

@bors r+

@bors
Copy link
Contributor

bors commented May 21, 2018

📌 Commit 7ad9ef3 has been approved by michaelwoerister

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 21, 2018
kennytm added a commit to kennytm/rust that referenced this pull request May 22, 2018
…woerister

CheckLoopVisitor: also visit closure arguments

This turns the ICE rust-lang#50581 in this code:

```rust
fn main() {
    |_: [u8; break]| ();
}
```

from
```
    'assertion failed: self.tcx.sess.err_count() > 0', librustc_typeck/check/mod.rs
```
to
```
    librustc_mir/hair/cx/expr.rs:543: invalid loop id for break: not inside loop scope
```

which is an ICE as well but at a later stage during compilation and most importantly
fixes of bug rust-lang#50576 will fix this as well.

As this "only" moves an ICE to a later stage, I didn't add any tests.

Now I have manually verified the default impls of the visitor trait to check whether we have missed any other opportunity to visit more stuff and coudln't find anything (except the missing `break` visit I've fixed in rust-lang#50829 but that one was already r+'d so I didn't want to push more commits).
bors added a commit that referenced this pull request May 22, 2018
Rollup of 15 pull requests

Successful merges:

 - #50846 (Add E0665)
 - #50849 (CheckLoopVisitor: also visit closure arguments)
 - #50863 (Make `[T]::len` and `str::len` const fn)
 - #50875 (rustdoc: use "short form" doc(cfg) printing even when combined with other conditionals)
 - #50913 (Fix typo in cell.rs)
 - #50914 (Issue #50636: Improve error diagnostic with missing commas after struct fields.)
 - #50931 (Inline `try_get`.)
 - #50932 (Optimize seen Predicate filtering.)
 - #50945 (Stabilize feature from_ref)
 - #50946 (rustc: Fix procedural macros generating lifetime tokens)
 - #50947 (rustdoc: set tab width in rust source blocks)
 - #50952 (Add the 2018 edition of the book to doc.rust-lang.org)
 - #50958 (Micro-optimization on PR#50697)
 - #50961 (Fix FileCheck finding with MSVC)
 - #50963 (Right-size the `VecDeque` in `coerce_unsized`.)

Failed merges:
@bors bors merged commit 7ad9ef3 into rust-lang:master May 22, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants