Don't use gensym_if_underscore to resolve _ bindings

Instead add a disambiguator to the keys used for distinguishing
resolutions.

Author: matthewjasper

src/librustc_resolve/build_reduced_graph.rs

@@ -93,7 +93,8 @@ impl<'a> Resolver<'a> {
         where T: ToNameBinding<'a>,
     {
         let binding = def.to_name_binding(self.arenas);
-        if let Err(old_binding) = self.try_define(parent, ident, ns, binding) {
+        let key = self.new_key(ident, ns);
+        if let Err(old_binding) = self.try_define(parent, key, binding) {
             self.report_conflict(parent, ident, ns, old_binding, &binding);
         }
     }
@@ -349,9 +350,12 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> {
 
         self.r.indeterminate_imports.push(directive);
         match directive.subclass {
+            // Don't add unresolved underscore imports to modules
+            SingleImport { target: Ident { name: kw::Underscore, .. }, .. } => {}
             SingleImport { target, type_ns_only, .. } => {
                 self.r.per_ns(|this, ns| if !type_ns_only || ns == TypeNS {
-                    let mut resolution = this.resolution(current_module, target, ns).borrow_mut();
+                    let key = this.new_key(target, ns);
+                    let mut resolution = this.resolution(current_module, key).borrow_mut();
                     resolution.add_single_import(directive);
                 });
             }
@@ -407,7 +411,7 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> {
         };
         match use_tree.kind {
             ast::UseTreeKind::Simple(rename, ..) => {
-                let mut ident = use_tree.ident().gensym_if_underscore();
+                let mut ident = use_tree.ident();
                 let mut module_path = prefix;
                 let mut source = module_path.pop().unwrap();
                 let mut type_ns_only = false;
@@ -585,7 +589,7 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> {
         let parent_scope = &self.parent_scope;
         let parent = parent_scope.module;
         let expansion = parent_scope.expansion;
-        let ident = item.ident.gensym_if_underscore();
+        let ident = item.ident;
         let sp = item.span;
         let vis = self.resolve_visibility(&item.vis);
 
@@ -850,10 +854,6 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> {
     fn build_reduced_graph_for_external_crate_res(&mut self, child: Export<NodeId>) {
         let parent = self.parent_scope.module;
         let Export { ident, res, vis, span } = child;
-        // FIXME: We shouldn't create the gensym here, it should come from metadata,
-        // but metadata cannot encode gensyms currently, so we create it here.
-        // This is only a guess, two equivalent idents may incorrectly get different gensyms here.
-        let ident = ident.gensym_if_underscore();
         let expansion = ExpnId::root(); // FIXME(jseyfried) intercrate hygiene
         // Record primary definitions.
         match res {

src/librustc_resolve/diagnostics.rs

@@ -80,11 +80,11 @@ impl<'a> Resolver<'a> {
         names: &mut Vec<TypoSuggestion>,
         filter_fn: &impl Fn(Res) -> bool,
     ) {
-        for (&(ident, _), resolution) in self.resolutions(module).borrow().iter() {
+        for (key, resolution) in self.resolutions(module).borrow().iter() {
             if let Some(binding) = resolution.borrow().binding {
                 let res = binding.res();
                 if filter_fn(res) {
-                    names.push(TypoSuggestion::from_res(ident.name, res));
+                    names.push(TypoSuggestion::from_res(key.ident.name, res));
                 }
             }
         }
@@ -849,7 +849,7 @@ impl<'a, 'b> ImportResolver<'a, 'b> {
         }
 
         let resolutions = self.r.resolutions(crate_module).borrow();
-        let resolution = resolutions.get(&(ident, MacroNS))?;
+        let resolution = resolutions.get(&self.r.new_key(ident, MacroNS))?;
         let binding = resolution.borrow().binding()?;
         if let Res::Def(DefKind::Macro(MacroKind::Bang), _) = binding.res() {
             let module_name = crate_module.kind.name().unwrap();

src/librustc_resolve/lib.rs

@@ -431,7 +431,22 @@ impl ModuleKind {
     }
 }
 
-type Resolutions<'a> = RefCell<FxIndexMap<(Ident, Namespace), &'a RefCell<NameResolution<'a>>>>;
+/// A key that identifies a binding in a given `Module`.
+///
+/// Multiple bindings in the same module can have the same key (in a valid
+/// program) if all but one of them come from glob imports.
+#[derive(Copy, Clone, PartialEq, Eq, Hash)]
+struct BindingKey {
+    /// The identifier for the binding, aways the `modern` version of the
+    /// identifier.
+    ident: Ident,
+    ns: Namespace,
+    /// 0 if ident is not `_`, otherwise a value that's unique to the specific
+    /// `_` in the expanded AST that introduced this binding.
+    disambiguator: u32,
+}
+
+type Resolutions<'a> = RefCell<FxIndexMap<BindingKey, &'a RefCell<NameResolution<'a>>>>;
 
 /// One node in the tree of modules.
 pub struct ModuleData<'a> {
@@ -491,8 +506,8 @@ impl<'a> ModuleData<'a> {
     fn for_each_child<R, F>(&'a self, resolver: &mut R, mut f: F)
         where R: AsMut<Resolver<'a>>, F: FnMut(&mut R, Ident, Namespace, &'a NameBinding<'a>)
     {
-        for (&(ident, ns), name_resolution) in resolver.as_mut().resolutions(self).borrow().iter() {
-            name_resolution.borrow().binding.map(|binding| f(resolver, ident, ns, binding));
+        for (key, name_resolution) in resolver.as_mut().resolutions(self).borrow().iter() {
+            name_resolution.borrow().binding.map(|binding| f(resolver, key.ident, key.ns, binding));
         }
     }
 
@@ -879,6 +894,7 @@ pub struct Resolver<'a> {
     module_map: FxHashMap<DefId, Module<'a>>,
     extern_module_map: FxHashMap<(DefId, bool /* MacrosOnly? */), Module<'a>>,
     binding_parent_modules: FxHashMap<PtrKey<'a, NameBinding<'a>>, Module<'a>>,
+    underscore_disambiguator: u32,
 
     /// Maps glob imports to the names of items actually imported.
     pub glob_map: GlobMap,
@@ -1156,6 +1172,7 @@ impl<'a> Resolver<'a> {
             label_res_map: Default::default(),
             export_map: FxHashMap::default(),
             trait_map: Default::default(),
+            underscore_disambiguator: 0,
             empty_module,
             module_map,
             block_map: Default::default(),
@@ -1280,6 +1297,17 @@ impl<'a> Resolver<'a> {
         self.arenas.alloc_module(module)
     }
 
+    fn new_key(&mut self, ident: Ident, ns: Namespace) -> BindingKey {
+        let ident = ident.modern();
+        let disambiguator = if ident.name == kw::Underscore {
+            self.underscore_disambiguator += 1;
+            self.underscore_disambiguator
+        } else {
+            0
+        };
+        BindingKey { ident, ns, disambiguator }
+    }
+
     fn resolutions(&mut self, module: Module<'a>) -> &'a Resolutions<'a> {
         if module.populate_on_access.get() {
             module.populate_on_access.set(false);
@@ -1288,9 +1316,9 @@ impl<'a> Resolver<'a> {
         &module.lazy_resolutions
     }
 
-    fn resolution(&mut self, module: Module<'a>, ident: Ident, ns: Namespace)
+    fn resolution(&mut self, module: Module<'a>, key: BindingKey)
                   -> &'a RefCell<NameResolution<'a>> {
-        *self.resolutions(module).borrow_mut().entry((ident.modern(), ns))
+        *self.resolutions(module).borrow_mut().entry(key)
                .or_insert_with(|| self.arenas.alloc_name_resolution())
     }