From 6c4b551403624b064c4a5836dfa59971a215cf4a Mon Sep 17 00:00:00 2001 From: Jeffrey Seyfried Date: Mon, 31 Oct 2016 06:48:59 +0000 Subject: [PATCH 1/3] Cleanup `Resolver::disallowed_shadowing`. --- src/librustc_resolve/lib.rs | 14 +++++++------- src/librustc_resolve/macros.rs | 8 ++++---- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index 0b382fcbfdd51..fcd299b56b0a5 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -77,7 +77,7 @@ use std::mem::replace; use std::rc::Rc; use resolve_imports::{ImportDirective, NameResolution}; -use macros::{InvocationData, LegacyBinding, LegacyScope}; +use macros::{InvocationData, LegacyBinding}; // NB: This module needs to be declared first so diagnostics are // registered before they are used. @@ -1067,7 +1067,7 @@ pub struct Resolver<'a> { privacy_errors: Vec>, ambiguity_errors: Vec>, - disallowed_shadowing: Vec<(Name, Span, LegacyScope<'a>)>, + disallowed_shadowing: Vec<&'a LegacyBinding<'a>>, arenas: &'a ResolverArenas<'a>, dummy_binding: &'a NameBinding<'a>, @@ -3364,11 +3364,11 @@ impl<'a> Resolver<'a> { fn report_shadowing_errors(&mut self) { 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, false).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(); diff --git a/src/librustc_resolve/macros.rs b/src/librustc_resolve/macros.rs index 72e5823598ea1..eb72145b774e1 100644 --- a/src/librustc_resolve/macros.rs +++ b/src/librustc_resolve/macros.rs @@ -74,10 +74,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, - span: Span, + pub span: Span, } pub type LegacyImports = FnvHashMap, Span)>; @@ -213,7 +213,7 @@ 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)); + self.disallowed_shadowing.push(binding); } return Some(binding.ext.clone()); } From 076c5d445b7611995a8d7e26e94c327a79eb20e2 Mon Sep 17 00:00:00 2001 From: Jeffrey Seyfried Date: Mon, 31 Oct 2016 22:17:15 +0000 Subject: [PATCH 2/3] Fix shadowing checking. --- src/librustc_resolve/lib.rs | 10 ++++++++-- src/librustc_resolve/macros.rs | 19 +++++++++++++------ 2 files changed, 21 insertions(+), 8 deletions(-) diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index fcd299b56b0a5..e7d83a64e03eb 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -77,7 +77,7 @@ use std::mem::replace; use std::rc::Rc; use resolve_imports::{ImportDirective, NameResolution}; -use macros::{InvocationData, LegacyBinding}; +use macros::{InvocationData, LegacyBinding, LegacyScope}; // NB: This module needs to be declared first so diagnostics are // registered before they are used. @@ -1077,6 +1077,7 @@ pub struct Resolver<'a> { crate_loader: &'a mut CrateLoader, macro_names: FnvHashSet, builtin_macros: FnvHashMap>, + lexical_macro_resolutions: Vec<(Name, LegacyScope<'a>)>, // Maps the `Mark` of an expansion to its containing module or block. invocations: FnvHashMap>, @@ -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,9 +3365,13 @@ 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 binding in replace(&mut self.disallowed_shadowing, Vec::new()) { - if self.resolve_macro_name(binding.parent, binding.name, false).is_some() && + 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) diff --git a/src/librustc_resolve/macros.rs b/src/librustc_resolve/macros.rs index eb72145b774e1..356158e58da6f 100644 --- a/src/librustc_resolve/macros.rs +++ b/src/librustc_resolve/macros.rs @@ -174,7 +174,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); @@ -189,17 +189,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> { + 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; @@ -212,7 +213,10 @@ impl<'a> Resolver<'a> { } LegacyScope::Binding(binding) => { if binding.name == name { - if record_used && relative_depth > 0 { + 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()); @@ -222,6 +226,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() } From 1e6c275b1c685454b357d9ec5357a45de333963e Mon Sep 17 00:00:00 2001 From: Jeffrey Seyfried Date: Tue, 1 Nov 2016 01:35:05 +0000 Subject: [PATCH 3/3] Add regression test. --- .../compile-fail/auxiliary/define_macro.rs | 16 ++++++++++++++ .../compile-fail/out-of-order-shadowing.rs | 21 +++++++++++++++++++ 2 files changed, 37 insertions(+) create mode 100644 src/test/compile-fail/auxiliary/define_macro.rs create mode 100644 src/test/compile-fail/out-of-order-shadowing.rs diff --git a/src/test/compile-fail/auxiliary/define_macro.rs b/src/test/compile-fail/auxiliary/define_macro.rs new file mode 100644 index 0000000000000..6b6b14a896b29 --- /dev/null +++ b/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 or the MIT license +// , 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 { () => {} } + } +} diff --git a/src/test/compile-fail/out-of-order-shadowing.rs b/src/test/compile-fail/out-of-order-shadowing.rs new file mode 100644 index 0000000000000..1fafaf85112be --- /dev/null +++ b/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 or the MIT license +// , 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() {}