Auto merge of #53913 - petrochenkov:biattr4, r=<try>

resolve: Future proof resolutions for potentially built-in attributes

Based on #53778

This is not full "pass all attributes through name resolution", but a more conservative solution.
If built-in attribute is ambiguous with any other macro in scope, then an error is reported.
TODO: Explain what complications arise with the full solution.

Author: bors

src/librustc_resolve/build_reduced_graph.rs

@@ -39,6 +39,7 @@ use syntax::ext::base::{MacroKind, SyntaxExtension};
 use syntax::ext::base::Determinacy::Undetermined;
 use syntax::ext::hygiene::Mark;
 use syntax::ext::tt::macro_rules;
+use syntax::feature_gate::is_builtin_attr;
 use syntax::parse::token::{self, Token};
 use syntax::std_inject::injected_crate_name;
 use syntax::symbol::keywords;
@@ -933,7 +934,7 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
 
 pub struct BuildReducedGraphVisitor<'a, 'b: 'a, 'c: 'b> {
     pub resolver: &'a mut Resolver<'b, 'c>,
-    pub legacy_scope: LegacyScope<'b>,
+    pub current_legacy_scope: LegacyScope<'b>,
     pub expansion: Mark,
 }
 
@@ -943,7 +944,8 @@ impl<'a, 'b, 'cl> BuildReducedGraphVisitor<'a, 'b, 'cl> {
         self.resolver.current_module.unresolved_invocations.borrow_mut().insert(mark);
         let invocation = self.resolver.invocations[&mark];
         invocation.module.set(self.resolver.current_module);
-        invocation.legacy_scope.set(self.legacy_scope);
+        invocation.parent_legacy_scope.set(self.current_legacy_scope);
+        invocation.output_legacy_scope.set(self.current_legacy_scope);
         invocation
     }
 }
@@ -969,29 +971,30 @@ impl<'a, 'b, 'cl> Visitor<'a> for BuildReducedGraphVisitor<'a, 'b, 'cl> {
     fn visit_item(&mut self, item: &'a Item) {
         let macro_use = match item.node {
             ItemKind::MacroDef(..) => {
-                self.resolver.define_macro(item, self.expansion, &mut self.legacy_scope);
+                self.resolver.define_macro(item, self.expansion, &mut self.current_legacy_scope);
                 return
             }
             ItemKind::Mac(..) => {
-                self.legacy_scope = LegacyScope::Expansion(self.visit_invoc(item.id));
+                self.current_legacy_scope = LegacyScope::Invocation(self.visit_invoc(item.id));
                 return
             }
             ItemKind::Mod(..) => self.resolver.contains_macro_use(&item.attrs),
             _ => false,
         };
 
-        let (parent, legacy_scope) = (self.resolver.current_module, self.legacy_scope);
+        let orig_current_module = self.resolver.current_module;
+        let orig_current_legacy_scope = self.current_legacy_scope;
         self.resolver.build_reduced_graph_for_item(item, self.expansion);
         visit::walk_item(self, item);
-        self.resolver.current_module = parent;
+        self.resolver.current_module = orig_current_module;
         if !macro_use {
-            self.legacy_scope = legacy_scope;
+            self.current_legacy_scope = orig_current_legacy_scope;
         }
     }
 
     fn visit_stmt(&mut self, stmt: &'a ast::Stmt) {
         if let ast::StmtKind::Mac(..) = stmt.node {
-            self.legacy_scope = LegacyScope::Expansion(self.visit_invoc(stmt.id));
+            self.current_legacy_scope = LegacyScope::Invocation(self.visit_invoc(stmt.id));
         } else {
             visit::walk_stmt(self, stmt);
         }
@@ -1008,11 +1011,12 @@ impl<'a, 'b, 'cl> Visitor<'a> for BuildReducedGraphVisitor<'a, 'b, 'cl> {
     }
 
     fn visit_block(&mut self, block: &'a Block) {
-        let (parent, legacy_scope) = (self.resolver.current_module, self.legacy_scope);
+        let orig_current_module = self.resolver.current_module;
+        let orig_current_legacy_scope = self.current_legacy_scope;
         self.resolver.build_reduced_graph_for_block(block, self.expansion);
         visit::walk_block(self, block);
-        self.resolver.current_module = parent;
-        self.legacy_scope = legacy_scope;
+        self.resolver.current_module = orig_current_module;
+        self.current_legacy_scope = orig_current_legacy_scope;
     }
 
     fn visit_trait_item(&mut self, item: &'a TraitItem) {
@@ -1057,4 +1061,13 @@ impl<'a, 'b, 'cl> Visitor<'a> for BuildReducedGraphVisitor<'a, 'b, 'cl> {
             }
         }
     }
