From 89188815ed1ec2d6fa498e8dc1eeecdc26e355e4 Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Sun, 7 Jul 2019 15:03:07 +0100 Subject: [PATCH 1/2] Give index temporaries a drop scope --- src/librustc_mir/build/expr/as_place.rs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/librustc_mir/build/expr/as_place.rs b/src/librustc_mir/build/expr/as_place.rs index 0640c01d255c2..82accb47437c6 100644 --- a/src/librustc_mir/build/expr/as_place.rs +++ b/src/librustc_mir/build/expr/as_place.rs @@ -73,13 +73,15 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { let (usize_ty, bool_ty) = (this.hir.usize_ty(), this.hir.bool_ty()); let slice = unpack!(block = this.as_place(block, lhs)); - // region_scope=None so place indexes live forever. They are scalars so they - // do not need storage annotations, and they are often copied between - // places. // Making this a *fresh* temporary also means we do not have to worry about // the index changing later: Nothing will ever change this temporary. // The "retagging" transformation (for Stacked Borrows) relies on this. - let idx = unpack!(block = this.as_temp(block, None, index, Mutability::Mut)); + let idx = unpack!(block = this.as_temp( + block, + expr.temp_lifetime, + index, + Mutability::Not, + )); // bounds check: let (len, lt) = ( From 163c059354ed24d8b5e1c50177fd4dd2872d63e7 Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Sun, 7 Jul 2019 15:04:43 +0100 Subject: [PATCH 2/2] Only omit StorageLive/Dead for variable that are never initialized With `feature(never_type)`, it's not guaranteed that any variable with type `!` isn't ever assigned to. --- src/librustc_mir/build/expr/as_temp.rs | 65 ++++++++++++++--------- src/test/mir-opt/loop_test.rs | 1 + src/test/ui/borrowck/assign-never-type.rs | 14 +++++ 3 files changed, 55 insertions(+), 25 deletions(-) create mode 100644 src/test/ui/borrowck/assign-never-type.rs diff --git a/src/librustc_mir/build/expr/as_temp.rs b/src/librustc_mir/build/expr/as_temp.rs index 1fe6be8bbc82e..dbcc330eca382 100644 --- a/src/librustc_mir/build/expr/as_temp.rs +++ b/src/librustc_mir/build/expr/as_temp.rs @@ -3,6 +3,7 @@ use crate::build::{BlockAnd, BlockAndExtension, Builder}; use crate::build::scope::DropKind; use crate::hair::*; +use rustc::hir; use rustc::middle::region; use rustc::mir::*; @@ -66,32 +67,46 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { }; let temp_place = &Place::from(temp); - if !expr_ty.is_never() { - this.cfg.push( - block, - Statement { - source_info, - kind: StatementKind::StorageLive(temp), - }, - ); - - // In constants, `temp_lifetime` is `None` for temporaries that live for the - // `'static` lifetime. Thus we do not drop these temporaries and simply leak them. - // This is equivalent to what `let x = &foo();` does in functions. The temporary - // is lifted to their surrounding scope. In a function that means the temporary lives - // until just before the function returns. In constants that means it outlives the - // constant's initialization value computation. Anything outliving a constant - // must have the `'static` lifetime and live forever. - // Anything with a shorter lifetime (e.g the `&foo()` in `bar(&foo())` or anything - // within a block will keep the regular drops just like runtime code. - if let Some(temp_lifetime) = temp_lifetime { - this.schedule_drop( - expr_span, - temp_lifetime, - temp, - expr_ty, - DropKind::Storage, + match expr.kind { + // Don't bother with StorageLive and Dead for these temporaries, + // they are never assigned. + ExprKind::Break { .. } | + ExprKind::Continue { .. } | + ExprKind::Return { .. } => (), + ExprKind::Block { + body: hir::Block { expr: None, targeted_by_break: false, .. } + } if expr_ty.is_never() => (), + _ => { + this.cfg.push( + block, + Statement { + source_info, + kind: StatementKind::StorageLive(temp), + }, ); + + // In constants, `temp_lifetime` is `None` for temporaries that + // live for the `'static` lifetime. Thus we do not drop these + // temporaries and simply leak them. + // This is equivalent to what `let x = &foo();` does in + // functions. The temporary is lifted to their surrounding + // scope. In a function that means the temporary lives until + // just before the function returns. In constants that means it + // outlives the constant's initialization value computation. + // Anything outliving a constant must have the `'static` + // lifetime and live forever. + // Anything with a shorter lifetime (e.g the `&foo()` in + // `bar(&foo())` or anything within a block will keep the + // regular drops just like runtime code. + if let Some(temp_lifetime) = temp_lifetime { + this.schedule_drop( + expr_span, + temp_lifetime, + temp, + expr_ty, + DropKind::Storage, + ); + } } } diff --git a/src/test/mir-opt/loop_test.rs b/src/test/mir-opt/loop_test.rs index 177080c04f972..418febbdc01eb 100644 --- a/src/test/mir-opt/loop_test.rs +++ b/src/test/mir-opt/loop_test.rs @@ -26,6 +26,7 @@ fn main() { // _1 = (); // StorageDead(_2); // StorageDead(_1); +// StorageLive(_4); // goto -> bb5; // } // ... diff --git a/src/test/ui/borrowck/assign-never-type.rs b/src/test/ui/borrowck/assign-never-type.rs new file mode 100644 index 0000000000000..4f30ea1467023 --- /dev/null +++ b/src/test/ui/borrowck/assign-never-type.rs @@ -0,0 +1,14 @@ +// Regression test for issue 62165 + +// check-pass + +#![feature(never_type)] + +pub fn main() { + loop { + match None { + None => return, + Some(val) => val, + }; + }; +}