Little bit of refactoring in resolve

Author: nrc

src/librustc_resolve/lib.rs

@@ -459,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.
@@ -2705,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(
@@ -2818,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>,
@@ -2844,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 {
@@ -2869,17 +2866,15 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
                             // Continue.
                         }
                         Some(ref value_target) => {
-                            dest_import_resolution.value_target =
-                                Some(value_target.clone());
+                            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());
+                            dest_import_resolution.type_target = Some(type_target.clone());
                         }
                     }
                     dest_import_resolution.is_public = is_public;
@@ -2901,8 +2896,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,
@@ -2912,8 +2906,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_,
@@ -2958,41 +2951,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(
@@ -3012,6 +3003,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 \