Skip to content

Commit f374565

Browse files
borsbors
authored
Auto merge of #36767 - jseyfried:enforce_rfc_1560_shadowing, r=nrc
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
2 parents 144af3e + 057302b commit f374565

File tree

12 files changed

+210
-79
lines changed

12 files changed

+210
-79
lines changed

‎src/librustc/hir/map/def_collector.rs

+3-2
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ use hir::def_id::{CRATE_DEF_INDEX, DefId, DefIndex};
1717
use middle::cstore::InlinedItem;
1818

1919
use syntax::ast::*;
20+
use syntax::ext::hygiene::Mark;
2021
use syntax::visit;
2122
use syntax::parse::token::{self, keywords};
2223

@@ -31,7 +32,7 @@ pub struct DefCollector<'a> {
3132
}
3233

3334
pub struct MacroInvocationData {
34-
pub id: NodeId,
35+
pub mark: Mark,
3536
pub def_index: DefIndex,
3637
pub const_integer: bool,
3738
}
@@ -126,7 +127,7 @@ impl<'a> DefCollector<'a> {
126127
fn visit_macro_invoc(&mut self, id: NodeId, const_integer: bool) {
127128
if let Some(ref mut visit) = self.visit_macro_invoc {
128129
visit(MacroInvocationData {
129-
id: id,
130+
mark: Mark::from_placeholder_id(id),
130131
const_integer: const_integer,
131132
def_index: self.parent_def.unwrap(),
132133
})

‎src/librustc/middle/cstore.rs

+6-1
Original file line numberDiff line numberDiff line change
@@ -423,7 +423,12 @@ impl<'tcx> CrateStore<'tcx> for DummyCrateStore {
423423
fn metadata_encoding_version(&self) -> &[u8] { bug!("metadata_encoding_version") }
424424
}
425425

426-
pub enum LoadedMacro {
426+
pub struct LoadedMacro {
427+
pub import_site: Span,
428+
pub kind: LoadedMacroKind,
429+
}
430+
431+
pub enum LoadedMacroKind {
427432
Def(ast::MacroDef),
428433
CustomDerive(String, Rc<MultiItemModifier>),
429434
}

‎src/librustc_metadata/macro_import.rs

+30-20
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ use std::mem;
1818
use creader::{CrateLoader, Macros};
1919

2020
use rustc::hir::def_id::DefIndex;
21-
use rustc::middle::cstore::LoadedMacro;
21+
use rustc::middle::cstore::{LoadedMacro, LoadedMacroKind};
2222
use rustc::session::Session;
2323
use rustc::util::nodemap::FnvHashMap;
2424
use rustc_back::dynamic_lib::DynamicLibrary;
@@ -28,14 +28,19 @@ use syntax::ast;
2828
use syntax::attr;
2929
use syntax::parse::token;
3030
use syntax_ext::deriving::custom::CustomDerive;
31-
use syntax_pos::Span;
31+
use syntax_pos::{Span, DUMMY_SP};
3232

3333
pub fn call_bad_macro_reexport(a: &Session, b: Span) {
3434
span_err!(a, b, E0467, "bad macro reexport");
3535
}
3636

3737
pub type MacroSelection = FnvHashMap<token::InternedString, Span>;
3838

39+
enum ImportSelection {
40+
All(Span),
41+
Some(MacroSelection),
42+
}
43+
3944
pub fn load_macros(loader: &mut CrateLoader, extern_crate: &ast::Item, allows_macros: bool)
4045
-> Vec<LoadedMacro> {
4146
loader.load_crate(extern_crate, allows_macros)
@@ -46,7 +51,7 @@ impl<'a> CrateLoader<'a> {
4651
extern_crate: &ast::Item,
4752
allows_macros: bool) -> Vec<LoadedMacro> {
4853
// Parse the attributes relating to macros.
49-
let mut import = Some(FnvHashMap()); // None => load all
54+
let mut import = ImportSelection::Some(FnvHashMap());
5055
let mut reexport = FnvHashMap();
5156

5257
for attr in &extern_crate.attrs {
@@ -55,11 +60,9 @@ impl<'a> CrateLoader<'a> {
5560
"macro_use" => {
5661
let names = attr.meta_item_list();
5762
if names.is_none() {
58-
// no names => load all
59-
import = None;
60-
}
61-
if let (Some(sel), Some(names)) = (import.as_mut(), names) {
62-
for attr in names {
63+
import = ImportSelection::All(attr.span);
64+
} else if let ImportSelection::Some(ref mut sel) = import {
65+
for attr in names.unwrap() {
6366
if let Some(word) = attr.word() {
6467
sel.insert(word.name().clone(), attr.span());
6568
} else {
@@ -98,10 +101,10 @@ impl<'a> CrateLoader<'a> {
98101
fn load_macros<'b>(&mut self,
99102
vi: &ast::Item,
100103
allows_macros: bool,
101-
import: Option<MacroSelection>,
104+
import: ImportSelection,
102105
reexport: MacroSelection)
103106
-> Vec<LoadedMacro> {
104-
if let Some(sel) = import.as_ref() {
107+
if let ImportSelection::Some(ref sel) = import {
105108
if sel.is_empty() && reexport.is_empty() {
106109
return Vec::new();
107110
}
@@ -120,15 +123,19 @@ impl<'a> CrateLoader<'a> {
120123
for mut def in macros.macro_rules.drain(..) {
121124
let name = def.ident.name.as_str();
122125

123-
def.use_locally = match import.as_ref() {
124-
None => true,
125-
Some(sel) => sel.contains_key(&name),
126+
let import_site = match import {
127+
ImportSelection::All(span) => Some(span),
128+
ImportSelection::Some(ref sel) => sel.get(&name).cloned()
126129
};
130+
def.use_locally = import_site.is_some();
127131
def.export = reexport.contains_key(&name);
128132
def.allow_internal_unstable = attr::contains_name(&def.attrs,
129133
"allow_internal_unstable");
130134
debug!("load_macros: loaded: {:?}", def);
131-
ret.push(LoadedMacro::Def(def));
135+
ret.push(LoadedMacro {
136+
kind: LoadedMacroKind::Def(def),
137+
import_site: import_site.unwrap_or(DUMMY_SP),
138+
});
132139
seen.insert(name);
133140
}
134141

@@ -137,7 +144,7 @@ impl<'a> CrateLoader<'a> {
137144
// exported macros, enforced elsewhere
138145
assert_eq!(ret.len(), 0);
139146

140-
if import.is_some() {
147+
if let ImportSelection::Some(..) = import {
141148
self.sess.span_err(vi.span, "`rustc-macro` crates cannot be \
142149
selectively imported from, must \
143150
use `#[macro_use]`");
@@ -151,10 +158,10 @@ impl<'a> CrateLoader<'a> {
151158
self.load_derive_macros(vi.span, &macros, index, &mut ret);
152159
}
153160

154-
if let Some(sel) = import.as_ref() {
161+
if let ImportSelection::Some(sel) = import {
155162
for (name, span) in sel {
156163
if !seen.contains(&name) {
157-
span_err!(self.sess, *span, E0469,
164+
span_err!(self.sess, span, E0469,
158165
"imported macro not found");
159166
}
160167
}
@@ -199,18 +206,21 @@ impl<'a> CrateLoader<'a> {
199206
mem::transmute::<*mut u8, fn(&mut Registry)>(sym)
200207
};
201208

202-
struct MyRegistrar<'a>(&'a mut Vec<LoadedMacro>);
209+
struct MyRegistrar<'a>(&'a mut Vec<LoadedMacro>, Span);
203210

204211
impl<'a> Registry for MyRegistrar<'a> {
205212
fn register_custom_derive(&mut self,
206213
trait_name: &str,
207214
expand: fn(TokenStream) -> TokenStream) {
208215
let derive = Rc::new(CustomDerive::new(expand));
209-
self.0.push(LoadedMacro::CustomDerive(trait_name.to_string(), derive));
216+
self.0.push(LoadedMacro {
217+
kind: LoadedMacroKind::CustomDerive(trait_name.to_string(), derive),
218+
import_site: self.1,
219+
});
210220
}
211221
}
212222

213-
registrar(&mut MyRegistrar(ret));
223+
registrar(&mut MyRegistrar(ret, span));
214224

215225
// Intentionally leak the dynamic library. We can't ever unload it
216226
// since the library can make things that will live arbitrarily long.

‎src/librustc_resolve/build_reduced_graph.rs

+30-8
Original file line numberDiff line numberDiff line change
@@ -13,14 +13,15 @@
1313
//! Here we build the "reduced graph": the graph of the module tree without
1414
//! any imports resolved.
1515
16+
use macros;
1617
use resolve_imports::ImportDirectiveSubclass::{self, GlobImport};
1718
use {Module, ModuleS, ModuleKind};
1819
use Namespace::{self, TypeNS, ValueNS};
1920
use {NameBinding, NameBindingKind, ToNameBinding};
2021
use Resolver;
2122
use {resolve_error, resolve_struct_error, ResolutionError};
2223

23-
use rustc::middle::cstore::LoadedMacro;
24+
use rustc::middle::cstore::LoadedMacroKind;
2425
use rustc::hir::def::*;
2526
use rustc::hir::def_id::{CRATE_DEF_INDEX, DefId};
2627
use rustc::hir::map::DefPathData;
@@ -39,6 +40,7 @@ use syntax::ast::{Variant, ViewPathGlob, ViewPathList, ViewPathSimple};
3940
use syntax::ext::base::{MultiItemModifier, Resolver as SyntaxResolver};
4041
use syntax::ext::hygiene::Mark;
4142
use syntax::feature_gate::{self, emit_feature_err};
43+
use syntax::ext::tt::macro_rules;
4244
use syntax::parse::token::keywords;
4345
use syntax::visit::{self, Visitor};
4446

@@ -77,7 +79,7 @@ impl<'b> Resolver<'b> {
7779
}
7880

7981
/// Constructs the reduced graph for one item.
80-
fn build_reduced_graph_for_item(&mut self, item: &Item) {
82+
fn build_reduced_graph_for_item(&mut self, item: &Item, expansion: Mark) {
8183
let parent = self.current_module;
8284
let name = item.ident.name;
8385
let sp = item.span;
@@ -188,10 +190,29 @@ impl<'b> Resolver<'b> {
188190
// We need to error on `#[macro_use] extern crate` when it isn't at the
189191
// crate root, because `$crate` won't work properly.
190192
let is_crate_root = self.current_module.parent.is_none();
191-
for def in self.crate_loader.load_macros(item, is_crate_root) {
192-
match def {
193-
LoadedMacro::Def(def) => self.add_macro(Mark::root(), def),
194-
LoadedMacro::CustomDerive(name, ext) => {
193+
for loaded_macro in self.crate_loader.load_macros(item, is_crate_root) {
194+
match loaded_macro.kind {
195+
LoadedMacroKind::Def(mut def) => {
196+
let name = def.ident.name;
197+
if def.use_locally {
198+
let ext = macro_rules::compile(&self.session.parse_sess, &def);
199+
let shadowing =
200+
self.resolve_macro_name(Mark::root(), name, false).is_some();
201+
self.expansion_data[&Mark::root()].module.macros.borrow_mut()
202+
.insert(name, macros::NameBinding {
203+
ext: Rc::new(ext),
204+
expansion: expansion,
205+
shadowing: shadowing,
206+
span: loaded_macro.import_site,
207+
});
208+
self.macro_names.insert(name);
209+
}
210+
if def.export {
211+
def.id = self.next_node_id();
212+
self.exported_macros.push(def);
213+
}
214+
}
215+
LoadedMacroKind::CustomDerive(name, ext) => {
195216
self.insert_custom_derive(&name, ext, item.span);
196217
}
197218
}
@@ -527,11 +548,12 @@ impl<'b> Resolver<'b> {
527548

528549
pub struct BuildReducedGraphVisitor<'a, 'b: 'a> {
529550
pub resolver: &'a mut Resolver<'b>,
551+
pub expansion: Mark,
530552
}
531553

532554
impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> {
533555
fn visit_invoc(&mut self, id: ast::NodeId) {
534-
self.resolver.expansion_data.get_mut(&id.as_u32()).unwrap().module =
556+
self.resolver.expansion_data.get_mut(&Mark::from_placeholder_id(id)).unwrap().module =
535557
self.resolver.current_module;
536558
}
537559
}
@@ -562,7 +584,7 @@ impl<'a, 'b> Visitor for BuildReducedGraphVisitor<'a, 'b> {
562584
}
563585

564586
let parent = self.resolver.current_module;
565-
self.resolver.build_reduced_graph_for_item(item);
587+
self.resolver.build_reduced_graph_for_item(item, self.expansion);
566588
visit::walk_item(self, item);
567589
self.resolver.current_module = parent;
568590
}

‎src/librustc_resolve/lib.rs

+5-4
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,6 @@ use syntax::ext::base::MultiItemModifier;
5757
use syntax::ext::hygiene::Mark;
5858
use syntax::ast::{self, FloatTy};
5959
use syntax::ast::{CRATE_NODE_ID, Name, NodeId, IntTy, UintTy};
60-
use syntax::ext::base::SyntaxExtension;
6160
use syntax::parse::token::{self, keywords};
6261
use syntax::util::lev_distance::find_best_match_for_name;
6362

@@ -793,7 +792,7 @@ pub struct ModuleS<'a> {
793792
// `populate_module_if_necessary` call.
794793
populated: Cell<bool>,
795794

796-
macros: RefCell<FnvHashMap<Name, Rc<SyntaxExtension>>>,
795+
macros: RefCell<FnvHashMap<Name, macros::NameBinding>>,
797796
macros_escape: bool,
798797
}
799798

@@ -1074,6 +1073,7 @@ pub struct Resolver<'a> {
10741073

10751074
privacy_errors: Vec<PrivacyError<'a>>,
10761075
ambiguity_errors: Vec<AmbiguityError<'a>>,
1076+
macro_shadowing_errors: FnvHashSet<Span>,
10771077

10781078
arenas: &'a ResolverArenas<'a>,
10791079
dummy_binding: &'a NameBinding<'a>,
@@ -1085,7 +1085,7 @@ pub struct Resolver<'a> {
10851085
macro_names: FnvHashSet<Name>,
10861086

10871087
// Maps the `Mark` of an expansion to its containing module or block.
1088-
expansion_data: FnvHashMap<u32, macros::ExpansionData<'a>>,
1088+
expansion_data: FnvHashMap<Mark, macros::ExpansionData<'a>>,
10891089
}
10901090

10911091
pub struct ResolverArenas<'a> {
@@ -1203,7 +1203,7 @@ impl<'a> Resolver<'a> {
12031203
DefCollector::new(&mut definitions).collect_root();
12041204

12051205
let mut expansion_data = FnvHashMap();
1206-
expansion_data.insert(0, macros::ExpansionData::root(graph_root)); // Crate root expansion
1206+
expansion_data.insert(Mark::root(), macros::ExpansionData::root(graph_root));
12071207

12081208
Resolver {
12091209
session: session,
@@ -1249,6 +1249,7 @@ impl<'a> Resolver<'a> {
12491249

12501250
privacy_errors: Vec::new(),
12511251
ambiguity_errors: Vec::new(),
1252+
macro_shadowing_errors: FnvHashSet(),
12521253

12531254
arenas: arenas,
12541255
dummy_binding: arenas.alloc_name_binding(NameBinding {

0 commit comments

Comments
 (0)