Rollup merge of rust-lang#51731 - varkor:closure-array-break-length, …

…r=estebank Fix ICEs when using continue as an array length inside closures (inside loop conditions) Fixes rust-lang#51707. Fixes rust-lang#51708. r? @estebank
pietroalbini · Jun 26, 2018 · 7262824 · 7262824
2 parents b71f6df + c3d6ee9
commit 7262824
Show file tree

Hide file tree

Showing 9 changed files with 87 additions and 31 deletions.
diff --git a/src/librustc/hir/lowering.rs b/src/librustc/hir/lowering.rs
@@ -3536,12 +3536,22 @@ impl<'a> LoweringContext<'a> {
                         this.expr_block(block, ThinVec::new())
                     })
                 })
-            },
+            }
             ExprKind::Closure(
-                capture_clause, asyncness, movability, ref decl, ref body, fn_decl_span) =>
-            {
-                self.with_new_scopes(|this| {
-                    if let IsAsync::Async(async_closure_node_id) = asyncness {
+                capture_clause, asyncness, movability, ref decl, ref body, fn_decl_span
+            ) => {
+                if let IsAsync::Async(async_closure_node_id) = asyncness {
+                    let outer_decl = FnDecl {
+                        inputs: decl.inputs.clone(),
+                        output: FunctionRetTy::Default(fn_decl_span),
+                        variadic: false,
+                    };
+                    // We need to lower the declaration outside the new scope, because we
+                    // have to conserve the state of being inside a loop condition for the
+                    // closure argument types.
+                    let fn_decl = self.lower_fn_decl(&outer_decl, None, false, false);
+
+                    self.with_new_scopes(|this| {
                         // FIXME(cramertj) allow `async` non-`move` closures with
                         if capture_clause == CaptureBy::Ref &&
                             !decl.inputs.is_empty()
@@ -3561,11 +3571,6 @@ impl<'a> LoweringContext<'a> {
 
                         // Transform `async |x: u8| -> X { ... }` into
                         // `|x: u8| future_from_generator(|| -> X { ... })`
-                        let outer_decl = FnDecl {
-                            inputs: decl.inputs.clone(),
-                            output: FunctionRetTy::Default(fn_decl_span),
-                            variadic: false,
-                        };
                         let body_id = this.lower_body(Some(&outer_decl), |this| {
                             let async_ret_ty = if let FunctionRetTy::Ty(ty) = &decl.output {
                                 Some(&**ty)
@@ -3579,12 +3584,17 @@ impl<'a> LoweringContext<'a> {
                         });
                         hir::ExprClosure(
                             this.lower_capture_clause(capture_clause),
-                            this.lower_fn_decl(&outer_decl, None, false, false),
+                            fn_decl,
                             body_id,
                             fn_decl_span,
                             None,
                         )
-                    } else {
+                    })
+                } else {
+                    // Lower outside new scope to preserve `is_in_loop_condition`.
+                    let fn_decl = self.lower_fn_decl(decl, None, false, false);
+
+                    self.with_new_scopes(|this| {
                         let mut is_generator = false;
                         let body_id = this.lower_body(Some(decl), |this| {
                             let e = this.lower_expr(body);
@@ -3618,13 +3628,13 @@ impl<'a> LoweringContext<'a> {
                         };
                         hir::ExprClosure(
                             this.lower_capture_clause(capture_clause),
-                            this.lower_fn_decl(decl, None, false, false),
+                            fn_decl,
                             body_id,
                             fn_decl_span,
                             generator_option,
                         )
-                    }
-                })
+                    })
+                }
             }
             ExprKind::Block(ref blk, opt_label) => {
                 hir::ExprBlock(self.lower_block(blk,

diff --git a/src/librustc_mir/build/scope.rs b/src/librustc_mir/build/scope.rs
@@ -540,12 +540,12 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
     // ==============
     /// Finds the breakable scope for a given label. This is used for
     /// resolving `break` and `continue`.
-    pub fn find_breakable_scope(&mut self,
+    pub fn find_breakable_scope(&self,
                            span: Span,
                            label: region::Scope)
-                           -> &mut BreakableScope<'tcx> {
+                           -> &BreakableScope<'tcx> {
         // find the loop-scope with the correct id
-        self.breakable_scopes.iter_mut()
+        self.breakable_scopes.iter()
             .rev()
             .filter(|breakable_scope| breakable_scope.region_scope == label)
             .next()

diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs
@@ -3846,7 +3846,14 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
                 }
 
             }
-            hir::ExprContinue(_) => { tcx.types.never }
+            hir::ExprContinue(destination) => {
+                if let Ok(_) = destination.target_id {
+                    tcx.types.never
+                } else {
+                    // There was an error, make typecheck fail
+                    tcx.types.err
+                }
+            }
             hir::ExprRet(ref expr_opt) => {
                 if self.ret_coercion.is_none() {
                     struct_span_err!(self.tcx.sess, expr.span, E0572,

diff --git a/src/test/mir-opt/end_region_6.rs b/src/test/mir-opt/end_region_6.rs
@@ -68,17 +68,17 @@ fn foo<F>(f: F) where F: FnOnce() -> i32 {
 // fn main::{{closure}}(_1: [closure@NodeId(22) d:&'19s D]) -> i32 {
 //     let mut _0: i32;
 //     ...
-//     let _2: &'15_0rs D;
+//     let _2: &'16_0rs D;
 //     ...
 //     let mut _3: i32;
 //     bb0: {
 //         StorageLive(_2);
-//         _2 = &'15_0rs (*(_1.0: &'19s D));
+//         _2 = &'16_0rs (*(_1.0: &'19s D));
 //         StorageLive(_3);
 //         _3 = ((*_2).0: i32);
 //         _0 = move _3;
 //         StorageDead(_3);
-//         EndRegion('15_0rs);
+//         EndRegion('16_0rs);
 //         StorageDead(_2);
 //         return;
 //     }

diff --git a/src/test/mir-opt/end_region_7.rs b/src/test/mir-opt/end_region_7.rs
@@ -76,17 +76,17 @@ fn foo<F>(f: F) where F: FnOnce() -> i32 {
 // fn main::{{closure}}(_1: [closure@NodeId(22) d:D]) -> i32 {
 //     let mut _0: i32;
 //     ...
-//     let _2: &'15_0rs D;
+//     let _2: &'16_0rs D;
 //     ...
 //     let mut _3: i32;
 //     bb0: {
 //         StorageLive(_2);
-//         _2 = &'15_0rs (_1.0: D);
+//         _2 = &'16_0rs (_1.0: D);
 //         StorageLive(_3);
 //         _3 = ((*_2).0: i32);
 //         _0 = move _3;
 //         StorageDead(_3);
-//         EndRegion('15_0rs);
+//         EndRegion('16_0rs);
 //         StorageDead(_2);
 //         drop(_1) -> [return: bb2, unwind: bb1];
 //     }

diff --git a/src/test/mir-opt/validate_1.rs b/src/test/mir-opt/validate_1.rs
@@ -64,14 +64,14 @@ fn main() {
 //     bb0: {
 //         Validate(Acquire, [_1: &ReFree(DefId(0/1:11 ~ validate_1[317d]::main[0]::{{closure}}[0]), BrEnv) [closure@NodeId(50)], _2: &ReFree(DefId(0/1:11 ~ validate_1[317d]::main[0]::{{closure}}[0]), BrAnon(0)) mut i32]);
 //         StorageLive(_3);
-//         Validate(Suspend(ReScope(Remainder(BlockRemainder { block: ItemLocalId(22), first_statement_index: 0 }))), [(*_2): i32]);
+//         Validate(Suspend(ReScope(Remainder(BlockRemainder { block: ItemLocalId(25), first_statement_index: 0 }))), [(*_2): i32]);
 //         _3 = &ReErased (*_2);
-//         Validate(Acquire, [(*_3): i32/ReScope(Remainder(BlockRemainder { block: ItemLocalId(22), first_statement_index: 0 })) (imm)]);
+//         Validate(Acquire, [(*_3): i32/ReScope(Remainder(BlockRemainder { block: ItemLocalId(25), first_statement_index: 0 })) (imm)]);
 //         StorageLive(_4);
 //         _4 = (*_3);
 //         _0 = move _4;
 //         StorageDead(_4);
-//         EndRegion(ReScope(Remainder(BlockRemainder { block: ItemLocalId(22), first_statement_index: 0 })));
+//         EndRegion(ReScope(Remainder(BlockRemainder { block: ItemLocalId(25), first_statement_index: 0 })));
 //         StorageDead(_3);
 //         return;
 //     }

diff --git a/src/test/mir-opt/validate_5.rs b/src/test/mir-opt/validate_5.rs
@@ -53,12 +53,12 @@ fn main() {
 //         StorageLive(_3);
 //         StorageLive(_4);
 //         StorageLive(_5);
-//         Validate(Suspend(ReScope(Node(ItemLocalId(9)))), [(*_2): i32]);
+//         Validate(Suspend(ReScope(Node(ItemLocalId(12)))), [(*_2): i32]);
 //         _5 = &ReErased mut (*_2);
-//         Validate(Acquire, [(*_5): i32/ReScope(Node(ItemLocalId(9)))]);
+//         Validate(Acquire, [(*_5): i32/ReScope(Node(ItemLocalId(12)))]);
 //         _4 = move _5 as *mut i32 (Misc);
 //         _3 = move _4;
-//         EndRegion(ReScope(Node(ItemLocalId(9))));
+//         EndRegion(ReScope(Node(ItemLocalId(12))));
 //         StorageDead(_4);
 //         StorageDead(_5);
 //         Validate(Release, [_0: bool, _3: *mut i32]);

diff --git a/src/test/ui/closure-array-break-length.rs b/src/test/ui/closure-array-break-length.rs
@@ -0,0 +1,17 @@
+// Copyright 2018 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 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+fn main() {
+    |_: [_; continue]| {}; //~ ERROR: `continue` outside of loop
+
+    while |_: [_; continue]| {} {} //~ ERROR: `break` or `continue` with no label
+
+    while |_: [_; break]| {} {} //~ ERROR: `break` or `continue` with no label
+}
diff --git a/src/test/ui/closure-array-break-length.stderr b/src/test/ui/closure-array-break-length.stderr
@@ -0,0 +1,22 @@
+error[E0268]: `continue` outside of loop
+  --> $DIR/closure-array-break-length.rs:12:13
+   |
+LL |     |_: [_; continue]| {}; //~ ERROR: `continue` outside of loop
+   |             ^^^^^^^^ cannot break outside of a loop
+
+error[E0590]: `break` or `continue` with no label in the condition of a `while` loop
+  --> $DIR/closure-array-break-length.rs:14:19
+   |
+LL |     while |_: [_; continue]| {} {} //~ ERROR: `break` or `continue` with no label
+   |                   ^^^^^^^^ unlabeled `continue` in the condition of a `while` loop
+
+error[E0590]: `break` or `continue` with no label in the condition of a `while` loop
+  --> $DIR/closure-array-break-length.rs:16:19
+   |
+LL |     while |_: [_; break]| {} {} //~ ERROR: `break` or `continue` with no label
+   |                   ^^^^^ unlabeled `break` in the condition of a `while` loop
+
+error: aborting due to 3 previous errors
+
+Some errors occurred: E0268, E0590.
+For more information about an error, try `rustc --explain E0268`.