rustc: Cleaning up bad copies and other XXXes

Author: catamorphism

src/librustc/middle/trans/build.rs

@@ -602,7 +602,7 @@ pub fn GEP(cx: block, Pointer: ValueRef, Indices: &[ValueRef]) -> ValueRef {
 // Simple wrapper around GEP that takes an array of ints and wraps them
 // in C_i32()
 //
-// XXX: Use a small-vector optimization to avoid allocations here.
+// FIXME #6571: Use a small-vector optimization to avoid allocations here.
 pub fn GEPi(cx: block, base: ValueRef, ixs: &[uint]) -> ValueRef {
     let v = do vec::map(ixs) |i| { C_i32(*i as i32) };
     count_insn(cx, "gepi");

src/librustc/middle/trans/closure.rs

@@ -137,7 +137,7 @@ pub fn mk_tuplified_uniq_cbox_ty(tcx: ty::ctxt, cdata_ty: ty::t) -> ty::t {
 
 // Given a closure ty, emits a corresponding tuple ty
 pub fn mk_closure_tys(tcx: ty::ctxt,
-                      bound_values: ~[EnvValue])
+                      bound_values: &[EnvValue])
                    -> ty::t {
     // determine the types of the values in the env.  Note that this
     // is the actual types that will be stored in the map, not the
@@ -203,8 +203,7 @@ pub fn store_environment(bcx: block,
     let ccx = bcx.ccx(), tcx = ccx.tcx;
 
     // compute the shape of the closure
-    // XXX: Bad copy.
-    let cdata_ty = mk_closure_tys(tcx, copy bound_values);
+    let cdata_ty = mk_closure_tys(tcx, bound_values);
 
     // allocate closure in the heap
     let Result {bcx: bcx, val: llbox} = allocate_cbox(bcx, sigil, cdata_ty);

src/librustc/middle/trans/common.rs

@@ -1325,7 +1325,7 @@ pub fn is_null(val: ValueRef) -> bool {
 // Used to identify cached monomorphized functions and vtables
 #[deriving(Eq)]
 pub enum mono_param_id {
-    mono_precise(ty::t, Option<~[mono_id]>),
+    mono_precise(ty::t, Option<@~[mono_id]>),
     mono_any,
     mono_repr(uint /* size */,
               uint /* align */,

src/librustc/middle/trans/datum.rs

@@ -652,7 +652,7 @@ pub impl Datum {
                     ByRef => {
                         // Recast lv.val as a pointer to the newtype rather
                         // than a pointer to the struct type.
-                        // XXX: This isn't correct for structs with
+                        // FIXME #6572: This isn't correct for structs with
                         // destructors.
                         (
                             Some(Datum {

src/librustc/middle/trans/expr.rs

@@ -576,7 +576,7 @@ fn trans_rvalue_dps_unadjusted(bcx: block, expr: @ast::expr,
             };
         }
         ast::expr_struct(_, ref fields, base) => {
-            return trans_rec_or_struct(bcx, (*fields), base, expr.id, dest);
+            return trans_rec_or_struct(bcx, (*fields), base, expr.span, expr.id, dest);
         }
         ast::expr_tup(ref args) => {
             let repr = adt::represent_type(bcx.ccx(), expr_ty(bcx, expr));
@@ -721,7 +721,7 @@ fn trans_def_dps_unadjusted(bcx: block, ref_expr: @ast::expr,
         }
         ast::def_struct(*) => {
             // Nothing to do here.
-            // XXX: May not be true in the case of classes with destructors.
+            // FIXME #6572: May not be true in the case of classes with destructors.
             return bcx;
         }
         _ => {
@@ -1129,6 +1129,7 @@ pub fn with_field_tys<R>(tcx: ty::ctxt,
 fn trans_rec_or_struct(bcx: block,
                        fields: &[ast::field],
                        base: Option<@ast::expr>,
+                       expr_span: codemap::span,
                        id: ast::node_id,
                        dest: Dest) -> block
 {
@@ -1167,8 +1168,7 @@ fn trans_rec_or_struct(bcx: block,
             }
             None => {
                 if need_base.any(|b| *b) {
-                    // XXX should be span bug
-                    tcx.sess.bug(~"missing fields and no base expr")
+                    tcx.sess.span_bug(expr_span, ~"missing fields and no base expr")
                 }
                 None
             }
@@ -1232,8 +1232,8 @@ fn trans_adt(bcx: block, repr: &adt::Repr, discr: int,
         temp_cleanups.push(dest);
     }
     for optbase.each |base| {
-        // XXX is it sound to use the destination's repr on the base?
-        // XXX would it ever be reasonable to be here with discr != 0?
+        // FIXME #6573: is it sound to use the destination's repr on the base?
+        // And, would it ever be reasonable to be here with discr != 0?
         let base_datum = unpack_datum!(bcx, trans_to_datum(bcx, base.expr));
         for base.fields.each |&(i, t)| {
             let datum = do base_datum.get_element(bcx, t, ZeroMem) |srcval| {

src/librustc/middle/trans/foreign.rs

@@ -550,14 +550,13 @@ pub fn trans_intrinsic(ccx: @CrateContext,
 
     let output_type = ty::ty_fn_ret(ty::node_id_to_type(ccx.tcx, item.id));
 
-    // XXX: Bad copy.
     let fcx = new_fn_ctxt_w_id(ccx,
                                path,
                                decl,
                                item.id,
                                output_type,
                                None,
-                               Some(copy substs),
+                               Some(substs),
                                Some(item.span));
 
     // Set the fixed stack segment flag if necessary.

src/librustc/middle/trans/glue.rs

@@ -669,7 +669,7 @@ pub fn declare_tydesc(ccx: @CrateContext, t: ty::t) -> @mut tydesc_info {
     let llsize = llsize_of(ccx, llty);
     let llalign = llalign_of(ccx, llty);
     let addrspace = declare_tydesc_addrspace(ccx, t);
-    //XXX this triggers duplicate LLVM symbols
+    // FIXME #6574: this triggers duplicate LLVM symbols
     let name = @(if false /*ccx.sess.opts.debuginfo*/ {
         mangle_internal_name_by_type_only(ccx, t, "tydesc")
     } else {
@@ -703,14 +703,13 @@ pub fn declare_generic_glue(ccx: @CrateContext, t: ty::t, llfnty: TypeRef,
                             name: ~str) -> ValueRef {
     let _icx = ccx.insn_ctxt("declare_generic_glue");
     let name = name;
-    //XXX this triggers duplicate LLVM symbols
+    // FIXME #6574 this triggers duplicate LLVM symbols
     let fn_nm = @(if false /*ccx.sess.opts.debuginfo*/ {
         mangle_internal_name_by_type_only(ccx, t, (~"glue_" + name))
     } else {
         mangle_internal_name_by_seq(ccx, (~"glue_" + name))
     });
     debug!("%s is for type %s", *fn_nm, ppaux::ty_to_str(ccx.tcx, t));
-    // XXX: Bad copy.
     note_unique_llvm_symbol(ccx, fn_nm);
     let llfn = decl_cdecl_fn(ccx.llmod, *fn_nm, llfnty);
     set_glue_inlining(llfn, t);

src/librustc/middle/trans/meth.rs

@@ -401,7 +401,7 @@ pub fn method_with_name_or_default(ccx: @CrateContext,
                       Some(pmis) => {
                           for pmis.each |pmi| {
                               if pmi.method_info.ident == name {
-                                  debug!("XXX %?", pmi.method_info.did);
+                                  debug!("pmi.method_info.did = %?", pmi.method_info.did);
                                   return pmi.method_info.did;
                               }
                           }
@@ -734,15 +734,15 @@ pub fn trans_trait_callee_from_llval(bcx: block,
 }
 
 pub fn vtable_id(ccx: @CrateContext,
-                 origin: typeck::vtable_origin)
+                 origin: &typeck::vtable_origin)
               -> mono_id {
     match origin {
-        typeck::vtable_static(impl_id, substs, sub_vtables) => {
+        &typeck::vtable_static(impl_id, ref substs, sub_vtables) => {
             monomorphize::make_mono_id(
                 ccx,
                 impl_id,
-                substs,
-                if (*sub_vtables).len() == 0u {
+                *substs,
+                if sub_vtables.is_empty() {
                     None
                 } else {
                     Some(sub_vtables)
@@ -759,8 +759,7 @@ pub fn vtable_id(ccx: @CrateContext,
 pub fn get_vtable(ccx: @CrateContext,
                   origin: typeck::vtable_origin)
                -> ValueRef {
-    // XXX: Bad copy.
-    let hash_id = vtable_id(ccx, copy origin);
+    let hash_id = vtable_id(ccx, &origin);
     match ccx.vtables.find(&hash_id) {
       Some(&val) => val,
       None => match origin {

src/librustc/middle/trans/monomorphize.rs

@@ -70,8 +70,7 @@ pub fn monomorphic_fn(ccx: @CrateContext,
     for real_substs.each() |s| { assert!(!ty::type_has_params(*s)); }
     for substs.each() |s| { assert!(!ty::type_has_params(*s)); }
     let param_uses = type_use::type_uses_for(ccx, fn_id, substs.len());
-    // XXX: Bad copy.
-    let hash_id = make_mono_id(ccx, fn_id, copy substs, vtables, impl_did_opt,
+    let hash_id = make_mono_id(ccx, fn_id, substs, vtables, impl_did_opt,
                                Some(param_uses));
     if vec::any(hash_id.params,
                 |p| match *p { mono_precise(_, _) => false, _ => true }) {
@@ -350,10 +349,10 @@ pub fn make_mono_id(ccx: @CrateContext,
         vec::map_zip(*item_ty.generics.type_param_defs, substs, |type_param_def, subst| {
             let mut v = ~[];
             for type_param_def.bounds.trait_bounds.each |_bound| {
-                v.push(meth::vtable_id(ccx, /*bad*/copy vts[i]));
+                v.push(meth::vtable_id(ccx, &vts[i]));
                 i += 1;
             }
-            (*subst, if !v.is_empty() { Some(v) } else { None })
+            (*subst, if !v.is_empty() { Some(@v) } else { None })
         })
       }
       None => {
@@ -369,8 +368,7 @@ pub fn make_mono_id(ccx: @CrateContext,
                 }
             } else {
                 match *id {
-                    // XXX: Bad copy.
-                    (a, copy b@Some(_)) => mono_precise(a, b),
+                    (a, b@Some(_)) => mono_precise(a, b),
                     (subst, None) => {
                         if *uses == 0 {
                             mono_any

src/librustc/middle/trans/reflect.rs

@@ -84,7 +84,7 @@ pub impl Reflector {
           self.c_tydesc(mt.ty)]
     }
 
-    fn visit(&mut self, ty_name: ~str, args: ~[ValueRef]) {
+    fn visit(&mut self, ty_name: ~str, args: &[ValueRef]) {
         let tcx = self.bcx.tcx();
         let mth_idx = ty::method_idx(
             tcx.sess.ident_of(~"visit_" + ty_name),
@@ -121,10 +121,9 @@ pub impl Reflector {
 
     fn bracketed(&mut self,
                  bracket_name: ~str,
-                 extra: ~[ValueRef],
+                 extra: &[ValueRef],
                  inner: &fn(&mut Reflector)) {
-        // XXX: Bad copy.
-        self.visit(~"enter_" + bracket_name, copy extra);
+        self.visit(~"enter_" + bracket_name, extra);
         inner(self);
         self.visit(~"leave_" + bracket_name, extra);
     }
@@ -226,7 +225,7 @@ pub impl Reflector {
                           self.c_uint(sigilval),
                           self.c_uint(fty.sig.inputs.len()),
                           self.c_uint(retval)];
-            self.visit(~"enter_fn", copy extra);    // XXX: Bad copy.
+            self.visit(~"enter_fn", extra);
             self.visit_sig(retval, &fty.sig);
             self.visit(~"leave_fn", extra);
           }
@@ -241,7 +240,7 @@ pub impl Reflector {
                           self.c_uint(sigilval),
                           self.c_uint(fty.sig.inputs.len()),
                           self.c_uint(retval)];
-            self.visit(~"enter_fn", copy extra);    // XXX: Bad copy.
+            self.visit(~"enter_fn", extra);
             self.visit_sig(retval, &fty.sig);
             self.visit(~"leave_fn", extra);
           }

src/librustc/middle/trans/type_of.rs

@@ -45,7 +45,7 @@ pub fn type_of_fn(cx: @CrateContext, inputs: &[ty::t], output: ty::t)
         if !output_is_immediate {
             atys.push(T_ptr(lloutputtype));
         } else {
-            // XXX: Eliminate this.
+            // FIXME #6575: Eliminate this.
             atys.push(T_ptr(T_i8()));
         }
 
@@ -200,7 +200,6 @@ pub fn type_of(cx: @CrateContext, t: ty::t) -> TypeRef {
         return llty;
     }
 
-    // XXX: This is a terrible terrible copy.
     let llty = match ty::get(t).sty {
       ty::ty_nil | ty::ty_bot => T_nil(),
       ty::ty_bool => T_bool(),
@@ -219,7 +218,7 @@ pub fn type_of(cx: @CrateContext, t: ty::t) -> TypeRef {
         common::T_named_struct(llvm_type_name(cx,
                                               an_enum,
                                               did,
-                                              /*bad*/copy substs.tps))
+                                              substs.tps))
       }
       ty::ty_estr(ty::vstore_box) => {
         T_box_ptr(T_box(cx, T_vec(cx, T_i8())))
@@ -280,7 +279,7 @@ pub fn type_of(cx: @CrateContext, t: ty::t) -> TypeRef {
           T_named_struct(llvm_type_name(cx,
                                         a_struct,
                                         did,
-                                        /*bad*/ copy substs.tps))
+                                        substs.tps))
         }
       }
       ty::ty_self(*) => cx.tcx.sess.unimpl(~"type_of: ty_self"),

src/librustc/middle/ty.rs

@@ -681,7 +681,6 @@ pub enum type_err {
     terr_trait_stores_differ(terr_vstore_kind, expected_found<TraitStore>),
     terr_in_field(@type_err, ast::ident),
     terr_sorts(expected_found<t>),
-    terr_self_substs,
     terr_integer_as_char,
     terr_int_mismatch(expected_found<IntVarValue>),
     terr_float_mismatch(expected_found<ast::float_ty>),
@@ -3722,9 +3721,6 @@ pub fn type_err_to_str(cx: ctxt, err: &type_err) -> ~str {
                      values.found.user_string(cx))
             }
         }
-        terr_self_substs => {
-            ~"inconsistent self substitution" // XXX this is more of a bug
-        }
         terr_integer_as_char => {
             fmt!("expected an integral type but found char")
         }

src/librustc/middle/typeck/check/method.rs

@@ -438,7 +438,7 @@ pub impl<'self> LookupContext<'self> {
                 return; // inapplicable
             }
             ast::sty_region(_) => vstore_slice(r)
-            ast::sty_box(_) => vstore_box, // XXX NDM mutability
+            ast::sty_box(_) => vstore_box, // NDM mutability, as per #5762
             ast::sty_uniq(_) => vstore_uniq
         }
         */
@@ -594,7 +594,7 @@ pub impl<'self> LookupContext<'self> {
             let method = ty::method(self.tcx(),
                                     provided_method_info.method_info.did);
 
-            // XXX: Needs to support generics.
+            // FIXME #4099 (?) Needs to support generics.
             let dummy_substs = substs {
                 self_r: None,
                 self_ty: None,

src/librustc/middle/typeck/check/mod.rs

@@ -1817,7 +1817,6 @@ pub fn check_expr_with_unifier(fcx: @mut FnCtxt,
         let mut class_field_map = HashMap::new();
         let mut fields_found = 0;
         for field_types.each |field| {
-            // XXX: Check visibility here.
             class_field_map.insert(field.ident, (field.id, false));
         }
 

src/librustc/middle/typeck/infer/combine.rs

@@ -286,7 +286,9 @@ pub fn super_self_tys<C:Combine>(
         // I think it should never happen that we unify two substs and
         // one of them has a self_ty and one doesn't...? I could be
         // wrong about this.
-        Err(ty::terr_self_substs)
+          this.infcx().tcx.sess.bug(
+              fmt!("substitution a had a self_ty and substitution b didn't, \
+                    or vice versa"));
       }
     }
 }