]> git.lizzy.rs Git - rust.git/commitdiff
Auto merge of #36767 - jseyfried:enforce_rfc_1560_shadowing, r=nrc
authorbors <bors@rust-lang.org>
Mon, 3 Oct 2016 08:30:32 +0000 (01:30 -0700)
committerGitHub <noreply@github.com>
Mon, 3 Oct 2016 08:30:32 +0000 (01:30 -0700)
Enforce the shadowing restrictions from RFC 1560 for today's macros

This PR enforces a weakened version of the shadowing restrictions from RFC 1560. More specifically,
 - If a macro expansion contains a `macro_rules!` macro definition that is used outside of the expansion, the defined macro may not shadow an existing macro.
 - If a macro expansion contains a `#[macro_use] extern crate` macro import that is used outside of the expansion, the imported macro may not shadow an existing macro.

This is a [breaking-change]. For example,
```rust
macro_rules! m { () => {} }
macro_rules! n { () => {
    macro_rules! m { () => {} } //< This shadows an existing macro.
    m!(); //< This is inside the expansion that generated `m`'s definition, so it is OK.
} }
n!();
m!(); //< This use of `m` is outside the expansion, so it causes the shadowing to be an error.
```

r? @nrc

1  2 
src/librustc/hir/map/def_collector.rs
src/librustc_resolve/lib.rs
src/librustc_resolve/macros.rs

index 49d889ff08dec6a5aab1436be986bdad355d7322,0941af0b81106978b227f32ea121f181163f16f0..421843a7f11d8752af6ac3bfa71126a14d68d097
@@@ -17,6 -17,7 +17,7 @@@ use hir::def_id::{CRATE_DEF_INDEX, DefI
  use middle::cstore::InlinedItem;
  
  use syntax::ast::*;
+ use syntax::ext::hygiene::Mark;
  use syntax::visit;
  use syntax::parse::token::{self, keywords};
  
@@@ -31,7 -32,7 +32,7 @@@ pub struct DefCollector<'a> 
  }
  
  pub struct MacroInvocationData {
-     pub id: NodeId,
+     pub mark: Mark,
      pub def_index: DefIndex,
      pub const_integer: bool,
  }