+
+    fn visit_attribute(&mut self, attr: &'a ast::Attribute) {
+        if !attr.is_sugared_doc && is_builtin_attr(attr) {
+            self.resolver.current_module.builtin_attrs.borrow_mut().push((
+                attr.path.segments[0].ident, self.expansion, self.current_legacy_scope
+            ));
+        }
+        visit::walk_attribute(self, attr);
+    }
 }

src/librustc_resolve/lib.rs

@@ -81,7 +81,7 @@ use std::mem::replace;
 use rustc_data_structures::sync::Lrc;
 
 use resolve_imports::{ImportDirective, ImportDirectiveSubclass, NameResolution, ImportResolver};
-use macros::{InvocationData, LegacyBinding};
+use macros::{InvocationData, LegacyBinding, LegacyScope};
 
 // NB: This module needs to be declared first so diagnostics are
 // registered before they are used.
@@ -1010,8 +1010,9 @@ pub struct ModuleData<'a> {
     normal_ancestor_id: DefId,
 
     resolutions: RefCell<FxHashMap<(Ident, Namespace), &'a RefCell<NameResolution<'a>>>>,
-    legacy_macro_resolutions: RefCell<Vec<(Mark, Ident, MacroKind, Option<Def>)>>,
+    legacy_macro_resolutions: RefCell<Vec<(Ident, MacroKind, Mark, LegacyScope<'a>, Option<Def>)>>,
     macro_resolutions: RefCell<Vec<(Box<[Ident]>, Span)>>,
+    builtin_attrs: RefCell<Vec<(Ident, Mark, LegacyScope<'a>)>>,
 
     // Macro invocations that can expand into items in this module.
     unresolved_invocations: RefCell<FxHashSet<Mark>>,
@@ -1050,6 +1051,7 @@ impl<'a> ModuleData<'a> {
             resolutions: RefCell::new(FxHashMap()),
             legacy_macro_resolutions: RefCell::new(Vec::new()),
             macro_resolutions: RefCell::new(Vec::new()),
+            builtin_attrs: RefCell::new(Vec::new()),
             unresolved_invocations: RefCell::new(FxHashSet()),
             no_implicit_prelude: false,
             glob_importers: RefCell::new(Vec::new()),
@@ -1166,7 +1168,6 @@ struct UseError<'a> {
 struct AmbiguityError<'a> {
     span: Span,
     name: Name,
-    lexical: bool,
     b1: &'a NameBinding<'a>,
     b2: &'a NameBinding<'a>,
 }
@@ -1270,6 +1271,23 @@ impl<'a> NameBinding<'a> {
     fn descr(&self) -> &'static str {
         if self.is_extern_crate() { "extern crate" } else { self.def().kind_name() }
     }
+
+    // Suppose that we resolved macro invocation with `invoc_parent_expansion` to binding `binding`
+    // at some expansion round `max(invoc, binding)` when they both emerged from macros.
+    // Then this function returns `true` if `self` may emerge from a macro *after* that
+    // in some later round and screw up our previously found resolution.
+    fn may_appear_after(&self, invoc_parent_expansion: Mark, binding: &NameBinding) -> bool {
+        // self > max(invoc, binding) => !(self <= invoc || self <= binding)
+        // Expansions are partially ordered, so "may appear after" is an inversion of
+        // "certainly appears before or simultaneously" and includes unordered cases.
+        let self_parent_expansion = self.expansion;
+        let other_parent_expansion = binding.expansion;
+        let certainly_before_other_or_simultaneously =
+            other_parent_expansion.is_descendant_of(self_parent_expansion);
+        let certainly_before_invoc_or_simultaneously =
+            invoc_parent_expansion.is_descendant_of(self_parent_expansion);
+        !(certainly_before_other_or_simultaneously || certainly_before_invoc_or_simultaneously)
+    }
 }
 
 /// Interns the names of the primitive types.
@@ -1403,8 +1421,6 @@ pub struct Resolver<'a, 'b: 'a> {
     proc_mac_errors: Vec<macros::ProcMacError>,
     /// crate-local macro expanded `macro_export` referred to by a module-relative path
     macro_expanded_macro_export_errors: BTreeSet<(Span, Span)>,
-    /// macro-expanded `macro_rules` shadowing existing macros
-    disallowed_shadowing: Vec<&'a LegacyBinding<'a>>,
 
     arenas: &'a ResolverArenas<'a>,
     dummy_binding: &'a NameBinding<'a>,
@@ -1715,7 +1731,6 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> {
             ambiguity_errors: Vec::new(),
             use_injections: Vec::new(),
             proc_mac_errors: Vec::new(),
-            disallowed_shadowing: Vec::new(),
             macro_expanded_macro_export_errors: BTreeSet::new(),
 
             arenas,
@@ -1814,7 +1829,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
             }
@@ -3468,6 +3483,20 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> {
         record_used: bool,
         path_span: Span,
         crate_lint: CrateLint,
+    ) -> PathResult<'a> {
+        self.resolve_path_with_parent_expansion(base_module, path, opt_ns, Mark::root(),
+                                                record_used, path_span, crate_lint)
+    }
+
+    fn resolve_path_with_parent_expansion(
+        &mut self,
+        base_module: Option<ModuleOrUniformRoot<'a>>,
+        path: &[Ident],
+        opt_ns: Option<Namespace>, // `None` indicates a module path
+        parent_expansion: Mark,
+        record_used: bool,
+        path_span: Span,
+        crate_lint: CrateLint,
     ) -> PathResult<'a> {
         let mut module = base_module;
         let mut allow_super = true;
