From 9e543219c133b1dc9bd4335a080559b9453d5f81 Mon Sep 17 00:00:00 2001 From: Ariel Ben-Yehuda Date: Mon, 12 Sep 2016 01:53:43 +0300 Subject: [PATCH 1/4] use `adt::trans_const` when translating constant closures and tuples Fixes #36401 --- src/librustc_trans/mir/constant.rs | 86 ++++++++++++++++++++---------- src/test/run-pass/issue-36401.rs | 25 +++++++++ 2 files changed, 83 insertions(+), 28 deletions(-) create mode 100644 src/test/run-pass/issue-36401.rs diff --git a/src/librustc_trans/mir/constant.rs b/src/librustc_trans/mir/constant.rs index 31fee560fe369..2c83b7d07cc54 100644 --- a/src/librustc_trans/mir/constant.rs +++ b/src/librustc_trans/mir/constant.rs @@ -36,6 +36,7 @@ use value::Value; use syntax_pos::{Span, DUMMY_SP}; +use std::fmt; use std::ptr; use super::operand::{OperandRef, OperandValue}; @@ -147,6 +148,12 @@ impl<'tcx> Const<'tcx> { } } +impl<'tcx> fmt::Debug for Const<'tcx> { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + write!(f, "Const({:?}: {:?})", Value(self.llval), self.ty) + } +} + #[derive(Copy, Clone)] enum Base { /// A constant value without an unique address. @@ -466,7 +473,8 @@ impl<'a, 'tcx> MirConstContext<'a, 'tcx> { fn const_operand(&self, operand: &mir::Operand<'tcx>, span: Span) -> Result, ConstEvalFailure> { - match *operand { + debug!("const_operand({:?} @ {:?})", operand, span); + let result = match *operand { mir::Operand::Consume(ref lvalue) => { Ok(self.const_lvalue(lvalue, span)?.to_const(span)) } @@ -495,13 +503,33 @@ impl<'a, 'tcx> MirConstContext<'a, 'tcx> { } } } - } + }; + debug!("const_operand({:?} @ {:?}) = {:?}", operand, span, + result.as_ref().ok()); + result + } + + fn const_array(&self, array_ty: Ty<'tcx>, fields: &[ValueRef]) + -> Const<'tcx> + { + let elem_ty = array_ty.builtin_index().unwrap_or_else(|| { + bug!("bad array type {:?}", array_ty) + }); + let llunitty = type_of::type_of(self.ccx, elem_ty); + // If the array contains enums, an LLVM array won't work. + let val = if fields.iter().all(|&f| val_ty(f) == llunitty) { + C_array(llunitty, fields) + } else { + C_struct(self.ccx, fields, false) + }; + Const::new(val, array_ty) } fn const_rvalue(&self, rvalue: &mir::Rvalue<'tcx>, dest_ty: Ty<'tcx>, span: Span) -> Result, ConstEvalFailure> { let tcx = self.ccx.tcx(); + debug!("const_rvalue({:?}: {:?} @ {:?})", rvalue, dest_ty, span); let val = match *rvalue { mir::Rvalue::Use(ref operand) => self.const_operand(operand, span)?, @@ -509,15 +537,7 @@ impl<'a, 'tcx> MirConstContext<'a, 'tcx> { let elem = self.const_operand(elem, span)?; let size = count.value.as_u64(tcx.sess.target.uint_type); let fields = vec![elem.llval; size as usize]; - - let llunitty = type_of::type_of(self.ccx, elem.ty); - // If the array contains enums, an LLVM array won't work. - let val = if val_ty(elem.llval) == llunitty { - C_array(llunitty, &fields) - } else { - C_struct(self.ccx, &fields, false) - }; - Const::new(val, dest_ty) + self.const_array(dest_ty, &fields) } mir::Rvalue::Aggregate(ref kind, ref operands) => { @@ -541,22 +561,26 @@ impl<'a, 'tcx> MirConstContext<'a, 'tcx> { self.monomorphize(&substs)); } - let val = if let mir::AggregateKind::Adt(adt_def, index, _) = *kind { - let repr = adt::represent_type(self.ccx, dest_ty); - let disr = Disr::from(adt_def.variants[index].disr_val); - adt::trans_const(self.ccx, &repr, disr, &fields) - } else if let ty::TyArray(elem_ty, _) = dest_ty.sty { - let llunitty = type_of::type_of(self.ccx, elem_ty); - // If the array contains enums, an LLVM array won't work. - if fields.iter().all(|&f| val_ty(f) == llunitty) { - C_array(llunitty, &fields) - } else { - C_struct(self.ccx, &fields, false) + match *kind { + mir::AggregateKind::Vec => { + self.const_array(dest_ty, &fields) } - } else { - C_struct(self.ccx, &fields, false) - }; - Const::new(val, dest_ty) + mir::AggregateKind::Adt(..) | + mir::AggregateKind::Closure(..) | + mir::AggregateKind::Tuple => { + let disr = match *kind { + mir::AggregateKind::Adt(adt_def, index, _) => { + Disr::from(adt_def.variants[index].disr_val) + } + _ => Disr(0) + }; + let repr = adt::represent_type(self.ccx, dest_ty); + Const::new( + adt::trans_const(self.ccx, &repr, disr, &fields), + dest_ty + ) + } + } } mir::Rvalue::Cast(ref kind, ref source, cast_ty) => { @@ -780,6 +804,8 @@ impl<'a, 'tcx> MirConstContext<'a, 'tcx> { _ => span_bug!(span, "{:?} in constant", rvalue) }; + debug!("const_rvalue({:?}: {:?} @ {:?}) = {:?}", rvalue, dest_ty, span, val); + Ok(val) } @@ -881,6 +907,7 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> { constant: &mir::Constant<'tcx>) -> Const<'tcx> { + debug!("trans_constant({:?})", constant); let ty = bcx.monomorphize(&constant.ty); let result = match constant.literal.clone() { mir::Literal::Item { def_id, substs } => { @@ -905,7 +932,7 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> { } }; - match result { + let result = match result { Ok(v) => v, Err(ConstEvalFailure::Compiletime(_)) => { // We've errored, so we don't have to produce working code. @@ -917,7 +944,10 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> { "MIR constant {:?} results in runtime panic: {:?}", constant, err.description()) } - } + }; + + debug!("trans_constant({:?}) = {:?}", constant, result); + result } } diff --git a/src/test/run-pass/issue-36401.rs b/src/test/run-pass/issue-36401.rs new file mode 100644 index 0000000000000..7b08eba9e4988 --- /dev/null +++ b/src/test/run-pass/issue-36401.rs @@ -0,0 +1,25 @@ +// Copyright 2016 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. + +#[derive(Debug)] +pub enum Event { + Key(u8), + Resize, + Unknown(u16), +} + +static XTERM_SINGLE_BYTES : [(u8, Event); 1] = [(1, Event::Resize)]; + +fn main() { + match XTERM_SINGLE_BYTES[0] { + (1, Event::Resize) => {}, + ref bad => panic!("unexpected {:?}", bad) + } +} From 0129358ffc9ad7a60b0f983eebac1383db4f6268 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Tue, 13 Sep 2016 16:04:27 -0400 Subject: [PATCH 2/4] invoke drop glue with a ptr to (data, meta) This is done by creating a little space on the stack. Hokey, but it's the simplest fix I can see. --- src/librustc_trans/mir/block.rs | 24 +++++++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/src/librustc_trans/mir/block.rs b/src/librustc_trans/mir/block.rs index 56d02fa1fac4f..fbd2191ea7183 100644 --- a/src/librustc_trans/mir/block.rs +++ b/src/librustc_trans/mir/block.rs @@ -242,10 +242,28 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> { let lvalue = self.trans_lvalue(&bcx, location); let drop_fn = glue::get_drop_glue(bcx.ccx(), ty); let drop_ty = glue::get_drop_glue_type(bcx.tcx(), ty); - let llvalue = if drop_ty != ty { - bcx.pointercast(lvalue.llval, type_of::type_of(bcx.ccx(), drop_ty).ptr_to()) + let is_sized = common::type_is_sized(bcx.tcx(), ty); + let llvalue = if is_sized { + if drop_ty != ty { + bcx.pointercast(lvalue.llval, type_of::type_of(bcx.ccx(), drop_ty).ptr_to()) + } else { + lvalue.llval + } } else { - lvalue.llval + // FIXME(#36457) Currently drop glue takes sized + // values as a `*(data, meta)`, but elsewhere in + // MIR we pass `(data, meta)` as two separate + // arguments. It would be better to fix drop glue, + // but I am shooting for a quick fix to #35546 + // here that can be cleanly backported to beta, so + // I want to avoid touching all of trans. + bcx.with_block(|bcx| { + let scratch = base::alloc_ty(bcx, ty, "drop"); + base::call_lifetime_start(bcx, scratch); + build::Store(bcx, lvalue.llval, base::get_dataptr(bcx, scratch)); + build::Store(bcx, lvalue.llextra, base::get_meta(bcx, scratch)); + scratch + }) }; if let Some(unwind) = unwind { bcx.invoke(drop_fn, From b31e845d57eb99d3f2f6bbe6f071794533004996 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Tue, 13 Sep 2016 18:33:35 -0400 Subject: [PATCH 3/4] add missing test --- src/test/run-pass/issue-35546.rs | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) create mode 100644 src/test/run-pass/issue-35546.rs diff --git a/src/test/run-pass/issue-35546.rs b/src/test/run-pass/issue-35546.rs new file mode 100644 index 0000000000000..e8d14f1d42146 --- /dev/null +++ b/src/test/run-pass/issue-35546.rs @@ -0,0 +1,28 @@ +// Copyright 2016 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. + +// Regression test for #35546. Check that we are able to codegen +// this. Before we had problems because of the drop glue signature +// around dropping a trait object (specifically, when dropping the +// `value` field of `Node`). + +struct Node { + next: Option>>, + value: T, +} + +fn clear(head: &mut Option>>) { + match head.take() { + Some(node) => *head = node.next, + None => (), + } +} + +fn main() {} From f0d4add18232d16fabf2e01a6eb0d60a4c7293a4 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Wed, 21 Sep 2016 15:15:39 +0200 Subject: [PATCH 4/4] fix PR 36459 post backport to beta. --- src/librustc_trans/mir/block.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/librustc_trans/mir/block.rs b/src/librustc_trans/mir/block.rs index fbd2191ea7183..6d03b44744b43 100644 --- a/src/librustc_trans/mir/block.rs +++ b/src/librustc_trans/mir/block.rs @@ -23,6 +23,7 @@ use common::{C_bool, C_str_slice, C_struct, C_u32, C_undef}; use consts; use debuginfo::DebugLoc; use Disr; +use expr; use machine::{llalign_of_min, llbitsize_of_real}; use meth; use type_of; @@ -260,8 +261,8 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> { bcx.with_block(|bcx| { let scratch = base::alloc_ty(bcx, ty, "drop"); base::call_lifetime_start(bcx, scratch); - build::Store(bcx, lvalue.llval, base::get_dataptr(bcx, scratch)); - build::Store(bcx, lvalue.llextra, base::get_meta(bcx, scratch)); + build::Store(bcx, lvalue.llval, expr::get_dataptr(bcx, scratch)); + build::Store(bcx, lvalue.llextra, expr::get_meta(bcx, scratch)); scratch }) };