Fix some bugs with globs and shadowing

src/libcollections/slice.rs

@@ -1343,7 +1343,10 @@ pub mod raw {
 #[cfg(test)]
 mod tests {
     use std::boxed::Box;
-    use prelude::*;
+    use prelude::{Some, None, range, Vec, ToString, Clone, Greater, Less, Equal};
+    use prelude::{SliceExt, Iterator, IteratorExt, DoubleEndedIteratorExt};
+    use prelude::{OrdSliceExt, CloneSliceExt, PartialEqSliceExt, AsSlice};
+    use prelude::{RandomAccessIterator, Ord, VectorVector};
     use core::cell::Cell;
     use core::default::Default;
     use core::mem;

src/libcollections/str.rs

@@ -3331,7 +3331,7 @@ mod tests {
 #[cfg(test)]
 mod bench {
     use super::*;
-    use prelude::*;
+    use prelude::{SliceExt, IteratorExt, DoubleEndedIteratorExt};
     use test::Bencher;
     use test::black_box;
 

src/librustc_resolve/lib.rs

@@ -345,9 +345,6 @@ impl Rib {
 #[deriving(Show,PartialEq,Clone,Copy)]
 enum Shadowable {
     Always,
-    /// Means that the recorded import obeys the glob shadowing rules, i.e., can
-    /// only be shadowed by another glob import.
-    Glob,
     Never
 }
 
@@ -462,6 +459,22 @@ impl ImportResolution {
 
         target.unwrap().shadowable
     }
+
+    fn set_target_and_id(&mut self,
+                         namespace: Namespace,
+                         target: Option<Target>,
+                         id: NodeId) {
+        match namespace {
+            TypeNS  => {
+                self.type_target = target;
+                self.type_id = id;
+            }
+            ValueNS => {
+                self.value_target = target;
+                self.value_id = id;
+            }
+        }
+    }
 }
 
 /// The link from a module up to its nearest parent node.
@@ -1719,11 +1732,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
                                                     view_path.span,
                                                     id,
                                                     is_public,
-                                                    if shadowable == Shadowable::Never {
-                                                        Shadowable::Glob
-                                                    } else {
-                                                        shadowable
-                                                    });
+                                                    shadowable);
                     }
                 }
             }
@@ -2712,64 +2721,45 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
         // We've successfully resolved the import. Write the results in.
         let mut import_resolutions = module_.import_resolutions.borrow_mut();
         let import_resolution = &mut (*import_resolutions)[target];
