From ed1e00268b240ff725e0ff1173e366a00c5573cc Mon Sep 17 00:00:00 2001 From: Jeffrey Seyfried Date: Sun, 2 Oct 2016 01:41:19 +0000 Subject: [PATCH] Enforce the weakened shadowing restriction. --- src/librustc_resolve/build_reduced_graph.rs | 32 +++++++++++-- src/librustc_resolve/lib.rs | 2 + src/librustc_resolve/macros.rs | 51 ++++++++++++++++----- 3 files changed, 68 insertions(+), 17 deletions(-) diff --git a/src/librustc_resolve/build_reduced_graph.rs b/src/librustc_resolve/build_reduced_graph.rs index daf6a280ac0..2bf517600b7 100644 --- a/src/librustc_resolve/build_reduced_graph.rs +++ b/src/librustc_resolve/build_reduced_graph.rs @@ -13,6 +13,7 @@ //! Here we build the "reduced graph": the graph of the module tree without //! any imports resolved. +use macros; use resolve_imports::ImportDirectiveSubclass::{self, GlobImport}; use {Module, ModuleS, ModuleKind}; use Namespace::{self, TypeNS, ValueNS}; @@ -39,6 +40,7 @@ use syntax::ast::{Variant, ViewPathGlob, ViewPathList, ViewPathSimple}; use syntax::ext::base::{MultiItemModifier, Resolver as SyntaxResolver}; use syntax::ext::hygiene::Mark; use syntax::feature_gate::{self, emit_feature_err}; +use syntax::ext::tt::macro_rules; use syntax::parse::token::keywords; use syntax::visit::{self, Visitor}; @@ -77,7 +79,7 @@ impl<'b> Resolver<'b> { } /// Constructs the reduced graph for one item. - fn build_reduced_graph_for_item(&mut self, item: &Item) { + 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; @@ -188,9 +190,28 @@ impl<'b> Resolver<'b> { // We need to error on `#[macro_use] extern crate` when it isn't at the // crate root, because `$crate` won't work properly. let is_crate_root = self.current_module.parent.is_none(); - for def in self.crate_loader.load_macros(item, is_crate_root) { - match def.kind { - LoadedMacroKind::Def(def) => self.add_macro(Mark::root(), def), + for loaded_macro in self.crate_loader.load_macros(item, is_crate_root) { + match loaded_macro.kind { + 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, + }); + self.macro_names.insert(name); + } + if def.export { + def.id = self.next_node_id(); + self.exported_macros.push(def); + } + } LoadedMacroKind::CustomDerive(name, ext) => { self.insert_custom_derive(&name, ext, item.span); } @@ -527,6 +548,7 @@ impl<'b> Resolver<'b> { pub struct BuildReducedGraphVisitor<'a, 'b: 'a> { pub resolver: &'a mut Resolver<'b>, + pub expansion: Mark, } impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> { @@ -562,7 +584,7 @@ impl<'a, 'b> Visitor for BuildReducedGraphVisitor<'a, 'b> { } let parent = self.resolver.current_module; - self.resolver.build_reduced_graph_for_item(item); + self.resolver.build_reduced_graph_for_item(item, self.expansion); visit::walk_item(self, item); self.resolver.current_module = parent; } diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index 1ddd53d9c1b..ab6b0f24338 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -1073,6 +1073,7 @@ pub struct Resolver<'a> { privacy_errors: Vec>, ambiguity_errors: Vec>, + macro_shadowing_errors: FnvHashSet, arenas: &'a ResolverArenas<'a>, dummy_binding: &'a NameBinding<'a>, @@ -1248,6 +1249,7 @@ impl<'a> Resolver<'a> { privacy_errors: Vec::new(), ambiguity_errors: Vec::new(), + macro_shadowing_errors: FnvHashSet(), arenas: arenas, dummy_binding: arenas.alloc_name_binding(NameBinding { diff --git a/src/librustc_resolve/macros.rs b/src/librustc_resolve/macros.rs index bdd8d96aa7b..55f077b4d9a 100644 --- a/src/librustc_resolve/macros.rs +++ b/src/librustc_resolve/macros.rs @@ -22,10 +22,14 @@ use syntax::ext::hygiene::{Mark, SyntaxContext}; use syntax::ext::tt::macro_rules; use syntax::parse::token::intern; use syntax::util::lev_distance::find_best_match_for_name; +use syntax_pos::{Span, DUMMY_SP}; // FIXME(jseyfried) Merge with `::NameBinding`. pub struct NameBinding { - ext: Rc, + pub ext: Rc, + pub expansion: Mark, + pub shadowing: bool, + pub span: Span, } #[derive(Clone)] @@ -69,7 +73,7 @@ impl<'a> base::Resolver for Resolver<'a> { fn visit_expansion(&mut self, mark: Mark, expansion: &Expansion) { self.collect_def_ids(mark, expansion); self.current_module = self.expansion_data[&mark].module; - expansion.visit_with(&mut BuildReducedGraphVisitor { resolver: self }); + expansion.visit_with(&mut BuildReducedGraphVisitor { resolver: self, expansion: mark }); } fn add_macro(&mut self, scope: Mark, mut def: ast::MacroDef) { @@ -77,16 +81,18 @@ impl<'a> base::Resolver for Resolver<'a> { self.session.span_err(def.span, "user-defined macros may not be named `macro_rules`"); } if def.use_locally { - self.macro_names.insert(def.ident.name); - let ext = macro_rules::compile(&self.session.parse_sess, &def); - - let mut module = self.expansion_data[&scope].module; + let ExpansionData { mut module, backtrace, .. } = self.expansion_data[&scope]; while module.macros_escape { module = module.parent.unwrap(); } - module.macros.borrow_mut().insert(def.ident.name, NameBinding { - ext: Rc::new(ext), - }); + let binding = NameBinding { + ext: Rc::new(macro_rules::compile(&self.session.parse_sess, &def)), + expansion: backtrace.data().prev_ctxt.data().outer_mark, + shadowing: self.resolve_macro_name(scope, def.ident.name, false).is_some(), + span: def.span, + }; + module.macros.borrow_mut().insert(def.ident.name, binding); + self.macro_names.insert(def.ident.name); } if def.export { def.id = self.next_node_id(); @@ -100,6 +106,9 @@ impl<'a> base::Resolver for Resolver<'a> { } self.graph_root.macros.borrow_mut().insert(ident.name, NameBinding { ext: ext, + expansion: Mark::root(), + shadowing: false, + span: DUMMY_SP, }); } @@ -138,7 +147,7 @@ impl<'a> base::Resolver for Resolver<'a> { InvocationKind::Attr { ref attr, .. } => (intern(&*attr.name()), attr.span), }; - self.resolve_macro_name(scope, name).or_else(|| { + self.resolve_macro_name(scope, name, true).or_else(|| { let mut err = self.session.struct_span_err(span, &format!("macro undefined: '{}!'", name)); self.suggest_macro_name(&name.as_str(), &mut err); @@ -153,10 +162,28 @@ impl<'a> base::Resolver for Resolver<'a> { } impl<'a> Resolver<'a> { - fn resolve_macro_name(&mut self, scope: Mark, name: ast::Name) -> Option> { - let mut module = self.expansion_data[&scope].module; + pub fn resolve_macro_name(&mut self, scope: Mark, name: ast::Name, record_used: bool) + -> Option> { + let ExpansionData { mut module, backtrace, .. } = self.expansion_data[&scope]; loop { if let Some(binding) = module.macros.borrow().get(&name) { + let mut backtrace = backtrace.data(); + while binding.expansion != backtrace.outer_mark { + if backtrace.outer_mark != Mark::root() { + backtrace = backtrace.prev_ctxt.data(); + continue + } + + if record_used && binding.shadowing && + self.macro_shadowing_errors.insert(binding.span) { + let msg = format!("`{}` is already in scope", name); + self.session.struct_span_err(binding.span, &msg) + .note("macro-expanded `macro_rules!`s and `#[macro_use]`s \ + may not shadow existing macros (see RFC 1560)") + .emit(); + } + break + } return Some(binding.ext.clone()); } match module.parent {