From dbb655a1e3a6e0b5525b8ef8109a0546d874f707 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?John=20K=C3=A5re=20Alsaker?= Date: Tue, 13 Jun 2017 18:36:01 +0200 Subject: [PATCH 01/10] Change the for-loop desugar so the `break` does not affect type inference. Fixes #42618 --- src/libcore/iter/mod.rs | 6 ++++-- src/librustc/hir/lowering.rs | 37 ++++++++++++++++++++++++++---------- 2 files changed, 31 insertions(+), 12 deletions(-) diff --git a/src/libcore/iter/mod.rs b/src/libcore/iter/mod.rs index ee81151348772..c91fd16391aaf 100644 --- a/src/libcore/iter/mod.rs +++ b/src/libcore/iter/mod.rs @@ -191,10 +191,12 @@ //! { //! let result = match IntoIterator::into_iter(values) { //! mut iter => loop { -//! let x = match iter.next() { -//! Some(val) => val, +//! let next; +//! match iter.next() { +//! Some(val) => next = val, //! None => break, //! }; +//! let x = next; //! let () = { println!("{}", x); }; //! }, //! }; diff --git a/src/librustc/hir/lowering.rs b/src/librustc/hir/lowering.rs index a6ab67e04693d..1283d136d3287 100644 --- a/src/librustc/hir/lowering.rs +++ b/src/librustc/hir/lowering.rs @@ -2170,11 +2170,13 @@ impl<'a> LoweringContext<'a> { // let result = match ::std::iter::IntoIterator::into_iter() { // mut iter => { // [opt_ident]: loop { - // let = match ::std::iter::Iterator::next(&mut iter) { - // ::std::option::Option::Some(val) => val, + // let next; + // match ::std::iter::Iterator::next(&mut iter) { + // ::std::option::Option::Some(val) => next = val, // ::std::option::Option::None => break // }; - // SemiExpr(); + // let = next; + // StmtExpr(); // } // } // }; @@ -2186,13 +2188,18 @@ impl<'a> LoweringContext<'a> { let iter = self.str_to_ident("iter"); - // `::std::option::Option::Some(val) => val` + let next_ident = self.str_to_ident("next"); + let next_pat = self.pat_ident(e.span, next_ident); + + // `::std::option::Option::Some(val) => next = val` let pat_arm = { let val_ident = self.str_to_ident("val"); let val_pat = self.pat_ident(e.span, val_ident); let val_expr = P(self.expr_ident(e.span, val_ident, val_pat.id)); + let next_expr = P(self.expr_ident(e.span, next_ident, next_pat.id)); + let assign = P(self.expr(e.span, hir::ExprAssign(next_expr, val_expr), ThinVec::new())); let some_pat = self.pat_some(e.span, val_pat); - self.arm(hir_vec![some_pat], val_expr) + self.arm(hir_vec![some_pat], assign) }; // `::std::option::Option::None => break` @@ -2222,10 +2229,20 @@ impl<'a> LoweringContext<'a> { hir::MatchSource::ForLoopDesugar), ThinVec::new())) }; + let match_stmt = respan(e.span, hir::StmtExpr(match_expr, self.next_id())); + + let next_expr = P(self.expr_ident(e.span, next_ident, next_pat.id)); + + // `let next` + let next_let = self.stmt_let_pat(e.span, + None, + next_pat, + hir::LocalSource::ForLoopDesugar); + // `let = next` let pat = self.lower_pat(pat); let pat_let = self.stmt_let_pat(e.span, - match_expr, + Some(next_expr), pat, hir::LocalSource::ForLoopDesugar); @@ -2234,7 +2251,7 @@ impl<'a> LoweringContext<'a> { let body_expr = P(self.expr_block(body_block, ThinVec::new())); let body_stmt = respan(e.span, hir::StmtExpr(body_expr, self.next_id())); - let loop_block = P(self.block_all(e.span, hir_vec![pat_let, body_stmt], None)); + let loop_block = P(self.block_all(e.span, hir_vec![next_let, match_stmt, pat_let, body_stmt], None)); // `[opt_ident]: loop { ... }` let loop_expr = hir::ExprLoop(loop_block, self.lower_opt_sp_ident(opt_ident), @@ -2601,14 +2618,14 @@ impl<'a> LoweringContext<'a> { fn stmt_let_pat(&mut self, sp: Span, - ex: P, + ex: Option>, pat: P, source: hir::LocalSource) -> hir::Stmt { let local = P(hir::Local { pat: pat, ty: None, - init: Some(ex), + init: ex, id: self.next_id(), span: sp, attrs: ThinVec::new(), @@ -2626,7 +2643,7 @@ impl<'a> LoweringContext<'a> { self.pat_ident(sp, ident) }; let pat_id = pat.id; - (self.stmt_let_pat(sp, ex, pat, hir::LocalSource::Normal), pat_id) + (self.stmt_let_pat(sp, Some(ex), pat, hir::LocalSource::Normal), pat_id) } fn block_expr(&mut self, expr: P) -> hir::Block { From 2d379b33936d6cd9a9b643cf5ddc36450176553c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?John=20K=C3=A5re=20Alsaker?= Date: Wed, 14 Jun 2017 13:36:30 +0200 Subject: [PATCH 02/10] Fix formatting and add a test for destruction order of unbound values --- src/librustc/hir/lowering.rs | 15 +++++++--- .../for-loop-lifetime-of-unbound-values.rs | 28 +++++++++++++++++++ 2 files changed, 39 insertions(+), 4 deletions(-) create mode 100644 src/test/run-pass/for-loop-lifetime-of-unbound-values.rs diff --git a/src/librustc/hir/lowering.rs b/src/librustc/hir/lowering.rs index 1283d136d3287..f2a434979535d 100644 --- a/src/librustc/hir/lowering.rs +++ b/src/librustc/hir/lowering.rs @@ -2190,14 +2190,16 @@ impl<'a> LoweringContext<'a> { let next_ident = self.str_to_ident("next"); let next_pat = self.pat_ident(e.span, next_ident); - + // `::std::option::Option::Some(val) => next = val` let pat_arm = { let val_ident = self.str_to_ident("val"); let val_pat = self.pat_ident(e.span, val_ident); let val_expr = P(self.expr_ident(e.span, val_ident, val_pat.id)); let next_expr = P(self.expr_ident(e.span, next_ident, next_pat.id)); - let assign = P(self.expr(e.span, hir::ExprAssign(next_expr, val_expr), ThinVec::new())); + let assign = P(self.expr(e.span, + hir::ExprAssign(next_expr, val_expr), + ThinVec::new())); let some_pat = self.pat_some(e.span, val_pat); self.arm(hir_vec![some_pat], assign) }; @@ -2232,7 +2234,7 @@ impl<'a> LoweringContext<'a> { let match_stmt = respan(e.span, hir::StmtExpr(match_expr, self.next_id())); let next_expr = P(self.expr_ident(e.span, next_ident, next_pat.id)); - + // `let next` let next_let = self.stmt_let_pat(e.span, None, @@ -2251,7 +2253,12 @@ impl<'a> LoweringContext<'a> { let body_expr = P(self.expr_block(body_block, ThinVec::new())); let body_stmt = respan(e.span, hir::StmtExpr(body_expr, self.next_id())); - let loop_block = P(self.block_all(e.span, hir_vec![next_let, match_stmt, pat_let, body_stmt], None)); + let loop_block = P(self.block_all(e.span, + hir_vec![next_let, + match_stmt, + pat_let, + body_stmt], + None)); // `[opt_ident]: loop { ... }` let loop_expr = hir::ExprLoop(loop_block, self.lower_opt_sp_ident(opt_ident), diff --git a/src/test/run-pass/for-loop-lifetime-of-unbound-values.rs b/src/test/run-pass/for-loop-lifetime-of-unbound-values.rs new file mode 100644 index 0000000000000..a273fb579fa0b --- /dev/null +++ b/src/test/run-pass/for-loop-lifetime-of-unbound-values.rs @@ -0,0 +1,28 @@ +use std::cell::Cell; + +struct Flag<'a>(&'a Cell); + +impl<'a> Drop for Flag<'a> { + fn drop(&mut self) { + self.0.set(false) + } +} + +fn main() { + let alive2 = Cell::new(true); + for _i in std::iter::once(Flag(&alive2)) { + // The Flag value should be alive in the for loop body + assert_eq!(alive2.get(), true); + } + // The Flag value should be dead outside of the loop + assert_eq!(alive2.get(), false); + + let alive = Cell::new(true); + for _ in std::iter::once(Flag(&alive)) { + // The Flag value should be alive in the for loop body even if it wasn't + // bound by the for loop + assert_eq!(alive.get(), true); + } + // The Flag value should be dead outside of the loop + assert_eq!(alive.get(), false); +} \ No newline at end of file From 8d65dd62b17892102231101a7e58de0a20b5dd21 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?John=20K=C3=A5re=20Alsaker?= Date: Wed, 14 Jun 2017 19:26:42 +0200 Subject: [PATCH 03/10] Fix test formatting --- .../for-loop-lifetime-of-unbound-values.rs | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/src/test/run-pass/for-loop-lifetime-of-unbound-values.rs b/src/test/run-pass/for-loop-lifetime-of-unbound-values.rs index a273fb579fa0b..a0562edfadd6c 100644 --- a/src/test/run-pass/for-loop-lifetime-of-unbound-values.rs +++ b/src/test/run-pass/for-loop-lifetime-of-unbound-values.rs @@ -1,11 +1,21 @@ +// Copyright 2017 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + use std::cell::Cell; struct Flag<'a>(&'a Cell); impl<'a> Drop for Flag<'a> { - fn drop(&mut self) { - self.0.set(false) - } + fn drop(&mut self) { + self.0.set(false) + } } fn main() { @@ -16,7 +26,7 @@ fn main() { } // The Flag value should be dead outside of the loop assert_eq!(alive2.get(), false); - + let alive = Cell::new(true); for _ in std::iter::once(Flag(&alive)) { // The Flag value should be alive in the for loop body even if it wasn't From a80840f75196b69ea3f6c8b1fdaa032be609e9f5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?John=20K=C3=A5re=20Alsaker?= Date: Thu, 15 Jun 2017 02:09:53 +0200 Subject: [PATCH 04/10] Added more tests --- .../for-loop-unconstrained-element-type.rs | 13 +++++++++++++ .../for-loop-lifetime-of-unbound-values.rs | 2 +- ...op-unconstrained-element-type-i32-fallback.rs | 16 ++++++++++++++++ 3 files changed, 30 insertions(+), 1 deletion(-) create mode 100644 src/test/compile-fail/for-loop-unconstrained-element-type.rs create mode 100644 src/test/run-pass/for-loop-unconstrained-element-type-i32-fallback.rs diff --git a/src/test/compile-fail/for-loop-unconstrained-element-type.rs b/src/test/compile-fail/for-loop-unconstrained-element-type.rs new file mode 100644 index 0000000000000..f7dccc7f2ac19 --- /dev/null +++ b/src/test/compile-fail/for-loop-unconstrained-element-type.rs @@ -0,0 +1,13 @@ +// Copyright 2017 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +fn main() { + for i in Vec::new() { } //~ ERROR type annotations needed +} diff --git a/src/test/run-pass/for-loop-lifetime-of-unbound-values.rs b/src/test/run-pass/for-loop-lifetime-of-unbound-values.rs index a0562edfadd6c..4653a493898ee 100644 --- a/src/test/run-pass/for-loop-lifetime-of-unbound-values.rs +++ b/src/test/run-pass/for-loop-lifetime-of-unbound-values.rs @@ -35,4 +35,4 @@ fn main() { } // The Flag value should be dead outside of the loop assert_eq!(alive.get(), false); -} \ No newline at end of file +} diff --git a/src/test/run-pass/for-loop-unconstrained-element-type-i32-fallback.rs b/src/test/run-pass/for-loop-unconstrained-element-type-i32-fallback.rs new file mode 100644 index 0000000000000..d9a876d9c9521 --- /dev/null +++ b/src/test/run-pass/for-loop-unconstrained-element-type-i32-fallback.rs @@ -0,0 +1,16 @@ +// Copyright 2017 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +fn main() { + let mut sum = 0; + for i in Vec::new() { + sum += i; + } +} From d5fd8fef67a0a0b4d30dd8b23052c380632ae0e2 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Thu, 15 Jun 2017 12:27:15 -0400 Subject: [PATCH 05/10] explain purpose of test --- .../compile-fail/for-loop-unconstrained-element-type.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/test/compile-fail/for-loop-unconstrained-element-type.rs b/src/test/compile-fail/for-loop-unconstrained-element-type.rs index f7dccc7f2ac19..dd09e4a79ecdc 100644 --- a/src/test/compile-fail/for-loop-unconstrained-element-type.rs +++ b/src/test/compile-fail/for-loop-unconstrained-element-type.rs @@ -8,6 +8,12 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. +// Test that `for` loops don't introduce artificial +// constraints on the type of the binding (`i`). +// Subtle changes in the desugaring can cause the +// type of elements in the vector to (incorrectly) +// fallback to `!` or `()`. + fn main() { for i in Vec::new() { } //~ ERROR type annotations needed } From e4baa26d2a24845bb960a9a51fa3c4ecf63bdd4a Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Thu, 15 Jun 2017 12:28:07 -0400 Subject: [PATCH 06/10] document purpose of test --- src/test/run-pass/for-loop-lifetime-of-unbound-values.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/test/run-pass/for-loop-lifetime-of-unbound-values.rs b/src/test/run-pass/for-loop-lifetime-of-unbound-values.rs index 4653a493898ee..7a088b5133472 100644 --- a/src/test/run-pass/for-loop-lifetime-of-unbound-values.rs +++ b/src/test/run-pass/for-loop-lifetime-of-unbound-values.rs @@ -8,6 +8,11 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. +// Test when destructors run in a for loop. The intention is +// that the value for each iteration is dropped *after* the loop +// body has executed. This is true even when the value is assigned +// to a `_` pattern (and hence ignored). + use std::cell::Cell; struct Flag<'a>(&'a Cell); From f11cf60944eb52eaebae8f4b510a0e805c679958 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Thu, 15 Jun 2017 12:31:45 -0400 Subject: [PATCH 07/10] Create for-loop-unconstrained-element-type-i32-fallback.rs --- .../for-loop-unconstrained-element-type-i32-fallback.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/test/run-pass/for-loop-unconstrained-element-type-i32-fallback.rs b/src/test/run-pass/for-loop-unconstrained-element-type-i32-fallback.rs index d9a876d9c9521..b36afcf87b3ee 100644 --- a/src/test/run-pass/for-loop-unconstrained-element-type-i32-fallback.rs +++ b/src/test/run-pass/for-loop-unconstrained-element-type-i32-fallback.rs @@ -8,6 +8,10 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. +// Test that the type of `sum` falls back to `i32` here, +// and that the for loop desugaring doesn't inferfere with +// that. + fn main() { let mut sum = 0; for i in Vec::new() { From 09bc09201ca02c9bdbcba1c72610209c3598bca1 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Thu, 15 Jun 2017 15:45:37 -0400 Subject: [PATCH 08/10] remove trailing whitespace --- src/test/compile-fail/for-loop-unconstrained-element-type.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/compile-fail/for-loop-unconstrained-element-type.rs b/src/test/compile-fail/for-loop-unconstrained-element-type.rs index dd09e4a79ecdc..fb5553166bafb 100644 --- a/src/test/compile-fail/for-loop-unconstrained-element-type.rs +++ b/src/test/compile-fail/for-loop-unconstrained-element-type.rs @@ -11,7 +11,7 @@ // Test that `for` loops don't introduce artificial // constraints on the type of the binding (`i`). // Subtle changes in the desugaring can cause the -// type of elements in the vector to (incorrectly) +// type of elements in the vector to (incorrectly) // fallback to `!` or `()`. fn main() { From bd7cc779b65e658ff5b6bbd9d2bdd8ed07ea38f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?John=20K=C3=A5re=20Alsaker?= Date: Sat, 17 Jun 2017 01:51:55 +0200 Subject: [PATCH 09/10] Make the `next` variable mutable to allow for ref mut in for patterns. --- src/librustc/hir/lowering.rs | 14 +++++++------- src/test/run-pass/for-loop-mut-ref-element.rs | 15 +++++++++++++++ 2 files changed, 22 insertions(+), 7 deletions(-) create mode 100644 src/test/run-pass/for-loop-mut-ref-element.rs diff --git a/src/librustc/hir/lowering.rs b/src/librustc/hir/lowering.rs index f2a434979535d..4b8ee8d8aecfb 100644 --- a/src/librustc/hir/lowering.rs +++ b/src/librustc/hir/lowering.rs @@ -2170,12 +2170,12 @@ impl<'a> LoweringContext<'a> { // let result = match ::std::iter::IntoIterator::into_iter() { // mut iter => { // [opt_ident]: loop { - // let next; + // let mut _next; // match ::std::iter::Iterator::next(&mut iter) { - // ::std::option::Option::Some(val) => next = val, + // ::std::option::Option::Some(val) => _next = val, // ::std::option::Option::None => break // }; - // let = next; + // let = _next; // StmtExpr(); // } // } @@ -2188,8 +2188,8 @@ impl<'a> LoweringContext<'a> { let iter = self.str_to_ident("iter"); - let next_ident = self.str_to_ident("next"); - let next_pat = self.pat_ident(e.span, next_ident); + let next_ident = self.str_to_ident("_next"); + let next_pat = self.pat_ident_binding_mode(e.span, next_ident, hir::BindByValue(hir::MutMutable)); // `::std::option::Option::Some(val) => next = val` let pat_arm = { @@ -2235,13 +2235,13 @@ impl<'a> LoweringContext<'a> { let next_expr = P(self.expr_ident(e.span, next_ident, next_pat.id)); - // `let next` + // `let mut _next` let next_let = self.stmt_let_pat(e.span, None, next_pat, hir::LocalSource::ForLoopDesugar); - // `let = next` + // `let = _next` let pat = self.lower_pat(pat); let pat_let = self.stmt_let_pat(e.span, Some(next_expr), diff --git a/src/test/run-pass/for-loop-mut-ref-element.rs b/src/test/run-pass/for-loop-mut-ref-element.rs new file mode 100644 index 0000000000000..14ce23b07242c --- /dev/null +++ b/src/test/run-pass/for-loop-mut-ref-element.rs @@ -0,0 +1,15 @@ +// Copyright 2014 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// Tests that for loops can bind elements as mutable references + +fn main() { + for ref mut _a in std::iter::once(true) {} +} \ No newline at end of file From 1409e707a2e6b152ab66a9d836e5e367f6f066cc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?John=20K=C3=A5re=20Alsaker?= Date: Sat, 17 Jun 2017 14:46:37 +0200 Subject: [PATCH 10/10] Fix formatting --- src/librustc/hir/lowering.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/librustc/hir/lowering.rs b/src/librustc/hir/lowering.rs index 4b8ee8d8aecfb..9715db42474da 100644 --- a/src/librustc/hir/lowering.rs +++ b/src/librustc/hir/lowering.rs @@ -2189,7 +2189,9 @@ impl<'a> LoweringContext<'a> { let iter = self.str_to_ident("iter"); let next_ident = self.str_to_ident("_next"); - let next_pat = self.pat_ident_binding_mode(e.span, next_ident, hir::BindByValue(hir::MutMutable)); + let next_pat = self.pat_ident_binding_mode(e.span, + next_ident, + hir::BindByValue(hir::MutMutable)); // `::std::option::Option::Some(val) => next = val` let pat_arm = {