Change the for-loop desugar so the break does not affect type inference. Fixes #42618
#42634
Conversation
|
r? @arielb1 (rust_highfive has picked a reviewer for you, use r? to override) |
eddyb
added
the
beta-nominated
|
This looks like a rather fragile area of code. I'm ok with the changes, but I'll rather r? @nikomatsakis |
arielb1
added
the
S-waiting-on-review
|
@Zoxc can we add a run-pass test checking the order of drops with comments explaining the expected order? |
|
We probably would need this to be in relnotes, presuming it lands. Tagging it as such. |
nikomatsakis
added
the
relnotes
|
@Zoxc can you also add a fn main() {
let mut sum = 0;
for i in Vec::new() {
sum += i;
}
}Similarly a fn main() {
for i in Vec::new() { } //~ ERROR type annotations needed
}r=me with those tests |
nikomatsakis
removed
the
relnotes
|
Removing relnotes, I didn't realize that this behavior had never contaminated stable. |
|
#42265 needs to be reverted from beta as this is applied |
|
@bors r+ |
|
📌 Commit e4baa26 has been approved by |
|
I took the liberty of adding a few comments to the test for posterity. |
|
@bors r- oops, forgot one test. |
|
@bors r+ |
|
📌 Commit f11cf60 has been approved by |
|
@bors r+ |
|
tidy error: |
| @@ -2170,11 +2170,13 @@ impl<'a> LoweringContext<'a> { | |||
| // let result = match ::std::iter::IntoIterator::into_iter(<head>) { | |||
| // mut iter => { | |||
| // [opt_ident]: loop { | |||
| // let <pat> = match ::std::iter::Iterator::next(&mut iter) { | |||
| // ::std::option::Option::Some(val) => val, | |||
| // let mut _next; | |||
There was a problem hiding this comment.
Does the _next name disable a "unused mut" warning?
|
@bors r+ |
|
📌 Commit 1409e70 has been approved by |
Change the for-loop desugar so the `break` does not affect type inference. Fixes #42618 Rewrite the `for` loop desugaring to avoid contaminating the inference results. Under the older desugaring, `for x in vec![] { .. }` would erroneously type-check, even though the type of `vec![]` is unconstrained. (written by @nikomatsakis)
|
☀️ Test successful - status-appveyor, status-travis |
brson
added
the
beta-accepted
alexcrichton
removed
the
beta-nominated
[beta] backports - #42785 - #42740 - #42735 - #42728 - #42695 - #42659 - #42634 - #42566 I just unilaterally accepted all the non-accepted nominations. Everything picked cleanly. Still testing locally. cc @rust-lang/compiler r? @alexcrichton
[beta] backports - #42785 - #42740 - #42735 - #42728 - #42695 - #42659 - #42634 - #42566 I just unilaterally accepted all the non-accepted nominations. Everything picked cleanly. Still testing locally. cc @rust-lang/compiler r? @alexcrichton
…ielb1 change binding name of for loop lowering to appease clippy With the latest change to for loop lowering (rust-lang#42634), a `_next` binding was introduced. Unfortunately, this [disturbs](https://github.com/Manishearth/rust-clippy/issues/1846) clippy's `used_underscore_binding` lint. This commit just renames the binding to `__next` so clippy will be happy. It should have no other effect.
Rewrite the
forloop desugaring to avoid contaminating the inference results. Under the older desugaring,for x in vec![] { .. }would erroneously type-check, even though the type ofvec![]is unconstrained. (written by @nikomatsakis)