Auto merge of #37506 - jseyfried:improve_shadowing_checks, r=nrc

macros: improve shadowing checks

This PR improves macro-expanded shadowing checks to work with out-of-(pre)order expansion.

Out-of-order expansion became possible in #37084, so this technically a [breaking-change] for nightly.
The regression test from this PR is an example of code that would break.

r? @nrc

src/librustc_resolve/lib.rs

@@ -1067,7 +1067,7 @@ pub struct Resolver<'a> {
 
  privacy_errors: Vec<PrivacyError<'a>>,
  ambiguity_errors: Vec<AmbiguityError<'a>>,
- disallowed_shadowing: Vec<(Name, Span, LegacyScope<'a>)>,
+ disallowed_shadowing: Vec<&'a LegacyBinding<'a>>,
 
  arenas: &'a ResolverArenas<'a>,
  dummy_binding: &'a NameBinding<'a>,
@@ -1077,6 +1077,7 @@ pub struct Resolver<'a> {
  crate_loader: &'a mut CrateLoader,
  macro_names: FnvHashSet<Name>,
  builtin_macros: FnvHashMap<Name, Rc<SyntaxExtension>>,
+ lexical_macro_resolutions: Vec<(Name, LegacyScope<'a>)>,
 
  // Maps the `Mark` of an expansion to its containing module or block.
  invocations: FnvHashMap<Mark, &'a InvocationData<'a>>,
@@ -1267,6 +1268,7 @@ impl<'a> Resolver<'a> {
  crate_loader: crate_loader,
  macro_names: FnvHashSet(),
  builtin_macros: FnvHashMap(),
+ lexical_macro_resolutions: Vec::new(),
  invocations: invocations,
  }
  }
@@ -3363,12 +3365,16 @@ impl<'a> Resolver<'a> {
  }
 
  fn report_shadowing_errors(&mut self) {
+ for (name, scope) in replace(&mut self.lexical_macro_resolutions, Vec::new()) {
+ self.resolve_macro_name(scope, name);
+ }
+
  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)
+ for binding in replace(&mut self.disallowed_shadowing, Vec::new()) {
+ if self.resolve_macro_name(binding.parent, binding.name).is_some() &&
+ reported_errors.insert((binding.name, binding.span)) {
+ let msg = format!("`{}` is already in scope", binding.name);
+ self.session.struct_span_err(binding.span, &msg)
  .note("macro-expanded `macro_rules!`s may not shadow \
  existing macros (see RFC 1560)")
  .emit();

src/librustc_resolve/macros.rs

@@ -73,10 +73,10 @@ impl<'a> LegacyScope<'a> {
 }
 
 pub struct LegacyBinding<'a> {
- parent: LegacyScope<'a>,
- name: ast::Name,
+ pub parent: LegacyScope<'a>,
+ pub name: ast::Name,
  ext: Rc<SyntaxExtension>,
- span: Span,
+ pub span: Span,
 }
 
 impl<'a> base::Resolver for Resolver<'a> {
@@ -171,7 +171,7 @@ impl<'a> base::Resolver for Resolver<'a> {
  if let LegacyScope::Expansion(parent) = invocation.legacy_scope.get() {
  invocation.legacy_scope.set(LegacyScope::simplify_expansion(parent));
  }
- self.resolve_macro_name(invocation.legacy_scope.get(), name, true).ok_or_else(|| {
+ self.resolve_macro_name(invocation.legacy_scope.get(), name).ok_or_else(|| {
  if force {
  let msg = format!("macro undefined: '{}!'", name);
  let mut err = self.session.struct_span_err(path.span, &msg);
@@ -186,17 +186,18 @@ impl<'a> base::Resolver for Resolver<'a> {
 }
 
 impl<'a> Resolver<'a> {
- pub fn resolve_macro_name(&mut self,
- mut scope: LegacyScope<'a>,
- name: ast::Name,
- record_used: bool)
+ pub fn resolve_macro_name(&mut self, mut scope: LegacyScope<'a>, name: ast::Name)
  -> Option<Rc<SyntaxExtension>> {
+ let mut possible_time_travel = None;
  let mut relative_depth: u32 = 0;
  loop {
  scope = match scope {
  LegacyScope::Empty => break,
  LegacyScope::Expansion(invocation) => {
  if let LegacyScope::Empty = invocation.expansion.get() {
+ if possible_time_travel.is_none() {
+ possible_time_travel = Some(scope);
+ }
  invocation.legacy_scope.get()
  } else {
  relative_depth += 1;
@@ -209,8 +210,11 @@ impl<'a> Resolver<'a> {
  }
  LegacyScope::Binding(binding) => {
  if binding.name == name {
- if record_used && relative_depth > 0 {
- self.disallowed_shadowing.push((name, binding.span, binding.parent));
+ if let Some(scope) = possible_time_travel {
+ // Check for disallowed shadowing later
+ self.lexical_macro_resolutions.push((name, scope));
+ } else if relative_depth > 0 {
+ self.disallowed_shadowing.push(binding);
  }
  return Some(binding.ext.clone());
  }
@@ -219,6 +223,9 @@ impl<'a> Resolver<'a> {
  };
  }
 
+ if let Some(scope) = possible_time_travel {
+ self.lexical_macro_resolutions.push((name, scope));
+ }
  self.builtin_macros.get(&name).cloned()
  }
 

src/test/compile-fail/auxiliary/define_macro.rs

@@ -0,0 +1,16 @@
+// Copyright 2016 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+#[macro_export]
+macro_rules! define_macro {
+ ($i:ident) => {
+ macro_rules! $i { () => {} }
+ }
+}

src/test/compile-fail/out-of-order-shadowing.rs

@@ -0,0 +1,21 @@
+// Copyright 2016 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+// aux-build:define_macro.rs
+// error-pattern: `bar` is already in scope
+
+macro_rules! bar { () => {} }
+define_macro!(bar);
+bar!();
+
+macro_rules! m { () => { #[macro_use] extern crate define_macro; } }
+m!();
+
+fn main() {}