resolve: Partially unify bindings from macro_rules and from other items

Author: petrochenkov

src/librustc_resolve/lib.rs

@@ -1166,7 +1166,6 @@ struct UseError<'a> {
 struct AmbiguityError<'a> {
     span: Span,
     name: Name,
-    lexical: bool,
     b1: &'a NameBinding<'a>,
     b2: &'a NameBinding<'a>,
 }
@@ -1814,7 +1813,7 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> {
             NameBindingKind::Import { .. } => false,
             NameBindingKind::Ambiguity { b1, b2 } => {
                 self.ambiguity_errors.push(AmbiguityError {
-                    span, name: ident.name, lexical: false, b1, b2,
+                    span, name: ident.name, b1, b2,
                 });
                 true
             }
@@ -4499,35 +4498,32 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> {
         vis.is_accessible_from(module.normal_ancestor_id, self)
     }
 
-    fn report_ambiguity_error(
-        &self, name: Name, span: Span, _lexical: bool,
-        def1: Def, is_import1: bool, is_glob1: bool, from_expansion1: bool, span1: Span,
-        def2: Def, is_import2: bool, _is_glob2: bool, _from_expansion2: bool, span2: Span,
-    ) {
+    fn report_ambiguity_error(&self, name: Name, span: Span, b1: &NameBinding, b2: &NameBinding) {
         let participle = |is_import: bool| if is_import { "imported" } else { "defined" };
-        let msg1 = format!("`{}` could refer to the name {} here", name, participle(is_import1));
+        let msg1 =
+            format!("`{}` could refer to the name {} here", name, participle(b1.is_import()));
         let msg2 =
-            format!("`{}` could also refer to the name {} here", name, participle(is_import2));
-        let note = if from_expansion1 {
-            Some(if let Def::Macro(..) = def1 {
+            format!("`{}` could also refer to the name {} here", name, participle(b2.is_import()));
+        let note = if b1.expansion != Mark::root() {
+            Some(if let Def::Macro(..) = b1.def() {
                 format!("macro-expanded {} do not shadow",
-                        if is_import1 { "macro imports" } else { "macros" })
+                        if b1.is_import() { "macro imports" } else { "macros" })
             } else {
                 format!("macro-expanded {} do not shadow when used in a macro invocation path",
-                        if is_import1 { "imports" } else { "items" })
+                        if b1.is_import() { "imports" } else { "items" })
             })
-        } else if is_glob1 {
+        } else if b1.is_glob_import() {
             Some(format!("consider adding an explicit import of `{}` to disambiguate", name))
         } else {
             None
         };
 
         let mut err = struct_span_err!(self.session, span, E0659, "`{}` is ambiguous", name);
-        err.span_note(span1, &msg1);
-        match def2 {
-            Def::Macro(..) if span2.is_dummy() =>
+        err.span_note(b1.span, &msg1);
+        match b2.def() {
+            Def::Macro(..) if b2.span.is_dummy() =>
                 err.note(&format!("`{}` is also a builtin macro", name)),
-            _ => err.span_note(span2, &msg2),
+            _ => err.span_note(b2.span, &msg2),
         };
         if let Some(note) = note {
             err.note(&note);
@@ -4552,15 +4548,9 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> {
             );
         }
 
-        for &AmbiguityError { span, name, b1, b2, lexical } in &self.ambiguity_errors {
+        for &AmbiguityError { span, name, b1, b2 } in &self.ambiguity_errors {
             if reported_spans.insert(span) {
-                self.report_ambiguity_error(
-                    name, span, lexical,
-                    b1.def(), b1.is_import(), b1.is_glob_import(),
-                    b1.expansion != Mark::root(), b1.span,
-                    b2.def(), b2.is_import(), b2.is_glob_import(),
-                    b2.expansion != Mark::root(), b2.span,
-                );
+                self.report_ambiguity_error(name, span, b1, b2);
             }
         }
 
@@ -4584,9 +4574,9 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> {
         let mut reported_errors = FxHashSet();
         for binding in replace(&mut self.disallowed_shadowing, Vec::new()) {
             if self.resolve_legacy_scope(&binding.parent, binding.ident, false).is_some() &&
-               reported_errors.insert((binding.ident, binding.span)) {
+               reported_errors.insert((binding.ident, binding.binding.span)) {
                 let msg = format!("`{}` is already in scope", binding.ident);
-                self.session.struct_span_err(binding.span, &msg)
+                self.session.struct_span_err(binding.binding.span, &msg)
                     .note("macro-expanded `macro_rules!`s may not shadow \
                            existing macros (see RFC 1560)")
                     .emit();

src/librustc_resolve/macros.rs

@@ -78,17 +78,12 @@ pub enum LegacyScope<'a> {
     Binding(&'a LegacyBinding<'a>),
 }
 
+// Binding produced by a `macro_rules` item.
+// Not modularized, can shadow previous legacy bindings, etc.
 pub struct LegacyBinding<'a> {
+    pub binding: &'a NameBinding<'a>,
     pub parent: Cell<LegacyScope<'a>>,
     pub ident: Ident,
-    def_id: DefId,
-    pub span: Span,
-}
-
-impl<'a> LegacyBinding<'a> {
-    fn def(&self) -> Def {
-        Def::Macro(self.def_id, MacroKind::Bang)
-    }
 }
 
 pub struct ProcMacError {
@@ -722,7 +717,6 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
                                 name: ident.name,
                                 b1: previous_result.0,
                                 b2: result.0,
-                                lexical: true,
                             });
                             return Ok(previous_result);
                         }
@@ -771,7 +765,7 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
                                   mut scope: &'a Cell<LegacyScope<'a>>,
                                   ident: Ident,
                                   record_used: bool)
-                                  -> Option<(&'a LegacyBinding<'a>, FromExpansion)> {
+                                  -> Option<(&'a NameBinding<'a>, FromExpansion)> {
         let ident = ident.modern();
         let mut relative_depth: u32 = 0;
         loop {
@@ -798,7 +792,7 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
                         if record_used && relative_depth > 0 {
                             self.disallowed_shadowing.push(potential_binding);
                         }
-                        return Some((potential_binding, FromExpansion(relative_depth > 0)));
+                        return Some((potential_binding.binding, FromExpansion(relative_depth > 0)));
                     }
                     scope = &potential_binding.parent;
                 }
@@ -858,18 +852,10 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
                     self.suggest_macro_name(&ident.as_str(), kind, &mut err, span);
                     err.emit();
                 },
-                (Some((legacy_binding, FromExpansion(from_expansion))),
-                 Ok((binding, FromPrelude(false)))) |
-                (Some((legacy_binding, FromExpansion(from_expansion @ true))),
-                 Ok((binding, FromPrelude(true)))) => {
-                    if legacy_binding.def() != binding.def_ignoring_ambiguity() {
-                        self.report_ambiguity_error(
-                            ident.name, span, true,
-                            legacy_binding.def(), false, false,
-                            from_expansion, legacy_binding.span,
-                            binding.def(), binding.is_import(), binding.is_glob_import(),
-                            binding.expansion != Mark::root(), binding.span,
-                        );
+                (Some((legacy_binding, FromExpansion(_))), Ok((binding, FromPrelude(false)))) |
+                (Some((legacy_binding, FromExpansion(true))), Ok((binding, FromPrelude(true)))) => {
+                    if legacy_binding.def_ignoring_ambiguity() != binding.def_ignoring_ambiguity() {
+                        self.report_ambiguity_error(ident.name, span, legacy_binding, binding);
                     }
                 },
                 // OK, non-macro-expanded legacy wins over macro prelude even if defs are different
@@ -990,10 +976,12 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
         if def.legacy {
             let ident = ident.modern();
             self.macro_names.insert(ident);
-            *legacy_scope = LegacyScope::Binding(self.arenas.alloc_legacy_binding(LegacyBinding {
-                parent: Cell::new(*legacy_scope), ident: ident, def_id: def_id, span: item.span,
-            }));
             let def = Def::Macro(def_id, MacroKind::Bang);
+            let vis = ty::Visibility::Invisible; // Doesn't matter for legacy bindings
+            let binding = (def, vis, item.span, expansion).to_name_binding(self.arenas);
+            *legacy_scope = LegacyScope::Binding(self.arenas.alloc_legacy_binding(
+                LegacyBinding { parent: Cell::new(*legacy_scope), binding, ident }
+            ));
             self.all_macros.insert(ident.name, def);
             if attr::contains_name(&item.attrs, "macro_export") {
                 let module = self.graph_root;

src/librustc_resolve/resolve_imports.rs

@@ -251,7 +251,6 @@ impl<'a, 'crateloader> Resolver<'a, 'crateloader> {
                         self.ambiguity_errors.push(AmbiguityError {
                             span: path_span,
                             name,
-                            lexical: false,
                             b1: binding,
                             b2: shadowed_glob,
                         });