From 111caef9a32e37e89b3503b3c08cdb9f309268e4 Mon Sep 17 00:00:00 2001 From: Jeffrey Seyfried Date: Tue, 11 Oct 2016 03:42:06 +0000 Subject: [PATCH] Clean up the scopes of expanded `#[macro_use]` imports. --- src/librustc_resolve/build_reduced_graph.rs | 17 +++++++++---- src/librustc_resolve/lib.rs | 4 +-- src/librustc_resolve/macros.rs | 27 +++++++++------------ src/test/compile-fail/macro-shadowing.rs | 15 +++++------- 4 files changed, 31 insertions(+), 32 deletions(-) diff --git a/src/librustc_resolve/build_reduced_graph.rs b/src/librustc_resolve/build_reduced_graph.rs index 12e5613159713..5600669d45fb9 100644 --- a/src/librustc_resolve/build_reduced_graph.rs +++ b/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::{InvocationData, LegacyImports, LegacyScope}; +use macros::{InvocationData, LegacyScope}; use resolve_imports::ImportDirectiveSubclass::{self, GlobImport}; use {Module, ModuleS, ModuleKind}; use Namespace::{self, TypeNS, ValueNS}; @@ -84,7 +84,7 @@ impl<'b> Resolver<'b> { } /// Constructs the reduced graph for one item. - fn build_reduced_graph_for_item(&mut self, item: &Item, legacy_imports: &mut LegacyImports) { + fn build_reduced_graph_for_item(&mut self, item: &Item, expansion: Mark) { let parent = self.current_module; let name = item.ident.name; let sp = item.span; @@ -202,7 +202,14 @@ impl<'b> Resolver<'b> { if def.use_locally { let ext = Rc::new(macro_rules::compile(&self.session.parse_sess, &def)); - legacy_imports.insert(name, (ext, loaded_macro.import_site)); + 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 { @@ -513,7 +520,7 @@ impl<'b> Resolver<'b> { pub struct BuildReducedGraphVisitor<'a, 'b: 'a> { pub resolver: &'a mut Resolver<'b>, pub legacy_scope: LegacyScope<'b>, - pub legacy_imports: LegacyImports, + pub expansion: Mark, } impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> { @@ -554,7 +561,7 @@ impl<'a, 'b> Visitor for BuildReducedGraphVisitor<'a, 'b> { }; let (parent, legacy_scope) = (self.resolver.current_module, self.legacy_scope); - self.resolver.build_reduced_graph_for_item(item, &mut self.legacy_imports); + self.resolver.build_reduced_graph_for_item(item, self.expansion); visit::walk_item(self, item); self.resolver.current_module = parent; if !macro_use { diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index fb18615263cf3..38a042735ff85 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -3389,8 +3389,8 @@ impl<'a> Resolver<'a> { 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 and `#[macro_use]`s \ - may not shadow existing macros (see RFC 1560)") + .note("macro-expanded `macro_rules!`s may not shadow \ + existing macros (see RFC 1560)") .emit(); } } diff --git a/src/librustc_resolve/macros.rs b/src/librustc_resolve/macros.rs index ca0958e085af0..6b00ebf3d05fc 100644 --- a/src/librustc_resolve/macros.rs +++ b/src/librustc_resolve/macros.rs @@ -111,19 +111,10 @@ impl<'a> base::Resolver for Resolver<'a> { let mut visitor = BuildReducedGraphVisitor { resolver: self, legacy_scope: LegacyScope::Invocation(invocation), - legacy_imports: FnvHashMap(), + expansion: mark, }; expansion.visit_with(&mut visitor); invocation.expansion.set(visitor.legacy_scope); - - if !visitor.legacy_imports.is_empty() { - invocation.legacy_scope.set({ - LegacyScope::Binding(self.arenas.alloc_legacy_binding(LegacyBinding { - parent: invocation.legacy_scope.get(), - kind: LegacyBindingKind::MacroUse(visitor.legacy_imports), - })) - }); - } } fn add_macro(&mut self, scope: Mark, mut def: ast::MacroDef) { @@ -173,7 +164,7 @@ impl<'a> base::Resolver for Resolver<'a> { None } - fn resolve_invoc(&mut self, scope: Mark, invoc: &Invocation, _force: bool) + fn resolve_invoc(&mut self, scope: Mark, invoc: &Invocation, force: bool) -> Result, Determinacy> { let (name, span) = match invoc.kind { InvocationKind::Bang { ref mac, .. } => { @@ -194,11 +185,15 @@ impl<'a> base::Resolver for Resolver<'a> { invocation.legacy_scope.set(LegacyScope::simplify_expansion(parent)); } self.resolve_macro_name(invocation.legacy_scope.get(), name, true).ok_or_else(|| { - let mut err = - self.session.struct_span_err(span, &format!("macro undefined: '{}!'", name)); - self.suggest_macro_name(&name.as_str(), &mut err); - err.emit(); - Determinacy::Determined + if force { + let mut err = + self.session.struct_span_err(span, &format!("macro undefined: '{}!'", name)); + self.suggest_macro_name(&name.as_str(), &mut err); + err.emit(); + Determinacy::Determined + } else { + Determinacy::Undetermined + } }) } diff --git a/src/test/compile-fail/macro-shadowing.rs b/src/test/compile-fail/macro-shadowing.rs index 22463825ef98f..8381dc34a6a15 100644 --- a/src/test/compile-fail/macro-shadowing.rs +++ b/src/test/compile-fail/macro-shadowing.rs @@ -12,31 +12,28 @@ macro_rules! foo { () => {} } macro_rules! macro_one { () => {} } +#[macro_use(macro_two)] extern crate two_macros; macro_rules! m1 { () => { macro_rules! foo { () => {} } //~ ERROR `foo` is already in scope - //~^ NOTE macro-expanded `macro_rules!`s and `#[macro_use]`s may not shadow existing macros + //~^ NOTE macro-expanded `macro_rules!`s may not shadow existing macros - #[macro_use] //~ ERROR `macro_one` is already in scope - //~^ NOTE macro-expanded `macro_rules!`s and `#[macro_use]`s may not shadow existing macros - extern crate two_macros; + #[macro_use] //~ ERROR `macro_two` is already in scope + //~^ NOTE macro-expanded `#[macro_use]`s may not shadow existing macros + extern crate two_macros as __; }} m1!(); //~ NOTE in this expansion //~| NOTE in this expansion //~| NOTE in this expansion //~| NOTE in this expansion -fn f() { macro_one!(); } foo!(); macro_rules! m2 { () => { macro_rules! foo { () => {} } - #[macro_use] extern crate two_macros as __; - - fn g() { macro_one!(); } foo!(); }} m2!(); -//^ Since `foo` and `macro_one` are not used outside this expansion, they are not shadowing errors. +//^ Since `foo` is not used outside this expansion, it is not a shadowing error. fn main() {}