+        {
+            let check_and_write_import = |namespace, result: &_, used_public: &mut bool| {
+                let namespace_name = match namespace {
+                    TypeNS => "type",
+                    ValueNS => "value",
+                };
 
-        match value_result {
-            BoundResult(ref target_module, ref name_bindings) => {
-                debug!("(resolving single import) found value target: {}",
-                       { name_bindings.value_def.borrow().clone().unwrap().def });
-                self.check_for_conflicting_import(
-                    &import_resolution.value_target,
-                    directive.span,
-                    target,
-                    ValueNS);
-
-                self.check_that_import_is_importable(
-                    &**name_bindings,
-                    directive.span,
-                    target,
-                    ValueNS);
-
-                import_resolution.value_target =
-                    Some(Target::new(target_module.clone(),
-                                     name_bindings.clone(),
-                                     directive.shadowable));
-                import_resolution.value_id = directive.id;
-                import_resolution.is_public = directive.is_public;
-                value_used_public = name_bindings.defined_in_public_namespace(ValueNS);
-            }
-            UnboundResult => { /* Continue. */ }
-            UnknownResult => {
-                panic!("value result should be known at this point");
-            }
-        }
-        match type_result {
-            BoundResult(ref target_module, ref name_bindings) => {
-                debug!("(resolving single import) found type target: {}",
-                       { name_bindings.type_def.borrow().clone().unwrap().type_def });
-                self.check_for_conflicting_import(
-                    &import_resolution.type_target,
-                    directive.span,
-                    target,
-                    TypeNS);
-
-                self.check_that_import_is_importable(
-                    &**name_bindings,
-                    directive.span,
-                    target,
-                    TypeNS);
-
-                import_resolution.type_target =
-                    Some(Target::new(target_module.clone(),
-                                     name_bindings.clone(),
-                                     directive.shadowable));
-                import_resolution.type_id = directive.id;
-                import_resolution.is_public = directive.is_public;
-                type_used_public = name_bindings.defined_in_public_namespace(TypeNS);
-            }
-            UnboundResult => { /* Continue. */ }
-            UnknownResult => {
-                panic!("type result should be known at this point");
-            }
+                match *result {
+                    BoundResult(ref target_module, ref name_bindings) => {
+                        debug!("(resolving single import) found {} target: {}",
+                               namespace_name,
+                               name_bindings.def_for_namespace(namespace));
+                        self.check_for_conflicting_import(
+                            &import_resolution.target_for_namespace(namespace),
+                            directive.span,
+                            target,
+                            namespace);
+
+                        self.check_that_import_is_importable(
+                            &**name_bindings,
+                            directive.span,
+                            target,
+                            namespace);
+
+                        let target = Some(Target::new(target_module.clone(),
+                                                      name_bindings.clone(),
+                                                      directive.shadowable));
+                        import_resolution.set_target_and_id(namespace, target, directive.id);
+                        import_resolution.is_public = directive.is_public;
+                        *used_public = name_bindings.defined_in_public_namespace(namespace);
+                    }
+                    UnboundResult => { /* Continue. */ }
+                    UnknownResult => {
+                        panic!("{} result should be known at this point", namespace_name);
+                    }
+                }
+            };
+            check_and_write_import(ValueNS, &value_result, &mut value_used_public);
+            check_and_write_import(TypeNS, &type_result, &mut type_used_public);
         }
 
         self.check_for_conflicts_between_imports_and_items(
@@ -2825,7 +2815,8 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
 
     // Resolves a glob import. Note that this function cannot fail; it either
     // succeeds or bails out (as importing * from an empty module or a module
-    // that exports nothing is valid).
+    // that exports nothing is valid). containing_module is the module we are
+    // actually importing, i.e., `foo` in `use foo::*`.
     fn resolve_glob_import(&mut self,
                            module_: &Module,
                            containing_module: Rc<Module>,
@@ -2851,12 +2842,11 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
         assert_eq!(containing_module.glob_count.get(), 0);
 
         // Add all resolved imports from the containing module.
-        let import_resolutions = containing_module.import_resolutions
-                                                  .borrow();
+        let import_resolutions = containing_module.import_resolutions.borrow();
         for (ident, target_import_resolution) in import_resolutions.iter() {
             debug!("(resolving glob import) writing module resolution \
                     {} into `{}`",
-                   target_import_resolution.type_target.is_none(),
+                   token::get_name(*ident),
                    self.module_to_string(module_));
 
             if !target_import_resolution.is_public {
@@ -2876,17 +2866,23 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
                             // Continue.
                         }
                         Some(ref value_target) => {
-                            dest_import_resolution.value_target =
-                                Some(value_target.clone());
+                            self.check_for_conflicting_import(&dest_import_resolution.value_target,
+                                                              import_directive.span,
+                                                              *ident,
+                                                              ValueNS);
+                            dest_import_resolution.value_target = Some(value_target.clone());
                         }
                     }
                     match target_import_resolution.type_target {
                         None => {
                             // Continue.
                         }
                         Some(ref type_target) => {
-                            dest_import_resolution.type_target =
-                                Some(type_target.clone());
+                            self.check_for_conflicting_import(&dest_import_resolution.type_target,
+                                                              import_directive.span,
+                                                              *ident,
+                                                              TypeNS);
+                            dest_import_resolution.type_target = Some(type_target.clone());
                         }
                     }
                     dest_import_resolution.is_public = is_public;
@@ -2908,8 +2904,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
         // Add all children from the containing module.
         self.populate_module_if_necessary(&containing_module);
 
-        for (&name, name_bindings) in containing_module.children
-                                                       .borrow().iter() {
+        for (&name, name_bindings) in containing_module.children.borrow().iter() {
             self.merge_import_resolution(module_,
                                          containing_module.clone(),
                                          import_directive,
@@ -2919,8 +2914,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
         }
 
         // Add external module children from the containing module.
-        for (&name, module) in containing_module.external_module_children
-                                                .borrow().iter() {
+        for (&name, module) in containing_module.external_module_children.borrow().iter() {
             let name_bindings =
                 Rc::new(Resolver::create_name_bindings_from_module(module.clone()));
             self.merge_import_resolution(module_,
@@ -2965,41 +2959,39 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
 
         debug!("(resolving glob import) writing resolution `{}` in `{}` \
                to `{}`",
-               token::get_name(name).get().to_string(),
+               token::get_name(name).get(),
                self.module_to_string(&*containing_module),
                self.module_to_string(module_));
 
         // Merge the child item into the import resolution.
-        if name_bindings.defined_in_namespace_with(ValueNS, IMPORTABLE | PUBLIC) {
-            debug!("(resolving glob import) ... for value target");
-            if dest_import_resolution.shadowable(ValueNS) == Shadowable::Never {
-                let msg = format!("a value named `{}` has already been imported \
-                                   in this module",
-                                  token::get_name(name).get());
-                self.session.span_err(import_directive.span, msg.as_slice());
-            } else {
-                dest_import_resolution.value_target =
-                    Some(Target::new(containing_module.clone(),
-                                     name_bindings.clone(),
-                                     import_directive.shadowable));
-                dest_import_resolution.value_id = id;
-            }
-        }
-        if name_bindings.defined_in_namespace_with(TypeNS, IMPORTABLE | PUBLIC) {
-            debug!("(resolving glob import) ... for type target");
-            if dest_import_resolution.shadowable(TypeNS) == Shadowable::Never {
-                let msg = format!("a type named `{}` has already been imported \
-                                   in this module",
-                                  token::get_name(name).get());
-                self.session.span_err(import_directive.span, msg.as_slice());
-            } else {
-                dest_import_resolution.type_target =
-                    Some(Target::new(containing_module,
-                                     name_bindings.clone(),
-                                     import_directive.shadowable));
-                dest_import_resolution.type_id = id;
-            }
+        {
+            let merge_child_item = |namespace| {
+                if name_bindings.defined_in_namespace_with(namespace, IMPORTABLE | PUBLIC) {
+                    let namespace_name = match namespace {
+                        TypeNS => "type",
+                        ValueNS => "value",
+                    };
+                    debug!("(resolving glob import) ... for {} target", namespace_name);
+                    if dest_import_resolution.shadowable(namespace) == Shadowable::Never {
+                        let msg = format!("a {} named `{}` has already been imported \
+                                           in this module",
+                                          namespace_name,
+                                          token::get_name(name).get());
+                        self.session.span_err(import_directive.span, msg.as_slice());
+                    } else {
+                        let target = Target::new(containing_module.clone(),
+                                                 name_bindings.clone(),
+                                                 import_directive.shadowable);
+                        dest_import_resolution.set_target_and_id(namespace,
+                                                                 Some(target),
+                                                                 id);
+                    }
+                }
+            };
+            merge_child_item(ValueNS);
+            merge_child_item(TypeNS);
         }
+
         dest_import_resolution.is_public = is_public;
 
         self.check_for_conflicts_between_imports_and_items(
@@ -3019,6 +3011,10 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
             return
         }
 
+        debug!("check_for_conflicting_import: {}; target exists: {}",
+               token::get_name(name).get(),
+               target.is_some());
+
         match *target {
             Some(ref target) if target.shadowable != Shadowable::Always => {
                 let msg = format!("a {} named `{}` has already been imported \

src/librustc_trans/trans/base.rs

@@ -1554,8 +1554,7 @@ pub fn arg_kind<'a, 'tcx>(cx: &FunctionContext<'a, 'tcx>, t: Ty<'tcx>)
 }
 
 // work around bizarre resolve errors
-pub type RvalueDatum<'tcx> = datum::Datum<'tcx, datum::Rvalue>;
-pub type LvalueDatum<'tcx> = datum::Datum<'tcx, datum::Lvalue>;
+type RvalueDatum<'tcx> = datum::Datum<'tcx, datum::Rvalue>;
 
 // create_datums_for_fn_args: creates rvalue datums for each of the
 // incoming function arguments. These will later be stored into

src/librustc_trans/trans/common.rs

@@ -190,8 +190,8 @@ pub fn validate_substs(substs: &Substs) {
 }
 
 // work around bizarre resolve errors
-pub type RvalueDatum<'tcx> = datum::Datum<'tcx, datum::Rvalue>;
-pub type LvalueDatum<'tcx> = datum::Datum<'tcx, datum::Lvalue>;
+type RvalueDatum<'tcx> = datum::Datum<'tcx, datum::Rvalue>;
+type LvalueDatum<'tcx> = datum::Datum<'tcx, datum::Lvalue>;
 
 // Function context.  Every LLVM function we create will have one of
 // these.

src/libstd/c_str.rs

@@ -537,7 +537,8 @@ pub unsafe fn from_c_multistring<F>(buf: *const libc::c_char,
 #[cfg(test)]
 mod tests {
     use super::*;
-    use prelude::*;
+    use prelude::{spawn, Some, None, Option, FnOnce, ToString, CloneSliceExt};
+    use prelude::{Clone, RawPtr, Iterator, SliceExt, StrExt};
     use ptr;
     use thread::Thread;
     use libc;