@@ -3557,8 +3586,9 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> {
                 self.resolve_ident_in_module(module, ident, ns, record_used, path_span)
             } else if opt_ns == Some(MacroNS) {
                 assert!(ns == TypeNS);
-                self.resolve_lexical_macro_path_segment(ident, ns, record_used, record_used,
-                                                        false, path_span).map(|(b, _)| b)
+                self.resolve_lexical_macro_path_segment(ident, ns, parent_expansion, record_used,
+                                                        record_used, false, path_span)
+                                                        .map(|(binding, _)| binding)
             } else {
                 let record_used_id =
                     if record_used { crate_lint.node_id().or(Some(CRATE_NODE_ID)) } else { None };
@@ -4499,35 +4529,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);
@@ -4536,7 +4563,6 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> {
     }
 
     fn report_errors(&mut self, krate: &Crate) {
-        self.report_shadowing_errors();
         self.report_with_use_injections(krate);
         self.report_proc_macro_import(krate);
         let mut reported_spans = FxHashSet();
@@ -4552,15 +4578,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);
             }
         }
 
@@ -4580,20 +4600,6 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> {
         }
     }
 
-    fn report_shadowing_errors(&mut self) {
-        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)) {
-                let msg = format!("`{}` is already in scope", binding.ident);
-                self.session.struct_span_err(binding.span, &msg)
-                    .note("macro-expanded `macro_rules!`s may not shadow \
-                           existing macros (see RFC 1560)")
-                    .emit();
-            }
-        }
-    }
-
     fn report_conflict<'b>(&mut self,
                        parent: Module,
                        ident: Ident,