diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index 101395e7ac2..26756e29926 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -1402,8 +1402,6 @@ pub struct Resolver<'a, 'b: 'a> { proc_mac_errors: Vec, /// crate-local macro expanded `macro_export` referred to by a module-relative path macro_expanded_macro_export_errors: BTreeSet<(Span, Span)>, - /// macro-expanded `macro_rules` shadowing existing macros - disallowed_shadowing: Vec<&'a LegacyBinding<'a>>, arenas: &'a ResolverArenas<'a>, dummy_binding: &'a NameBinding<'a>, @@ -1715,7 +1713,6 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> { ambiguity_errors: Vec::new(), use_injections: Vec::new(), proc_mac_errors: Vec::new(), - disallowed_shadowing: Vec::new(), macro_expanded_macro_export_errors: BTreeSet::new(), arenas, @@ -4534,7 +4531,6 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> { } fn report_errors(&mut self, krate: &Crate) { - self.report_shadowing_errors(); self.report_with_use_injections(krate); self.report_proc_macro_import(krate); let mut reported_spans = FxHashSet(); @@ -4572,20 +4568,6 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> { } } - fn report_shadowing_errors(&mut self) { - let mut reported_errors = FxHashSet(); - for binding in replace(&mut self.disallowed_shadowing, Vec::new()) { - if self.resolve_legacy_scope(&binding.parent, binding.ident, false).is_some() && - reported_errors.insert((binding.ident, binding.binding.span)) { - let msg = format!("`{}` is already in scope", binding.ident); - self.session.struct_span_err(binding.binding.span, &msg) - .note("macro-expanded `macro_rules!`s may not shadow \ - existing macros (see RFC 1560)") - .emit(); - } - } - } - fn report_conflict<'b>(&mut self, parent: Module, ident: Ident, diff --git a/src/librustc_resolve/macros.rs b/src/librustc_resolve/macros.rs index f240deb1b4c..3baf581c0de 100644 --- a/src/librustc_resolve/macros.rs +++ b/src/librustc_resolve/macros.rs @@ -81,9 +81,9 @@ pub enum LegacyScope<'a> { // Binding produced by a `macro_rules` item. // Not modularized, can shadow previous legacy bindings, etc. pub struct LegacyBinding<'a> { - pub binding: &'a NameBinding<'a>, - pub parent: Cell>, - pub ident: Ident, + binding: &'a NameBinding<'a>, + parent: Cell>, + ident: Ident, } pub struct ProcMacError { @@ -784,42 +784,101 @@ impl<'a, 'cl> Resolver<'a, 'cl> { } } - crate fn resolve_legacy_scope(&mut self, - mut scope: &'a Cell>, - ident: Ident, - record_used: bool) - -> Option<(&'a NameBinding<'a>, FromExpansion)> { + fn resolve_legacy_scope(&mut self, + scope: &'a Cell>, + ident: Ident, + record_used: bool) + -> Option<(&'a NameBinding<'a>, FromExpansion)> { let ident = ident.modern(); - let mut relative_depth: u32 = 0; + + // Names from inner scope that can't shadow names from outer scopes, e.g. + // macro_rules! mac { ... } + // { + // define_mac!(); // if this generates another `macro_rules! mac`, then it can't shadow + // // the outer `mac` and we have and ambiguity error + // mac!(); + // } + let mut potentially_ambiguous_result: Option<(&NameBinding, FromExpansion)> = None; + + // Go through all the scopes and try to resolve the name. + let mut where_to_resolve = scope; + let mut relative_depth = 0u32; loop { - match scope.get() { - LegacyScope::Empty => break, - LegacyScope::Expansion(invocation) => { - match invocation.expansion.get() { - LegacyScope::Invocation(_) => scope.set(invocation.legacy_scope.get()), - LegacyScope::Empty => { - scope = &invocation.legacy_scope; - } - _ => { - relative_depth += 1; - scope = &invocation.expansion; - } - } - } - LegacyScope::Invocation(invocation) => { - relative_depth = relative_depth.saturating_sub(1); - scope = &invocation.legacy_scope; - } - LegacyScope::Binding(potential_binding) => { - if potential_binding.ident == ident { - if record_used && relative_depth > 0 { - self.disallowed_shadowing.push(potential_binding); - } - return Some((potential_binding.binding, FromExpansion(relative_depth > 0))); - } - scope = &potential_binding.parent; + let result = match where_to_resolve.get() { + LegacyScope::Binding(legacy_binding) => if ident == legacy_binding.ident { + Some((legacy_binding.binding, FromExpansion(relative_depth > 0))) + } else { + None } + _ => None, }; + + macro_rules! continue_search { () => { + where_to_resolve = match where_to_resolve.get() { + LegacyScope::Binding(binding) => &binding.parent, + LegacyScope::Invocation(invocation) => { + relative_depth = relative_depth.saturating_sub(1); + &invocation.legacy_scope + } + LegacyScope::Expansion(invocation) => match invocation.expansion.get() { + LegacyScope::Empty => &invocation.legacy_scope, + LegacyScope::Binding(..) | LegacyScope::Expansion(..) => { + relative_depth += 1; + &invocation.expansion + } + LegacyScope::Invocation(..) => { + where_to_resolve.set(invocation.legacy_scope.get()); + where_to_resolve + } + } + LegacyScope::Empty => break, // nowhere else to search + }; + + continue; + }} + + match result { + Some(result) => { + if !record_used { + return Some(result); + } + + // Found a solution that is ambiguous with a previously found solution. + // Push an ambiguity error for later reporting and + // return something for better recovery. + if let Some(previous_result) = potentially_ambiguous_result { + if result.0.def() != previous_result.0.def() { + self.ambiguity_errors.push(AmbiguityError { + span: ident.span, + name: ident.name, + b1: previous_result.0, + b2: result.0, + }); + return Some(previous_result); + } + } + + // Found a solution that's not an ambiguity yet, but is "suspicious" and + // can participate in ambiguities later on. + // Remember it and go search for other solutions in outer scopes. + if (result.1).0 { + potentially_ambiguous_result = Some(result); + + continue_search!(); + } + + // Found a solution that can't be ambiguous. + return Some(result); + } + None => { + continue_search!(); + } + } + } + + // Previously found potentially ambiguous result turned out to not be ambiguous after all. + if let Some(previous_result) = potentially_ambiguous_result { + return Some(previous_result); } None @@ -846,8 +905,7 @@ impl<'a, 'cl> Resolver<'a, 'cl> { let check_consistency = |this: &Self, new_def: Def| { if let Some(def) = def { - if this.ambiguity_errors.is_empty() && this.disallowed_shadowing.is_empty() && - new_def != def && new_def != Def::Err { + if this.ambiguity_errors.is_empty() && new_def != def && new_def != Def::Err { // Make sure compilation does not succeed if preferred macro resolution // has changed after the macro had been expanded. In theory all such // situations should be reported as ambiguity errors, so this is span-bug. diff --git a/src/test/ui/macros/macro-shadowing.rs b/src/test/ui/macros/macro-shadowing.rs index 61abaf8a2dd..bf0a7fa21d3 100644 --- a/src/test/ui/macros/macro-shadowing.rs +++ b/src/test/ui/macros/macro-shadowing.rs @@ -17,14 +17,14 @@ macro_rules! macro_one { () => {} } #[macro_use(macro_two)] extern crate two_macros; macro_rules! m1 { () => { - macro_rules! foo { () => {} } //~ ERROR `foo` is already in scope + macro_rules! foo { () => {} } #[macro_use] //~ ERROR `macro_two` is already in scope extern crate two_macros as __; }} m1!(); -foo!(); +foo!(); //~ ERROR `foo` is ambiguous macro_rules! m2 { () => { macro_rules! foo { () => {} } diff --git a/src/test/ui/macros/macro-shadowing.stderr b/src/test/ui/macros/macro-shadowing.stderr index 28f09509a62..04f4abc4013 100644 --- a/src/test/ui/macros/macro-shadowing.stderr +++ b/src/test/ui/macros/macro-shadowing.stderr @@ -9,16 +9,27 @@ LL | m1!(); | = note: macro-expanded `#[macro_use]`s may not shadow existing macros (see RFC 1560) -error: `foo` is already in scope +error[E0659]: `foo` is ambiguous + --> $DIR/macro-shadowing.rs:27:1 + | +LL | foo!(); //~ ERROR `foo` is ambiguous + | ^^^ + | +note: `foo` could refer to the name defined here --> $DIR/macro-shadowing.rs:20:5 | -LL | macro_rules! foo { () => {} } //~ ERROR `foo` is already in scope +LL | macro_rules! foo { () => {} } | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ... LL | m1!(); | ------ in this macro invocation +note: `foo` could also refer to the name defined here + --> $DIR/macro-shadowing.rs:15:1 | - = note: macro-expanded `macro_rules!`s may not shadow existing macros (see RFC 1560) +LL | macro_rules! foo { () => {} } + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + = note: macro-expanded macros do not shadow error: aborting due to 2 previous errors +For more information about this error, try `rustc --explain E0659`. diff --git a/src/test/ui/out-of-order-shadowing.rs b/src/test/ui/out-of-order-shadowing.rs index 1fafaf85112..977b475b113 100644 --- a/src/test/ui/out-of-order-shadowing.rs +++ b/src/test/ui/out-of-order-shadowing.rs @@ -9,11 +9,10 @@ // except according to those terms. // aux-build:define_macro.rs -// error-pattern: `bar` is already in scope macro_rules! bar { () => {} } define_macro!(bar); -bar!(); +bar!(); //~ ERROR `bar` is ambiguous macro_rules! m { () => { #[macro_use] extern crate define_macro; } } m!(); diff --git a/src/test/ui/out-of-order-shadowing.stderr b/src/test/ui/out-of-order-shadowing.stderr index 78e32e23ff6..424c194d8fc 100644 --- a/src/test/ui/out-of-order-shadowing.stderr +++ b/src/test/ui/out-of-order-shadowing.stderr @@ -1,11 +1,22 @@ -error: `bar` is already in scope +error[E0659]: `bar` is ambiguous --> $DIR/out-of-order-shadowing.rs:15:1 | +LL | bar!(); //~ ERROR `bar` is ambiguous + | ^^^ + | +note: `bar` could refer to the name defined here + --> $DIR/out-of-order-shadowing.rs:14:1 + | LL | define_macro!(bar); | ^^^^^^^^^^^^^^^^^^^ +note: `bar` could also refer to the name defined here + --> $DIR/out-of-order-shadowing.rs:13:1 | - = note: macro-expanded `macro_rules!`s may not shadow existing macros (see RFC 1560) +LL | macro_rules! bar { () => {} } + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + = note: macro-expanded macros do not shadow = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) error: aborting due to previous error +For more information about this error, try `rustc --explain E0659`.