Auto merge of #21692 - pnkfelix:fsk-fix-coerce-match-20055, r=eddyb

trans: When coercing to `Box<Trait>` or `Box<[T]>`, leave datum in it's original L-/R-value state.

This fixes a subtle issue where temporaries were being allocated (but not necessarily initialized) to the (parent) terminating scope of a match expression; in particular, the code to zero out the temporary emitted by `datum.store_to` is only attached to the particular match-arm for that temporary, but when going down other arms of the match expression, the temporary may falsely appear to have been initialized, depending on what the stack held at that location, and thus may have its destructor erroneously run at the end of the terminating scope.

FIx #20055.

(There may be a latent bug still remaining in `fn into_fat_ptr`, but I am so annoyed by the test/run-pass/coerce_match.rs failures that I want to land this now.)

src/librustc_trans/trans/expr.rs

@@ -420,9 +420,15 @@ fn apply_adjustments<'blk, 'tcx>(bcx: Block<'blk, 'tcx>,
  let tcx = bcx.tcx();
 
  let datum_ty = datum.ty;
- // Arrange cleanup
- let lval = unpack_datum!(bcx,
- datum.to_lvalue_datum(bcx, "unsize_unique_vec", expr.id));
+
+ debug!("unsize_unique_vec expr.id={} datum_ty={} len={}",
+ expr.id, datum_ty.repr(tcx), len);
+
+ // We do not arrange cleanup ourselves; if we already are an
+ // L-value, then cleanup will have already been scheduled (and
+ // the `datum.store_to` call below will emit code to zero the
+ // drop flag when moving out of the L-value). If we are an R-value,
+ // then we do not need to schedule cleanup.
 
  let ll_len = C_uint(bcx.ccx(), len);
  let unit_ty = ty::sequence_element_type(tcx, ty::type_content(datum_ty));
@@ -433,7 +439,7 @@ fn apply_adjustments<'blk, 'tcx>(bcx: Block<'blk, 'tcx>,
  let base = PointerCast(bcx,
  base,
  type_of::type_of(bcx.ccx(), datum_ty).ptr_to());
- bcx = lval.store_to(bcx, base);
+ bcx = datum.store_to(bcx, base);
 
  Store(bcx, ll_len, get_len(bcx, scratch.val));
  DatumBlock::new(bcx, scratch.to_expr_datum())
@@ -455,22 +461,20 @@ fn apply_adjustments<'blk, 'tcx>(bcx: Block<'blk, 'tcx>,
  };
  let result_ty = ty::mk_uniq(tcx, ty::unsize_ty(tcx, unboxed_ty, k, expr.span));
 
- let lval = unpack_datum!(bcx,
- datum.to_lvalue_datum(bcx, "unsize_unique_expr", expr.id));
+ // We do not arrange cleanup ourselves; if we already are an
+ // L-value, then cleanup will have already been scheduled (and
+ // the `datum.store_to` call below will emit code to zero the
+ // drop flag when moving out of the L-value). If we are an R-value,
+ // then we do not need to schedule cleanup.
 
  let scratch = rvalue_scratch_datum(bcx, result_ty, "__uniq_fat_ptr");
  let llbox_ty = type_of::type_of(bcx.ccx(), datum_ty);
  let base = PointerCast(bcx, get_dataptr(bcx, scratch.val), llbox_ty.ptr_to());
- bcx = lval.store_to(bcx, base);
+ bcx = datum.store_to(bcx, base);
 
  let info = unsized_info(bcx, k, expr.id, unboxed_ty, |t| ty::mk_uniq(tcx, t));
  Store(bcx, info, get_len(bcx, scratch.val));
 
- let scratch = unpack_datum!(bcx,
- scratch.to_expr_datum().to_lvalue_datum(bcx,
- "fresh_uniq_fat_ptr",
- expr.id));
-
  DatumBlock::new(bcx, scratch.to_expr_datum())
  }
 }

src/test/run-pass/issue-20055-box-trait.rs

@@ -0,0 +1,48 @@
+// Copyright 2015 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.
+
+// See Issues #20055 and #21695.
+
+// We are checking here that the temporaries `Box<[i8, k]>`, for `k`
+// in 1, 2, 3, 4, that are induced by the match expression are
+// properly handled, in that only *one* will be initialized by
+// whichever arm is run, and subsequently dropped at the end of the
+// statement surrounding the `match`.
+
+trait Boo { }
+
+impl Boo for [i8; 1] { }
+impl Boo for [i8; 2] { }
+impl Boo for [i8; 3] { }
+impl Boo for [i8; 4] { }
+
+pub fn foo(box_1: fn () -> Box<[i8; 1]>,
+ box_2: fn () -> Box<[i8; 2]>,
+ box_3: fn () -> Box<[i8; 3]>,
+ box_4: fn () -> Box<[i8; 4]>,
+ ) {
+ println!("Hello World 1");
+ let _: Box<Boo> = match 3 {
+ 1 => box_1(),
+ 2 => box_2(),
+ 3 => box_3(),
+ _ => box_4(),
+ };
+ println!("Hello World 2");
+}
+
+pub fn main() {
+ fn box_1() -> Box<[i8; 1]> { Box::new( [1i8; 1] ) }
+ fn box_2() -> Box<[i8; 2]> { Box::new( [1i8; 2] ) }
+ fn box_3() -> Box<[i8; 3]> { Box::new( [1i8; 3] ) }
+ fn box_4() -> Box<[i8; 4]> { Box::new( [1i8; 4] ) }
+
+ foo(box_1, box_2, box_3, box_4);
+}

src/test/run-pass/issue-20055-box-unsized-array.rs

@@ -0,0 +1,38 @@
+// Copyright 2015 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.
+
+// Issue #2005: Check that boxed fixed-size arrays are properly
+// accounted for (namely, only deallocated if they were actually
+// created) when they appear as temporaries in unused arms of a match
+// expression.
+
+pub fn foo(box_1: fn () -> Box<[i8; 1]>,
+ box_2: fn () -> Box<[i8; 20]>,
+ box_3: fn () -> Box<[i8; 300]>,
+ box_4: fn () -> Box<[i8; 4000]>,
+ ) {
+ println!("Hello World 1");
+ let _: Box<[i8]> = match 3 {
+ 1 => box_1(),
+ 2 => box_2(),
+ 3 => box_3(),
+ _ => box_4(),
+ };
+ println!("Hello World 2");
+}
+
+pub fn main() {
+ fn box_1() -> Box<[i8; 1]> { Box::new( [1i8] ) }
+ fn box_2() -> Box<[i8; 20]> { Box::new( [1i8; 20] ) }
+ fn box_3() -> Box<[i8; 300]> { Box::new( [1i8; 300] ) }
+ fn box_4() -> Box<[i8; 4000]> { Box::new( [1i8; 4000] ) }
+
+ foo(box_1, box_2, box_3, box_4);
+}