Reduced the Lvalue constructors to a kernel of three constructors.

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.

Author: pnkfelix

src/librustc_trans/trans/_match.rs

@@ -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);
@@ -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);
@@ -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*

src/librustc_trans/trans/datum.rs

@@ -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 {
@@ -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

src/librustc_trans/trans/expr.rs

@@ -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 => {