Skip to content

Commit

Permalink
Reduced the Lvalue constructors to a kernel of three constructors.
Browse files Browse the repository at this point in the history 
Updated all call sites that used the other contructors to uniformly
call `Lvalue::new_with_hint`, passing along the appropriate kind
of hint for each context.

Placated tidy in a few other places in datum.rs.
  • Loading branch information
@pnkfelix
pnkfelix committed Jul 28, 2015
1 parent ff14eaf commit 494ce37
Show file tree
Hide file tree
Showing 3 changed files with 65 additions and 95 deletions.
33 changes: 26 additions & 7 deletions src/librustc_trans/trans/_match.rs
View file
Expand Up @@ -950,8 +950,15 @@ fn insert_lllocals<'blk, 'tcx>(mut bcx: Block<'blk, 'tcx>,
let lvalue = match binding_info.trmode {
TrByCopy(..) =>
Lvalue::new("_match::insert_lllocals"),
TrByMoveIntoCopy(..) =>
Lvalue::match_input("_match::insert_lllocals", bcx, binding_info.id),
TrByMoveIntoCopy(..) => {
// match_input moves from the input into a
// separate stack slot; it must zero (at least
// until we track drop flags for a fragmented
// parent match input expression).
let hint_kind = HintKind::ZeroAndMaintain;
Lvalue::new_with_hint("_match::insert_lllocals (match_input)",
bcx, binding_info.id, hint_kind)
}
_ => unreachable!(),
};
let datum = Datum::new(llval, binding_info.ty, lvalue);
Expand All @@ -971,10 +978,22 @@ fn insert_lllocals<'blk, 'tcx>(mut bcx: Block<'blk, 'tcx>,
TrByRef => (binding_info.llmatch, true),
};

let lvalue = Lvalue::local("_match::insert_lllocals",
bcx,
binding_info.id,
aliases_other_state);

// A local that aliases some other state must be zeroed, since
// the other state (e.g. some parent data that we matched
// into) will still have its subcomponents (such as this
// local) destructed at the end of the parent's scope. Longer
// term, we will properly map such parents to the set of
// unique drop flags for its fragments.
let hint_kind = if aliases_other_state {
HintKind::ZeroAndMaintain
} else {
HintKind::DontZeroJustUse
};
let lvalue = Lvalue::new_with_hint("_match::insert_lllocals (local)",
bcx,
binding_info.id,
hint_kind);
let datum = Datum::new(llval, binding_info.ty, lvalue);
if let Some(cs) = cs {
let opt_datum = lvalue.dropflag_hint(bcx);
Expand Down Expand Up @@ -1709,7 +1728,7 @@ fn mk_binding_alloca<'blk, 'tcx, A, F>(bcx: Block<'blk, 'tcx>,

// Allocate memory on stack for the binding.
let llval = alloc_ty(bcx, var_ty, &bcx.name(name));
let lvalue = Lvalue::binding(caller_name, bcx, p_id, name);
let lvalue = Lvalue::new_with_hint(caller_name, bcx, p_id, HintKind::DontZeroJustUse);
let datum = Datum::new(llval, var_ty, lvalue);

// Subtle: be sure that we *populate* the memory *before*
Expand Down
124 changes: 37 additions & 87 deletions src/librustc_trans/trans/datum.rs
View file
Expand Up @@ -189,96 +189,42 @@ pub struct Rvalue {
pub mode: RvalueMode
}

// XXX: reduce this to a smaller kernel of constructors.
impl Lvalue { // These are all constructors for various Lvalues.
pub fn new(source: &'static str) -> Lvalue {
Lvalue { source: source, drop_flag_info: DropFlagInfo::None }
}

pub fn upvar<'blk, 'tcx>(source: &'static str,
bcx: Block<'blk, 'tcx>,
id: ast::NodeId) -> Lvalue {
let info = if Lvalue::has_dropflag_hint(bcx, id) {
DropFlagInfo::ZeroAndMaintain(id)
} else {
DropFlagInfo::None
};
let info = if bcx.tcx().sess.nonzeroing_move_hints() { info } else { DropFlagInfo::None };
debug!("upvar Lvalue at {}, id: {} info: {:?}", source, id, info);
Lvalue { source: source, drop_flag_info: info }
}

pub fn match_input<'blk, 'tcx>(source: &'static str,
bcx: Block<'blk, 'tcx>,
id: ast::NodeId) -> Lvalue
{
let info = if Lvalue::has_dropflag_hint(bcx, id) {
// match_input is used to move from the input into a
// separate stack slot; it must zero (at least until we
// improve things to track drop flags for the fragmented
// parent match input expression).
DropFlagInfo::ZeroAndMaintain(id)
} else {
DropFlagInfo::None
};
let info = if bcx.tcx().sess.nonzeroing_move_hints() { info } else { DropFlagInfo::None };
debug!("match_input Lvalue at {}, id: {} info: {:?}", source, id, info);
Lvalue { source: source, drop_flag_info: info }
}
#[derive(Copy, Clone, Debug)]
pub enum HintKind {
ZeroAndMaintain,
DontZeroJustUse,
}

pub fn local<'blk, 'tcx>(source: &'static str,
bcx: Block<'blk, 'tcx>,
id: ast::NodeId,
aliases_other_state: bool)
-> Lvalue
{
let info = if Lvalue::has_dropflag_hint(bcx, id) {
if aliases_other_state {
DropFlagInfo::ZeroAndMaintain(id)
} else {
DropFlagInfo::DontZeroJustUse(id)
}
} else {
DropFlagInfo::None
};
let info = if bcx.tcx().sess.nonzeroing_move_hints() { info } else { DropFlagInfo::None };
debug!("local Lvalue at {}, id: {} info: {:?}", source, id, info);
Lvalue { source: source, drop_flag_info: info }
impl Lvalue { // Constructors for various Lvalues.
pub fn new<'blk, 'tcx>(source: &'static str) -> Lvalue {
debug!("Lvalue at {} no drop flag info", source);
Lvalue { source: source, drop_flag_info: DropFlagInfo::None }
}

pub fn store_arg<'blk, 'tcx>(source: &'static str,
bcx: Block<'blk, 'tcx>,
id: ast::NodeId) -> Lvalue
{
let info = if Lvalue::has_dropflag_hint(bcx, id) {
DropFlagInfo::ZeroAndMaintain(id)
} else {
DropFlagInfo::None
};
let info = if bcx.tcx().sess.nonzeroing_move_hints() { info } else { DropFlagInfo::None };
debug!("store_arg Lvalue at {}, id: {} info: {:?}", source, id, info);
Lvalue { source: source, drop_flag_info: info }
pub fn new_dropflag_hint(source: &'static str) -> Lvalue {
debug!("Lvalue at {} is drop flag hint", source);
Lvalue { source: source, drop_flag_info: DropFlagInfo::None }
}

pub fn binding<'blk, 'tcx>(source: &'static str,
bcx: Block<'blk, 'tcx>,
id: ast::NodeId,
name: ast::Name) -> Lvalue {
let info = if Lvalue::has_dropflag_hint(bcx, id) {
DropFlagInfo::DontZeroJustUse(id)
} else {
DropFlagInfo::None
pub fn new_with_hint<'blk, 'tcx>(source: &'static str,
bcx: Block<'blk, 'tcx>,
id: ast::NodeId,
k: HintKind) -> Lvalue {
let (opt_id, info) = {
let hint_available = Lvalue::has_dropflag_hint(bcx, id) &&
bcx.tcx().sess.nonzeroing_move_hints();
let info = match k {
HintKind::ZeroAndMaintain if hint_available =>
DropFlagInfo::ZeroAndMaintain(id),
HintKind::DontZeroJustUse if hint_available =>
DropFlagInfo::DontZeroJustUse(id),
_ => DropFlagInfo::None,
};
(Some(id), info)
};
let info = if bcx.tcx().sess.nonzeroing_move_hints() { info } else { DropFlagInfo::None };
debug!("binding Lvalue at {}, id: {} name: {} info: {:?}",
source, id, name, info);
debug!("Lvalue at {}, id: {:?} info: {:?}", source, opt_id, info);
Lvalue { source: source, drop_flag_info: info }
}

pub fn new_dropflag_hint(source: &'static str) -> Lvalue {
debug!("dropflag hint Lvalue at {}", source);
Lvalue { source: source, drop_flag_info: DropFlagInfo::None }
}
} // end Lvalue constructor methods.

impl Lvalue {
Expand Down Expand Up @@ -454,11 +400,15 @@ impl KindOps for Lvalue {
}
bcx
} else {
// XXX would be nice to assert this, but we currently are
// adding e.g. DontZeroJustUse flags. (The dropflag hint
// construction should be taking !type_needs_drop into
// account; earlier analysis phases may not have all the
// info they need to do it properly, I think...)
// FIXME (#5016) would be nice to assert this, but we have
// to allow for e.g. DontZeroJustUse flags, for now.
//
// (The dropflag hint construction should be taking
// !type_needs_drop into account; earlier analysis phases
// may not have all the info they need to include such
// information properly, I think; in particular the
// fragments analysis works on a non-monomorphized view of
// the code.)
//
// assert_eq!(self.drop_flag_info, DropFlagInfo::None);
bcx
Expand Down
3 changes: 2 additions & 1 deletion src/librustc_trans/trans/expr.rs
View file
Expand Up @@ -1321,7 +1321,8 @@ pub fn trans_local_var<'blk, 'tcx>(bcx: Block<'blk, 'tcx>,
def::DefUpvar(nid, _) => {
// Can't move upvars, so this is never a ZeroMemLastUse.
let local_ty = node_id_type(bcx, nid);
let lval = Lvalue::upvar("expr::trans_local_var", bcx, nid);
let lval = Lvalue::new_with_hint("expr::trans_local_var (upvar)",
bcx, nid, HintKind::ZeroAndMaintain);
match bcx.fcx.llupvars.borrow().get(&nid) {
Some(&val) => Datum::new(val, local_ty, lval),
None => {
Expand Down

0 comments on commit 494ce37

Please sign in to comment.