@@@ -126,7 -127,7 +127,7 @@@ impl<'a> DefCollector<'a> 
      fn visit_macro_invoc(&mut self, id: NodeId, const_integer: bool) {
          if let Some(ref mut visit) = self.visit_macro_invoc {
              visit(MacroInvocationData {
-                 id: id,
+                 mark: Mark::from_placeholder_id(id),
                  const_integer: const_integer,
                  def_index: self.parent_def.unwrap(),
              })
@@@ -286,7 -287,7 +287,7 @@@ impl<'a> visit::Visitor for DefCollecto
      fn visit_ty(&mut self, ty: &Ty) {
          match ty.node {
              TyKind::Mac(..) => return self.visit_macro_invoc(ty.id, false),
 -            TyKind::FixedLengthVec(_, ref length) => self.visit_ast_const_integer(length),
 +            TyKind::Array(_, ref length) => self.visit_ast_const_integer(length),
              TyKind::ImplTrait(..) => {
                  self.create_def(ty.id, DefPathData::ImplTrait);
              }
@@@ -448,7 -449,7 +449,7 @@@ impl<'ast> intravisit::Visitor<'ast> fo
      }
  
      fn visit_ty(&mut self, ty: &'ast hir::Ty) {
 -        if let hir::TyFixedLengthVec(_, ref length) = ty.node {
 +        if let hir::TyArray(_, ref length) = ty.node {
              self.visit_hir_const_integer(length);
          }
          if let hir::TyImplTrait(..) = ty.node {
index d1bfb7d786eb3192c5a58d2380bc34d0cc01a33c,ab6b0f24338ff4c3ff4c4bcd77248bb836a85663..90a9e5960617e65d6e41e8bc797e704232ce29a0
@@@ -57,7 -57,6 +57,6 @@@ use syntax::ext::base::MultiItemModifie
  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;
  
@@@ -793,7 -792,7 +792,7 @@@ pub struct ModuleS<'a> 
      // `populate_module_if_necessary` call.
      populated: Cell<bool>,
  
-     macros: RefCell<FnvHashMap<Name, Rc<SyntaxExtension>>>,
+     macros: RefCell<FnvHashMap<Name, macros::NameBinding>>,
      macros_escape: bool,
  }
  
@@@ -1074,6 -1073,7 +1073,7 @@@ pub struct Resolver<'a> 
  
      privacy_errors: Vec<PrivacyError<'a>>,
      ambiguity_errors: Vec<AmbiguityError<'a>>,
+     macro_shadowing_errors: FnvHashSet<Span>,
  
      arenas: &'a ResolverArenas<'a>,
      dummy_binding: &'a NameBinding<'a>,
      macro_names: FnvHashSet<Name>,
  
      // Maps the `Mark` of an expansion to its containing module or block.
-     expansion_data: FnvHashMap<u32, macros::ExpansionData<'a>>,
+     expansion_data: FnvHashMap<Mark, macros::ExpansionData<'a>>,
  }
  
  pub struct ResolverArenas<'a> {
@@@ -1203,7 -1203,7 +1203,7 @@@ impl<'a> Resolver<'a> 
          DefCollector::new(&mut definitions).collect_root();
  
          let mut expansion_data = FnvHashMap();
-         expansion_data.insert(0, macros::ExpansionData::root(graph_root)); // Crate root expansion
+         expansion_data.insert(Mark::root(), macros::ExpansionData::root(graph_root));
  
          Resolver {
              session: session,
  
              privacy_errors: Vec::new(),
              ambiguity_errors: Vec::new(),
+             macro_shadowing_errors: FnvHashSet(),
  
              arenas: arenas,
              dummy_binding: arenas.alloc_name_binding(NameBinding {
@@@ -3534,7 -3535,7 +3535,7 @@@ fn module_to_string(module: Module) -> 
          } else {
              // danger, shouldn't be ident?
              names.push(token::intern("<opaque>"));
 -            collect_mod(names, module);
 +            collect_mod(names, module.parent.unwrap());
          }
      }
      collect_mod(&mut names, module);
index 947aeb4439e16931cedf38a1612602f2d9edae8b,55f077b4d9ac333b6c18aecd744fb79dbb3b389b..3f6c69278bee8a2ec91472228925a01d262f4909
@@@ -18,13 -18,23 +18,23 @@@ use syntax::errors::DiagnosticBuilder
  use syntax::ext::base::{self, MultiModifier, MultiDecorator, MultiItemModifier};
  use syntax::ext::base::{NormalTT, SyntaxExtension};
  use syntax::ext::expand::{Expansion, Invocation, InvocationKind};
- use syntax::ext::hygiene::Mark;
+ 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 {
+     pub ext: Rc<SyntaxExtension>,
+     pub expansion: Mark,
+     pub shadowing: bool,
+     pub span: Span,
+ }
  
  #[derive(Clone)]
  pub struct ExpansionData<'a> {
+     backtrace: SyntaxContext,
      pub module: Module<'a>,
      def_index: DefIndex,
      // True if this expansion is in a `const_integer` position, for example `[u32; m!()]`.
@@@ -35,6 -45,7 +45,7 @@@
  impl<'a> ExpansionData<'a> {
      pub fn root(graph_root: Module<'a>) -> Self {
          ExpansionData {
+             backtrace: SyntaxContext::empty(),
              module: graph_root,
              def_index: CRATE_DEF_INDEX,
              const_integer: false,
@@@ -50,7 -61,8 +61,8 @@@ impl<'a> base::Resolver for Resolver<'a
      fn get_module_scope(&mut self, id: ast::NodeId) -> Mark {
          let mark = Mark::fresh();
          let module = self.module_map[&id];
-         self.expansion_data.insert(mark.as_u32(), ExpansionData {
+         self.expansion_data.insert(mark, ExpansionData {
+             backtrace: SyntaxContext::empty(),
              module: module,
              def_index: module.def_id().unwrap().index,
              const_integer: false,
@@@ -60,8 -72,8 +72,8 @@@
  
      fn visit_expansion(&mut self, mark: Mark, expansion: &Expansion) {
          self.collect_def_ids(mark, expansion);
-         self.current_module = self.expansion_data[&mark.as_u32()].module;
-         expansion.visit_with(&mut BuildReducedGraphVisitor { resolver: self });
+         self.current_module = self.expansion_data[&mark].module;
+         expansion.visit_with(&mut BuildReducedGraphVisitor { resolver: self, expansion: mark });
      }
  
      fn add_macro(&mut self, scope: Mark, mut def: ast::MacroDef) {
              self.session.span_err(def.span, "user-defined macros may not be named `macro_rules`");
          }
          if def.use_locally {
-             let ext = macro_rules::compile(&self.session.parse_sess, &def);
-             self.add_ext(scope, def.ident, Rc::new(ext));
+             let ExpansionData { mut module, backtrace, .. } = self.expansion_data[&scope];
+             while module.macros_escape {
+                 module = module.parent.unwrap();
+             }
+             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();
          }
      }
  
-     fn add_ext(&mut self, scope: Mark, ident: ast::Ident, ext: Rc<SyntaxExtension>) {
+     fn add_ext(&mut self, ident: ast::Ident, ext: Rc<SyntaxExtension>) {
          if let NormalTT(..) = *ext {
              self.macro_names.insert(ident.name);
          }
-         let mut module = self.expansion_data[&scope.as_u32()].module;
-         while module.macros_escape {
-             module = module.parent.unwrap();
-         }
-         module.macros.borrow_mut().insert(ident.name, ext);
+         self.graph_root.macros.borrow_mut().insert(ident.name, NameBinding {
+             ext: ext,
+             expansion: Mark::root(),
+             shadowing: false,
+             span: DUMMY_SP,
+         });
      }
  
      fn add_expansions_at_stmt(&mut self, id: ast::NodeId, macros: Vec<Mark>) {
      fn find_attr_invoc(&mut self, attrs: &mut Vec<ast::Attribute>) -> Option<ast::Attribute> {
          for i in 0..attrs.len() {
              let name = intern(&attrs[i].name());
-             match self.expansion_data[&0].module.macros.borrow().get(&name) {
-                 Some(ext) => match **ext {
+             match self.expansion_data[&Mark::root()].module.macros.borrow().get(&name) {
+                 Some(binding) => match *binding.ext {
                      MultiModifier(..) | MultiDecorator(..) | SyntaxExtension::AttrProcMacro(..) => {
                          return Some(attrs.remove(i))
                      }
              InvocationKind::Attr { ref attr, .. } => (intern(&*attr.name()), attr.span),
          };
  
-         let mut module = self.expansion_data[&scope.as_u32()].module;
+         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);
+             err.emit();
+             None
+         })
+     }
+     fn resolve_derive_mode(&mut self, ident: ast::Ident) -> Option<Rc<MultiItemModifier>> {
+         self.derive_modes.get(&ident.name).cloned()
+     }
+ }
+ impl<'a> Resolver<'a> {
+     pub fn resolve_macro_name(&mut self, scope: Mark, name: ast::Name, record_used: bool)
+                               -> Option<Rc<SyntaxExtension>> {
+         let ExpansionData { mut module, backtrace, .. } = self.expansion_data[&scope];
          loop {
-             if let Some(ext) = module.macros.borrow().get(&name) {
-                 return Some(ext.clone());
+             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 {
                  Some(parent) => module = parent,
                  None => break,
              }
          }
-         let mut err =
-             self.session.struct_span_err(span, &format!("macro undefined: '{}!'", name));
-         self.suggest_macro_name(&name.as_str(), &mut err);
-         err.emit();
          None
      }
  
-     fn resolve_derive_mode(&mut self, ident: ast::Ident) -> Option<Rc<MultiItemModifier>> {
-         self.derive_modes.get(&ident.name).cloned()
-     }
- }
- impl<'a> Resolver<'a> {
      fn suggest_macro_name(&mut self, name: &str, err: &mut DiagnosticBuilder<'a>) {
          if let Some(suggestion) = find_best_match_for_name(self.macro_names.iter(), name, None) {
              if suggestion != name {
  
      fn collect_def_ids(&mut self, mark: Mark, expansion: &Expansion) {
          let expansion_data = &mut self.expansion_data;
-         let ExpansionData { def_index, const_integer, module } = expansion_data[&mark.as_u32()];
+         let ExpansionData { backtrace, def_index, const_integer, module } = expansion_data[&mark];
          let visit_macro_invoc = &mut |invoc: map::MacroInvocationData| {
-             expansion_data.entry(invoc.id.as_u32()).or_insert(ExpansionData {
+             expansion_data.entry(invoc.mark).or_insert(ExpansionData {
+                 backtrace: backtrace.apply_mark(invoc.mark),
                  def_index: invoc.def_index,
                  const_integer: invoc.const_integer,
                  module: module,
  
          let mut def_collector = DefCollector::new(&mut self.definitions);
          def_collector.visit_macro_invoc = Some(visit_macro_invoc);
 -        def_collector.with_parent(def_index, |def_collector| if !const_integer {
 +        def_collector.with_parent(def_index, |def_collector| {
 +            if const_integer {
 +                if let Expansion::Expr(ref expr) = *expansion {
 +                    def_collector.visit_ast_const_integer(expr);
 +                }
 +            }
              expansion.visit_with(def_collector)
 -        } else if let Expansion::Expr(ref expr) = *expansion {
 -            def_collector.visit_ast_const_integer(expr);
          });
      }
  }