Rollup merge of #37084 - jseyfried:cleanup_expanded_macro_use_scopes, r=nrc

macros: clean up scopes of expanded `#[macro_use]` imports

This PR changes the scope of macro-expanded `#[macro_use]` imports to match that of unexpanded `#[macro_use]` imports. For example, this would be allowed:
```rust
example!();
macro_rules! m { () => { #[macro_use(example)] extern crate example_crate; } }
m!();
```

This PR also enforces the full shadowing restrictions from RFC 1560 on `#[macro_use]` imports (currently, we only enforce the weakened restrictions from #36767).

This is a [breaking-change], but I believe it is highly unlikely to cause breakage in practice.
r? @nrc

Author: alexcrichton

src/librustc/hir/lowering.rs

@@ -94,13 +94,7 @@ pub fn lower_crate(sess: &Session,
     let _ignore = sess.dep_graph.in_ignore();
 
     LoweringContext {
-        crate_root: if std_inject::no_core(krate) {
-            None
-        } else if std_inject::no_std(krate) {
-            Some("core")
-        } else {
-            Some("std")
-        },
+        crate_root: std_inject::injected_crate_name(krate),
         sess: sess,
         parent_def: None,
         resolver: resolver,

src/librustc_driver/driver.rs

@@ -649,7 +649,7 @@ pub fn phase_2_configure_and_expand<'a, F>(sess: &Session,
     let resolver_arenas = Resolver::arenas();
     let mut resolver =
         Resolver::new(sess, &krate, make_glob_map, &mut crate_loader, &resolver_arenas);
-    syntax_ext::register_builtins(&mut resolver, sess.features.borrow().quote);
+    syntax_ext::register_builtins(&mut resolver, syntax_exts, sess.features.borrow().quote);
 
     krate = time(time_passes, "expansion", || {
         // Windows dlls do not have rpaths, so they don't know how to find their
@@ -686,11 +686,17 @@ pub fn phase_2_configure_and_expand<'a, F>(sess: &Session,
             ..syntax::ext::expand::ExpansionConfig::default(crate_name.to_string())
         };
         let mut ecx = ExtCtxt::new(&sess.parse_sess, krate.config.clone(), cfg, &mut resolver);
-        let ret = syntax::ext::expand::expand_crate(&mut ecx, syntax_exts, krate);
+        let err_count = ecx.parse_sess.span_diagnostic.err_count();
+
+        let krate = ecx.monotonic_expander().expand_crate(krate);
+
+        if ecx.parse_sess.span_diagnostic.err_count() - ecx.resolve_err_count > err_count {
+            ecx.parse_sess.span_diagnostic.abort_if_errors();
+        }
         if cfg!(windows) {
             env::set_var("PATH", &old_path);
         }
-        ret
+        krate
     });
 
     krate.exported_macros = mem::replace(&mut resolver.exported_macros, Vec::new());

src/librustc_resolve/build_reduced_graph.rs

@@ -13,7 +13,7 @@
 //! Here we build the "reduced graph": the graph of the module tree without
 //! any imports resolved.
 
-use macros;
+use macros::{InvocationData, LegacyScope};
 use resolve_imports::ImportDirectiveSubclass::{self, GlobImport};
 use {Module, ModuleS, ModuleKind};
 use Namespace::{self, TypeNS, ValueNS};
@@ -200,16 +200,16 @@ impl<'b> Resolver<'b> {
                         LoadedMacroKind::Def(mut def) => {
                             let name = def.ident.name;
                             if def.use_locally {
-                                let ext = macro_rules::compile(&self.session.parse_sess, &def);
-                                let shadowing =
-                                    self.resolve_macro_name(Mark::root(), name, false).is_some();
-                                self.expansion_data[&Mark::root()].module.macros.borrow_mut()
-                                    .insert(name, macros::NameBinding {
-                                        ext: Rc::new(ext),
-                                        expansion: expansion,
-                                        shadowing: shadowing,
-                                        span: loaded_macro.import_site,
-                                    });
+                                let ext =
+                                    Rc::new(macro_rules::compile(&self.session.parse_sess, &def));
+                                if self.builtin_macros.insert(name, ext).is_some() &&
+                                   expansion != Mark::root() {
+                                    let msg = format!("`{}` is already in scope", name);
+                                    self.session.struct_span_err(loaded_macro.import_site, &msg)
+                                        .note("macro-expanded `#[macro_use]`s may not shadow \
+                                               existing macros (see RFC 1560)")
+                                        .emit();
+                                }
                                 self.macro_names.insert(name);
                             }
                             if def.export {
@@ -250,7 +250,6 @@ impl<'b> Resolver<'b> {
                         attr::contains_name(&item.attrs, "no_implicit_prelude")
                     },
                     normal_ancestor_id: Some(item.id),
-                    macros_escape: self.contains_macro_use(&item.attrs),
                     ..ModuleS::new(Some(parent), ModuleKind::Def(def, name))
                 });
                 self.define(parent, name, TypeNS, (module, sp, vis));
@@ -520,45 +519,62 @@ impl<'b> Resolver<'b> {
 
 pub struct BuildReducedGraphVisitor<'a, 'b: 'a> {
     pub resolver: &'a mut Resolver<'b>,
+    pub legacy_scope: LegacyScope<'b>,
     pub expansion: Mark,
 }
 
 impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> {
-    fn visit_invoc(&mut self, id: ast::NodeId) {
-        self.resolver.expansion_data.get_mut(&Mark::from_placeholder_id(id)).unwrap().module =
-            self.resolver.current_module;
+    fn visit_invoc(&mut self, id: ast::NodeId) -> &'b InvocationData<'b> {
+        let invocation = self.resolver.invocations[&Mark::from_placeholder_id(id)];
+        invocation.module.set(self.resolver.current_module);
+        invocation.legacy_scope.set(self.legacy_scope);
+        invocation
     }
 }
 
 macro_rules! method {
     ($visit:ident: $ty:ty, $invoc:path, $walk:ident) => {
         fn $visit(&mut self, node: &$ty) {
-            match node.node {
-                $invoc(..) => self.visit_invoc(node.id),
-                _ => visit::$walk(self, node),
+            if let $invoc(..) = node.node {
+                self.visit_invoc(node.id);
+            } else {
+                visit::$walk(self, node);
             }
         }
     }
 }
 
 impl<'a, 'b> Visitor for BuildReducedGraphVisitor<'a, 'b> {
     method!(visit_impl_item: ast::ImplItem, ast::ImplItemKind::Macro, walk_impl_item);
-    method!(visit_stmt:      ast::Stmt,     ast::StmtKind::Mac,       walk_stmt);
     method!(visit_expr:      ast::Expr,     ast::ExprKind::Mac,       walk_expr);
     method!(visit_pat:       ast::Pat,      ast::PatKind::Mac,        walk_pat);
     method!(visit_ty:        ast::Ty,       ast::TyKind::Mac,         walk_ty);
 
     fn visit_item(&mut self, item: &Item) {
-        match item.node {
+        let macro_use = match item.node {
             ItemKind::Mac(..) if item.id == ast::DUMMY_NODE_ID => return, // Scope placeholder
-            ItemKind::Mac(..) => return self.visit_invoc(item.id),
-            _ => {}
-        }
+            ItemKind::Mac(..) => {
+                return self.legacy_scope = LegacyScope::Expansion(self.visit_invoc(item.id));
+            }
+            ItemKind::Mod(..) => self.resolver.contains_macro_use(&item.attrs),
+            _ => false,
+        };
 
-        let parent = self.resolver.current_module;
+        let (parent, legacy_scope) = (self.resolver.current_module, self.legacy_scope);
         self.resolver.build_reduced_graph_for_item(item, self.expansion);
         visit::walk_item(self, item);
         self.resolver.current_module = parent;
+        if !macro_use {
+            self.legacy_scope = legacy_scope;
+        }
+    }
+
+    fn visit_stmt(&mut self, stmt: &ast::Stmt) {
+        if let ast::StmtKind::Mac(..) = stmt.node {
+            self.legacy_scope = LegacyScope::Expansion(self.visit_invoc(stmt.id));
+        } else {
+            visit::walk_stmt(self, stmt);
+        }
     }
 
     fn visit_foreign_item(&mut self, foreign_item: &ForeignItem) {
@@ -567,18 +583,20 @@ impl<'a, 'b> Visitor for BuildReducedGraphVisitor<'a, 'b> {
     }
 
     fn visit_block(&mut self, block: &Block) {
-        let parent = self.resolver.current_module;
+        let (parent, legacy_scope) = (self.resolver.current_module, self.legacy_scope);
         self.resolver.build_reduced_graph_for_block(block);
         visit::walk_block(self, block);
         self.resolver.current_module = parent;
+        self.legacy_scope = legacy_scope;
     }
 
     fn visit_trait_item(&mut self, item: &TraitItem) {
         let parent = self.resolver.current_module;
         let def_id = parent.def_id().unwrap();
 
         if let TraitItemKind::Macro(_) = item.node {
-            return self.visit_invoc(item.id);
+            self.visit_invoc(item.id);
+            return
         }
 
         // Add the item to the trait info.

src/librustc_resolve/lib.rs

@@ -57,6 +57,7 @@ use syntax::ext::base::MultiItemModifier;
 use syntax::ext::hygiene::Mark;
 use syntax::ast::{self, FloatTy};
 use syntax::ast::{CRATE_NODE_ID, Name, NodeId, IntTy, UintTy};
+use syntax::ext::base::SyntaxExtension;
 use syntax::parse::token::{self, keywords};
 use syntax::util::lev_distance::find_best_match_for_name;
 
@@ -77,6 +78,7 @@ use std::mem::replace;
 use std::rc::Rc;
 
 use resolve_imports::{ImportDirective, NameResolution};
+use macros::{InvocationData, LegacyBinding, LegacyScope};
 
 // NB: This module needs to be declared first so diagnostics are
 // registered before they are used.
@@ -791,9 +793,6 @@ pub struct ModuleS<'a> {
     // access the children must be preceded with a
     // `populate_module_if_necessary` call.
     populated: Cell<bool>,
-
-    macros: RefCell<FnvHashMap<Name, macros::NameBinding>>,
-    macros_escape: bool,
 }
 
 pub type Module<'a> = &'a ModuleS<'a>;
@@ -811,8 +810,6 @@ impl<'a> ModuleS<'a> {
             globs: RefCell::new((Vec::new())),
             traits: RefCell::new(None),
             populated: Cell::new(true),
-            macros: RefCell::new(FnvHashMap()),
-            macros_escape: false,
         }
     }
 
@@ -1076,7 +1073,7 @@ pub struct Resolver<'a> {
 
     privacy_errors: Vec<PrivacyError<'a>>,
     ambiguity_errors: Vec<AmbiguityError<'a>>,
-    macro_shadowing_errors: FnvHashSet<Span>,
+    disallowed_shadowing: Vec<(Name, Span, LegacyScope<'a>)>,
 
     arenas: &'a ResolverArenas<'a>,
     dummy_binding: &'a NameBinding<'a>,
@@ -1086,9 +1083,10 @@ pub struct Resolver<'a> {
     pub derive_modes: FnvHashMap<Name, Rc<MultiItemModifier>>,
     crate_loader: &'a mut CrateLoader,
     macro_names: FnvHashSet<Name>,
+    builtin_macros: FnvHashMap<Name, Rc<SyntaxExtension>>,
 
     // Maps the `Mark` of an expansion to its containing module or block.
-    expansion_data: FnvHashMap<Mark, macros::ExpansionData<'a>>,
+    invocations: FnvHashMap<Mark, &'a InvocationData<'a>>,
 }
 
 pub struct ResolverArenas<'a> {
@@ -1097,6 +1095,8 @@ pub struct ResolverArenas<'a> {
     name_bindings: arena::TypedArena<NameBinding<'a>>,
     import_directives: arena::TypedArena<ImportDirective<'a>>,
     name_resolutions: arena::TypedArena<RefCell<NameResolution<'a>>>,
+    invocation_data: arena::TypedArena<InvocationData<'a>>,
+    legacy_bindings: arena::TypedArena<LegacyBinding<'a>>,
 }
 
 impl<'a> ResolverArenas<'a> {
@@ -1120,6 +1120,13 @@ impl<'a> ResolverArenas<'a> {
     fn alloc_name_resolution(&'a self) -> &'a RefCell<NameResolution<'a>> {
         self.name_resolutions.alloc(Default::default())
     }
+    fn alloc_invocation_data(&'a self, expansion_data: InvocationData<'a>)
+                             -> &'a InvocationData<'a> {
+        self.invocation_data.alloc(expansion_data)
+    }
+    fn alloc_legacy_binding(&'a self, binding: LegacyBinding<'a>) -> &'a LegacyBinding<'a> {
+        self.legacy_bindings.alloc(binding)
+    }
 }
 
 impl<'a> ty::NodeIdTree for Resolver<'a> {
@@ -1205,8 +1212,9 @@ impl<'a> Resolver<'a> {
         let mut definitions = Definitions::new();
         DefCollector::new(&mut definitions).collect_root();
 
-        let mut expansion_data = FnvHashMap();
-        expansion_data.insert(Mark::root(), macros::ExpansionData::root(graph_root));
+        let mut invocations = FnvHashMap();
+        invocations.insert(Mark::root(),
+                           arenas.alloc_invocation_data(InvocationData::root(graph_root)));
 
         Resolver {
             session: session,
@@ -1252,7 +1260,7 @@ impl<'a> Resolver<'a> {
 
             privacy_errors: Vec::new(),
             ambiguity_errors: Vec::new(),
-            macro_shadowing_errors: FnvHashSet(),
+            disallowed_shadowing: Vec::new(),
 
             arenas: arenas,
             dummy_binding: arenas.alloc_name_binding(NameBinding {
@@ -1266,7 +1274,8 @@ impl<'a> Resolver<'a> {
             derive_modes: FnvHashMap(),
             crate_loader: crate_loader,
             macro_names: FnvHashSet(),
-            expansion_data: expansion_data,
+            builtin_macros: FnvHashMap(),
+            invocations: invocations,
         }
     }
 
@@ -1277,6 +1286,8 @@ impl<'a> Resolver<'a> {
             name_bindings: arena::TypedArena::new(),
             import_directives: arena::TypedArena::new(),
             name_resolutions: arena::TypedArena::new(),
+            invocation_data: arena::TypedArena::new(),
+            legacy_bindings: arena::TypedArena::new(),
         }
     }
 
@@ -3338,7 +3349,8 @@ impl<'a> Resolver<'a> {
         vis.is_accessible_from(module.normal_ancestor_id.unwrap(), self)
     }
 
-    fn report_errors(&self) {
+    fn report_errors(&mut self) {
+        self.report_shadowing_errors();
         let mut reported_spans = FnvHashSet();
 
         for &AmbiguityError { span, name, b1, b2 } in &self.ambiguity_errors {
@@ -3366,6 +3378,20 @@ impl<'a> Resolver<'a> {
         }
     }
 
+    fn report_shadowing_errors(&mut self) {
+        let mut reported_errors = FnvHashSet();
+        for (name, span, scope) in replace(&mut self.disallowed_shadowing, Vec::new()) {
+            if self.resolve_macro_name(scope, name, false).is_some() &&
+               reported_errors.insert((name, span)) {
+                let msg = format!("`{}` is already in scope", name);
+                self.session.struct_span_err(span, &msg)
+                    .note("macro-expanded `macro_rules!`s may not shadow \
+                           existing macros (see RFC 1560)")
+                    .emit();
+            }
+        }
+    }
+
     fn report_conflict(&self,
                        parent: Module,
                        name: Name,