From b35c306b657c25f676c3981e2c78ea73f44ef378 Mon Sep 17 00:00:00 2001 From: Jeffrey Seyfried Date: Fri, 2 Dec 2016 23:10:44 +0000 Subject: [PATCH 01/16] Fix the path resolutions of glob imports. --- src/librustc_resolve/resolve_imports.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/librustc_resolve/resolve_imports.rs b/src/librustc_resolve/resolve_imports.rs index 890891fd090..64a8e21f9e0 100644 --- a/src/librustc_resolve/resolve_imports.rs +++ b/src/librustc_resolve/resolve_imports.rs @@ -708,10 +708,7 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { } // Record the destination of this import - if let Some(did) = module.def_id() { - let resolution = PathResolution::new(Def::Mod(did)); - self.def_map.insert(directive.id, resolution); - } + self.def_map.insert(directive.id, PathResolution::new(module.def().unwrap())); } // Miscellaneous post-processing, including recording reexports, reporting conflicts, From 83ab9f7faccddee23545e12d323791f9e2d06380 Mon Sep 17 00:00:00 2001 From: Jeffrey Seyfried Date: Tue, 29 Nov 2016 02:07:02 +0000 Subject: [PATCH 02/16] Remove some unused functions and fix formatting. --- src/libsyntax/attr.rs | 30 ------------------------------ src/libsyntax/ext/hygiene.rs | 12 ++++++------ 2 files changed, 6 insertions(+), 36 deletions(-) diff --git a/src/libsyntax/attr.rs b/src/libsyntax/attr.rs index 45c120e0b95..c31bcfbd869 100644 --- a/src/libsyntax/attr.rs +++ b/src/libsyntax/attr.rs @@ -29,7 +29,6 @@ use symbol::Symbol; use util::ThinVec; use std::cell::{RefCell, Cell}; -use std::collections::HashSet; thread_local! { static USED_ATTRS: RefCell> = RefCell::new(Vec::new()); @@ -372,16 +371,6 @@ pub fn mk_spanned_attr_outer(sp: Span, id: AttrId, item: MetaItem) -> Attribute } } -pub fn mk_doc_attr_outer(id: AttrId, item: MetaItem, is_sugared_doc: bool) -> Attribute { - Attribute { - id: id, - style: ast::AttrStyle::Outer, - value: item, - is_sugared_doc: is_sugared_doc, - span: DUMMY_SP, - } -} - pub fn mk_sugared_doc_attr(id: AttrId, text: Symbol, lo: BytePos, hi: BytePos) -> Attribute { let style = doc_comment_style(&text.as_str()); @@ -421,13 +410,6 @@ pub fn first_attr_value_str_by_name(attrs: &[Attribute], name: &str) -> Option Option { - items.iter() - .rev() - .find(|mi| mi.check_name(name)) - .and_then(|i| i.value_str()) -} - /* Higher-level applications */ pub fn find_crate_name(attrs: &[Attribute]) -> Option { @@ -856,18 +838,6 @@ pub fn find_deprecation(diagnostic: &Handler, attrs: &[Attribute], find_deprecation_generic(diagnostic, attrs.iter(), item_sp) } -pub fn require_unique_names(diagnostic: &Handler, metas: &[MetaItem]) { - let mut set = HashSet::new(); - for meta in metas { - let name = meta.name(); - - if !set.insert(name.clone()) { - panic!(diagnostic.span_fatal(meta.span, - &format!("duplicate meta item `{}`", name))); - } - } -} - /// Parse #[repr(...)] forms. /// diff --git a/src/libsyntax/ext/hygiene.rs b/src/libsyntax/ext/hygiene.rs index 0fd72277cca..8842cb55b1e 100644 --- a/src/libsyntax/ext/hygiene.rs +++ b/src/libsyntax/ext/hygiene.rs @@ -115,12 +115,12 @@ impl SyntaxContext { }) } - /// If `ident` is macro expanded, return the source ident from the macro definition - /// and the mark of the expansion that created the macro definition. - pub fn source(self) -> (Self /* source context */, Mark /* source macro */) { - let macro_def_ctxt = self.data().prev_ctxt.data(); - (macro_def_ctxt.prev_ctxt, macro_def_ctxt.outer_mark) - } + /// If `ident` is macro expanded, return the source ident from the macro definition + /// and the mark of the expansion that created the macro definition. + pub fn source(self) -> (Self /* source context */, Mark /* source macro */) { + let macro_def_ctxt = self.data().prev_ctxt.data(); + (macro_def_ctxt.prev_ctxt, macro_def_ctxt.outer_mark) + } } impl fmt::Debug for SyntaxContext { From 59de7f8f04d9122dd776f2edd73eb77a4fc94054 Mon Sep 17 00:00:00 2001 From: Jeffrey Seyfried Date: Tue, 29 Nov 2016 02:07:12 +0000 Subject: [PATCH 03/16] Add `ident.unhygienize()` and use `Ident` more instead of `Name` in `resolve`. --- src/librustc_resolve/build_reduced_graph.rs | 108 +++++++++---------- src/librustc_resolve/lib.rs | 51 +++++---- src/librustc_resolve/macros.rs | 23 ++-- src/librustc_resolve/resolve_imports.rs | 112 ++++++++++---------- src/libsyntax/ast.rs | 12 ++- 5 files changed, 156 insertions(+), 150 deletions(-) diff --git a/src/librustc_resolve/build_reduced_graph.rs b/src/librustc_resolve/build_reduced_graph.rs index 25a37931ba3..4aa04dc34fe 100644 --- a/src/librustc_resolve/build_reduced_graph.rs +++ b/src/librustc_resolve/build_reduced_graph.rs @@ -28,7 +28,7 @@ use rustc::ty; use std::cell::Cell; use std::rc::Rc; -use syntax::ast::Name; +use syntax::ast::{Name, Ident}; use syntax::attr; use syntax::ast::{self, Block, ForeignItem, ForeignItemKind, Item, ItemKind}; @@ -76,12 +76,12 @@ struct LegacyMacroImports { impl<'b> Resolver<'b> { /// Defines `name` in namespace `ns` of module `parent` to be `def` if it is not yet defined; /// otherwise, reports an error. - fn define(&mut self, parent: Module<'b>, name: Name, ns: Namespace, def: T) + fn define(&mut self, parent: Module<'b>, ident: Ident, ns: Namespace, def: T) where T: ToNameBinding<'b>, { let binding = def.to_name_binding(); - if let Err(old_binding) = self.try_define(parent, name, ns, binding.clone()) { - self.report_conflict(parent, name, ns, old_binding, &binding); + if let Err(old_binding) = self.try_define(parent, ident, ns, binding.clone()) { + self.report_conflict(parent, ident, ns, old_binding, &binding); } } @@ -102,7 +102,7 @@ impl<'b> Resolver<'b> { /// Constructs the reduced graph for one item. fn build_reduced_graph_for_item(&mut self, item: &Item, expansion: Mark) { let parent = self.current_module; - let name = item.ident.name; + let ident = item.ident; let sp = item.span; let vis = self.resolve_visibility(&item.vis); @@ -157,8 +157,8 @@ impl<'b> Resolver<'b> { } let subclass = SingleImport { - target: binding.name, - source: source.name, + target: binding, + source: source, result: self.per_ns(|_, _| Cell::new(Err(Undetermined))), }; self.add_import_directive( @@ -187,13 +187,13 @@ impl<'b> Resolver<'b> { for source_item in source_items { let node = source_item.node; - let (module_path, name, rename) = { + let (module_path, ident, rename) = { if node.name.name != keywords::SelfValue.name() { - let rename = node.rename.unwrap_or(node.name).name; - (module_path.clone(), node.name.name, rename) + let rename = node.rename.unwrap_or(node.name); + (module_path.clone(), node.name, rename) } else { - let name = match module_path.last() { - Some(ident) => ident.name, + let ident = match module_path.last() { + Some(&ident) => ident, None => { resolve_error( self, @@ -205,13 +205,13 @@ impl<'b> Resolver<'b> { } }; let module_path = module_path.split_last().unwrap().1; - let rename = node.rename.map(|i| i.name).unwrap_or(name); - (module_path.to_vec(), name, rename) + let rename = node.rename.unwrap_or(ident); + (module_path.to_vec(), ident, rename) } }; let subclass = SingleImport { target: rename, - source: name, + source: ident, result: self.per_ns(|_, _| Cell::new(Err(Undetermined))), }; let id = source_item.node.id; @@ -251,7 +251,7 @@ impl<'b> Resolver<'b> { expansion: expansion, }); let imported_binding = self.import(binding, directive); - self.define(parent, name, TypeNS, imported_binding); + self.define(parent, ident, TypeNS, imported_binding); self.populate_module_if_necessary(module); self.process_legacy_macro_imports(item, module, expansion); } @@ -265,9 +265,9 @@ impl<'b> Resolver<'b> { attr::contains_name(&item.attrs, "no_implicit_prelude") }, normal_ancestor_id: Some(item.id), - ..ModuleS::new(Some(parent), ModuleKind::Def(def, name)) + ..ModuleS::new(Some(parent), ModuleKind::Def(def, ident.name)) }); - self.define(parent, name, TypeNS, (module, vis, sp, expansion)); + self.define(parent, ident, TypeNS, (module, vis, sp, expansion)); self.module_map.insert(item.id, module); // Descend into the module. @@ -280,27 +280,27 @@ impl<'b> Resolver<'b> { ItemKind::Static(_, m, _) => { let mutbl = m == Mutability::Mutable; let def = Def::Static(self.definitions.local_def_id(item.id), mutbl); - self.define(parent, name, ValueNS, (def, vis, sp, expansion)); + self.define(parent, ident, ValueNS, (def, vis, sp, expansion)); } ItemKind::Const(..) => { let def = Def::Const(self.definitions.local_def_id(item.id)); - self.define(parent, name, ValueNS, (def, vis, sp, expansion)); + self.define(parent, ident, ValueNS, (def, vis, sp, expansion)); } ItemKind::Fn(..) => { let def = Def::Fn(self.definitions.local_def_id(item.id)); - self.define(parent, name, ValueNS, (def, vis, sp, expansion)); + self.define(parent, ident, ValueNS, (def, vis, sp, expansion)); } // These items live in the type namespace. ItemKind::Ty(..) => { let def = Def::TyAlias(self.definitions.local_def_id(item.id)); - self.define(parent, name, TypeNS, (def, vis, sp, expansion)); + self.define(parent, ident, TypeNS, (def, vis, sp, expansion)); } ItemKind::Enum(ref enum_definition, _) => { let def = Def::Enum(self.definitions.local_def_id(item.id)); - let module = self.new_module(parent, ModuleKind::Def(def, name), true); - self.define(parent, name, TypeNS, (module, vis, sp, expansion)); + let module = self.new_module(parent, ModuleKind::Def(def, ident.name), true); + self.define(parent, ident, TypeNS, (module, vis, sp, expansion)); for variant in &(*enum_definition).variants { self.build_reduced_graph_for_variant(variant, module, vis, expansion); @@ -311,14 +311,14 @@ impl<'b> Resolver<'b> { ItemKind::Struct(ref struct_def, _) => { // Define a name in the type namespace. let def = Def::Struct(self.definitions.local_def_id(item.id)); - self.define(parent, name, TypeNS, (def, vis, sp, expansion)); + self.define(parent, ident, TypeNS, (def, vis, sp, expansion)); // If this is a tuple or unit struct, define a name // in the value namespace as well. if !struct_def.is_struct() { let ctor_def = Def::StructCtor(self.definitions.local_def_id(struct_def.id()), CtorKind::from_ast(struct_def)); - self.define(parent, name, ValueNS, (ctor_def, vis, sp, expansion)); + self.define(parent, ident, ValueNS, (ctor_def, vis, sp, expansion)); } // Record field names for error reporting. @@ -332,7 +332,7 @@ impl<'b> Resolver<'b> { ItemKind::Union(ref vdata, _) => { let def = Def::Union(self.definitions.local_def_id(item.id)); - self.define(parent, name, TypeNS, (def, vis, sp, expansion)); + self.define(parent, ident, TypeNS, (def, vis, sp, expansion)); // Record field names for error reporting. let field_names = vdata.fields().iter().filter_map(|field| { @@ -350,8 +350,8 @@ impl<'b> Resolver<'b> { // Add all the items within to a new module. let module = - self.new_module(parent, ModuleKind::Def(Def::Trait(def_id), name), true); - self.define(parent, name, TypeNS, (module, vis, sp, expansion)); + self.new_module(parent, ModuleKind::Def(Def::Trait(def_id), ident.name), true); + self.define(parent, ident, TypeNS, (module, vis, sp, expansion)); self.current_module = module; } ItemKind::Mac(_) => panic!("unexpanded macro in resolve!"), @@ -365,26 +365,23 @@ impl<'b> Resolver<'b> { parent: Module<'b>, vis: ty::Visibility, expansion: Mark) { - let name = variant.node.name.name; + let ident = variant.node.name; let def_id = self.definitions.local_def_id(variant.node.data.id()); // Define a name in the type namespace. let def = Def::Variant(def_id); - self.define(parent, name, TypeNS, (def, vis, variant.span, expansion)); + self.define(parent, ident, TypeNS, (def, vis, variant.span, expansion)); // Define a constructor name in the value namespace. // Braced variants, unlike structs, generate unusable names in // value namespace, they are reserved for possible future use. let ctor_kind = CtorKind::from_ast(&variant.node.data); let ctor_def = Def::VariantCtor(def_id, ctor_kind); - self.define(parent, name, ValueNS, (ctor_def, vis, variant.span, expansion)); + self.define(parent, ident, ValueNS, (ctor_def, vis, variant.span, expansion)); } /// Constructs the reduced graph for one foreign item. fn build_reduced_graph_for_foreign_item(&mut self, item: &ForeignItem, expansion: Mark) { - let parent = self.current_module; - let name = item.ident.name; - let def = match item.node { ForeignItemKind::Fn(..) => { Def::Fn(self.definitions.local_def_id(item.id)) @@ -393,8 +390,9 @@ impl<'b> Resolver<'b> { Def::Static(self.definitions.local_def_id(item.id), m) } }; + let parent = self.current_module; let vis = self.resolve_visibility(&item.vis); - self.define(parent, name, ValueNS, (def, vis, item.span, expansion)); + self.define(parent, item.ident, ValueNS, (def, vis, item.span, expansion)); } fn build_reduced_graph_for_block(&mut self, block: &Block) { @@ -414,7 +412,7 @@ impl<'b> Resolver<'b> { /// Builds the reduced graph for a single item in an external crate. fn build_reduced_graph_for_external_crate_def(&mut self, parent: Module<'b>, child: Export) { - let name = child.name; + let ident = Ident::with_empty_ctxt(child.name); let def = child.def; let def_id = def.def_id(); let vis = match def { @@ -425,25 +423,25 @@ impl<'b> Resolver<'b> { match def { Def::Mod(..) | Def::Enum(..) => { - let module = self.new_module(parent, ModuleKind::Def(def, name), false); - self.define(parent, name, TypeNS, (module, vis, DUMMY_SP, Mark::root())); + let module = self.new_module(parent, ModuleKind::Def(def, ident.name), false); + self.define(parent, ident, TypeNS, (module, vis, DUMMY_SP, Mark::root())); } Def::Variant(..) => { - self.define(parent, name, TypeNS, (def, vis, DUMMY_SP, Mark::root())); + self.define(parent, ident, TypeNS, (def, vis, DUMMY_SP, Mark::root())); } Def::VariantCtor(..) => { - self.define(parent, name, ValueNS, (def, vis, DUMMY_SP, Mark::root())); + self.define(parent, ident, ValueNS, (def, vis, DUMMY_SP, Mark::root())); } Def::Fn(..) | Def::Static(..) | Def::Const(..) | Def::AssociatedConst(..) | Def::Method(..) => { - self.define(parent, name, ValueNS, (def, vis, DUMMY_SP, Mark::root())); + self.define(parent, ident, ValueNS, (def, vis, DUMMY_SP, Mark::root())); } Def::Trait(..) => { - let module = self.new_module(parent, ModuleKind::Def(def, name), false); - self.define(parent, name, TypeNS, (module, vis, DUMMY_SP, Mark::root())); + let module = self.new_module(parent, ModuleKind::Def(def, ident.name), false); + self.define(parent, ident, TypeNS, (module, vis, DUMMY_SP, Mark::root())); // If this is a trait, add all the trait item names to the trait info. let trait_item_def_ids = self.session.cstore.associated_item_def_ids(def_id); @@ -455,27 +453,27 @@ impl<'b> Resolver<'b> { } } Def::TyAlias(..) | Def::AssociatedTy(..) => { - self.define(parent, name, TypeNS, (def, vis, DUMMY_SP, Mark::root())); + self.define(parent, ident, TypeNS, (def, vis, DUMMY_SP, Mark::root())); } Def::Struct(..) => { - self.define(parent, name, TypeNS, (def, vis, DUMMY_SP, Mark::root())); + self.define(parent, ident, TypeNS, (def, vis, DUMMY_SP, Mark::root())); // Record field names for error reporting. let field_names = self.session.cstore.struct_field_names(def_id); self.insert_field_names(def_id, field_names); } Def::StructCtor(..) => { - self.define(parent, name, ValueNS, (def, vis, DUMMY_SP, Mark::root())); + self.define(parent, ident, ValueNS, (def, vis, DUMMY_SP, Mark::root())); } Def::Union(..) => { - self.define(parent, name, TypeNS, (def, vis, DUMMY_SP, Mark::root())); + self.define(parent, ident, TypeNS, (def, vis, DUMMY_SP, Mark::root())); // Record field names for error reporting. let field_names = self.session.cstore.struct_field_names(def_id); self.insert_field_names(def_id, field_names); } Def::Macro(..) => { - self.define(parent, name, MacroNS, (def, vis, DUMMY_SP, Mark::root())); + self.define(parent, ident, MacroNS, (def, vis, DUMMY_SP, Mark::root())); } Def::Local(..) | Def::PrimTy(..) | @@ -574,12 +572,13 @@ impl<'b> Resolver<'b> { } if let Some(span) = legacy_imports.import_all { - module.for_each_child(|name, ns, binding| if ns == MacroNS { - self.legacy_import_macro(name, binding, span, allow_shadowing); + module.for_each_child(|ident, ns, binding| if ns == MacroNS { + self.legacy_import_macro(ident.name, binding, span, allow_shadowing); }); } else { for (name, span) in legacy_imports.imports { - let result = self.resolve_name_in_module(module, name, MacroNS, false, None); + let ident = Ident::with_empty_ctxt(name); + let result = self.resolve_ident_in_module(module, ident, MacroNS, false, None); if let Ok(binding) = result { self.legacy_import_macro(name, binding, span, allow_shadowing); } else { @@ -591,7 +590,8 @@ impl<'b> Resolver<'b> { let krate = module.def_id().unwrap().krate; self.used_crates.insert(krate); self.session.cstore.export_macros(krate); - let result = self.resolve_name_in_module(module, name, MacroNS, false, None); + let ident = Ident::with_empty_ctxt(name); + let result = self.resolve_ident_in_module(module, ident, MacroNS, false, None); if let Ok(binding) = result { self.macro_exports.push(Export { name: name, def: binding.def() }); } else { @@ -759,7 +759,7 @@ impl<'a, 'b> Visitor<'a> for BuildReducedGraphVisitor<'a, 'b> { self.resolver.trait_item_map.insert((item.ident.name, def_id), is_static_method); let vis = ty::Visibility::Public; - self.resolver.define(parent, item.ident.name, ns, (def, vis, item.span, self.expansion)); + self.resolver.define(parent, item.ident, ns, (def, vis, item.span, self.expansion)); self.resolver.current_module = parent.parent.unwrap(); // nearest normal ancestor visit::walk_trait_item(self, item); diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index 509ee704e2e..4d4f4629c75 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -781,8 +781,8 @@ pub struct ModuleS<'a> { // The node id of the closest normal module (`mod`) ancestor (including this module). normal_ancestor_id: Option, - resolutions: RefCell>>>, - legacy_macro_resolutions: RefCell>, + resolutions: RefCell>>>, + legacy_macro_resolutions: RefCell>, macro_resolutions: RefCell, PathScope, Span)>>, // Macro invocations that can expand into items in this module. @@ -794,7 +794,7 @@ pub struct ModuleS<'a> { globs: RefCell>>, // Used to memoize the traits in this module for faster searches through all traits in scope. - traits: RefCell)]>>>, + traits: RefCell)]>>>, // Whether this module is populated. If not populated, any attempt to // access the children must be preceded with a @@ -822,9 +822,9 @@ impl<'a> ModuleS<'a> { } } - fn for_each_child)>(&self, mut f: F) { - for (&(name, ns), name_resolution) in self.resolutions.borrow().iter() { - name_resolution.borrow().binding.map(|binding| f(name, ns, binding)); + fn for_each_child)>(&self, mut f: F) { + for (&(ident, ns), name_resolution) in self.resolutions.borrow().iter() { + name_resolution.borrow().binding.map(|binding| f(ident, ns, binding)); } } @@ -1334,7 +1334,7 @@ impl<'a> Resolver<'a> { }) } - fn record_use(&mut self, name: Name, ns: Namespace, binding: &'a NameBinding<'a>, span: Span) + fn record_use(&mut self, ident: Ident, ns: Namespace, binding: &'a NameBinding<'a>, span: Span) -> bool /* true if an error was reported */ { // track extern crates for unused_extern_crate lint if let Some(DefId { krate, .. }) = binding.module().and_then(ModuleS::def_id) { @@ -1345,13 +1345,13 @@ impl<'a> Resolver<'a> { NameBindingKind::Import { directive, binding, ref used } if !used.get() => { used.set(true); self.used_imports.insert((directive.id, ns)); - self.add_to_glob_map(directive.id, name); - self.record_use(name, ns, binding, span) + self.add_to_glob_map(directive.id, ident); + self.record_use(ident, ns, binding, span) } NameBindingKind::Import { .. } => false, NameBindingKind::Ambiguity { b1, b2 } => { self.ambiguity_errors.push(AmbiguityError { - span: span, name: name, lexical: false, b1: b1, b2: b2, + span: span, name: ident.name, lexical: false, b1: b1, b2: b2, }); true } @@ -1359,9 +1359,9 @@ impl<'a> Resolver<'a> { } } - fn add_to_glob_map(&mut self, id: NodeId, name: Name) { + fn add_to_glob_map(&mut self, id: NodeId, ident: Ident) { if self.make_glob_map { - self.glob_map.entry(id).or_insert_with(FxHashSet).insert(name); + self.glob_map.entry(id).or_insert_with(FxHashSet).insert(ident.name); } } @@ -1388,7 +1388,7 @@ impl<'a> Resolver<'a> { record_used: Option) -> Option> { if ns == TypeNS { - ident = Ident::with_empty_ctxt(ident.name); + ident = ident.unhygienize(); } // Walk backwards up the ribs in scope. @@ -1403,8 +1403,7 @@ impl<'a> Resolver<'a> { } if let ModuleRibKind(module) = self.ribs[ns][i].kind { - let name = ident.name; - let item = self.resolve_name_in_module(module, name, ns, false, record_used); + let item = self.resolve_ident_in_module(module, ident, ns, false, record_used); if let Ok(binding) = item { // The ident resolves to an item. return Some(LexicalScopeBinding::Item(binding)); @@ -1413,7 +1412,7 @@ impl<'a> Resolver<'a> { if let ModuleKind::Block(..) = module.kind { // We can see through blocks } else if !module.no_implicit_prelude { return self.prelude.and_then(|prelude| { - self.resolve_name_in_module(prelude, name, ns, false, None).ok() + self.resolve_ident_in_module(prelude, ident, ns, false, None).ok() }).map(LexicalScopeBinding::Item) } else { return None; @@ -2183,8 +2182,7 @@ impl<'a> Resolver<'a> { Def::VariantCtor(_, CtorKind::Const) | Def::Const(..) if !always_binding => { // A unit struct/variant or constant pattern. - let name = ident.node.name; - self.record_use(name, ValueNS, binding.unwrap(), ident.span); + self.record_use(ident.node, ValueNS, binding.unwrap(), ident.span); Some(PathResolution::new(def)) } Def::StructCtor(..) | Def::VariantCtor(..) | @@ -2363,9 +2361,9 @@ impl<'a> Resolver<'a> { allow_super = false; let binding = if let Some(module) = module { - self.resolve_name_in_module(module, ident.name, ns, false, record_used) + self.resolve_ident_in_module(module, ident, ns, false, record_used) } else if opt_ns == Some(MacroNS) { - self.resolve_lexical_macro_path_segment(ident.name, ns, record_used) + self.resolve_lexical_macro_path_segment(ident, ns, record_used) } else { match self.resolve_ident_in_lexical_scope(ident, ns, record_used) { Some(LexicalScopeBinding::Item(binding)) => Ok(binding), @@ -2953,16 +2951,15 @@ impl<'a> Resolver<'a> { in_module_is_extern)) = worklist.pop() { self.populate_module_if_necessary(in_module); - in_module.for_each_child(|name, ns, name_binding| { + in_module.for_each_child(|ident, ns, name_binding| { // avoid imports entirely if name_binding.is_import() && !name_binding.is_extern_crate() { return; } // collect results based on the filter function - if name == lookup_name && ns == namespace { + if ident.name == lookup_name && ns == namespace { if filter_fn(name_binding.def()) { // create the path - let ident = Ident::with_empty_ctxt(name); let params = PathParameters::none(); let segment = PathSegment { identifier: ident, @@ -2994,7 +2991,7 @@ impl<'a> Resolver<'a> { // form the path let mut path_segments = path_segments.clone(); path_segments.push(PathSegment { - identifier: Ident::with_empty_ctxt(name), + identifier: ident, parameters: PathParameters::none(), }); @@ -3124,13 +3121,13 @@ impl<'a> Resolver<'a> { fn report_conflict(&mut self, parent: Module, - name: Name, + ident: Ident, ns: Namespace, binding: &NameBinding, old_binding: &NameBinding) { // Error on the second of two conflicting names if old_binding.span.lo > binding.span.lo { - return self.report_conflict(parent, name, ns, old_binding, binding); + return self.report_conflict(parent, ident, ns, old_binding, binding); } let container = match parent.kind { @@ -3145,7 +3142,7 @@ impl<'a> Resolver<'a> { false => ("defined", "definition"), }; - let span = binding.span; + let (name, span) = (ident.name, binding.span); if let Some(s) = self.name_already_seen.get(&name) { if s == &span { diff --git a/src/librustc_resolve/macros.rs b/src/librustc_resolve/macros.rs index 6c02967672d..b2e31d54909 100644 --- a/src/librustc_resolve/macros.rs +++ b/src/librustc_resolve/macros.rs @@ -19,7 +19,7 @@ use rustc::hir::map::{self, DefCollector}; use rustc::ty; use std::cell::Cell; use std::rc::Rc; -use syntax::ast::{self, Name}; +use syntax::ast::{self, Name, Ident}; use syntax::errors::DiagnosticBuilder; use syntax::ext::base::{self, Determinacy, MultiModifier, MultiDecorator}; use syntax::ext::base::{NormalTT, SyntaxExtension}; @@ -246,7 +246,7 @@ impl<'a> base::Resolver for Resolver<'a> { let result = match self.resolve_legacy_scope(&invocation.legacy_scope, name, false) { Some(MacroBinding::Legacy(binding)) => Ok(binding.ext.clone()), Some(MacroBinding::Modern(binding)) => Ok(binding.get_macro(self)), - None => match self.resolve_lexical_macro_path_segment(name, MacroNS, None) { + None => match self.resolve_lexical_macro_path_segment(path[0], MacroNS, None) { Ok(binding) => Ok(binding.get_macro(self)), Err(Determinacy::Undetermined) if !force => return Err(Determinacy::Undetermined), _ => { @@ -260,7 +260,7 @@ impl<'a> base::Resolver for Resolver<'a> { }; if self.use_extern_macros { - self.current_module.legacy_macro_resolutions.borrow_mut().push((scope, name, span)); + self.current_module.legacy_macro_resolutions.borrow_mut().push((scope, path[0], span)); } result } @@ -269,7 +269,7 @@ impl<'a> base::Resolver for Resolver<'a> { impl<'a> Resolver<'a> { // Resolve the initial segment of a non-global macro path (e.g. `foo` in `foo::bar!();`) pub fn resolve_lexical_macro_path_segment(&mut self, - name: Name, + ident: Ident, ns: Namespace, record_used: Option) -> Result<&'a NameBinding<'a>, Determinacy> { @@ -278,7 +278,7 @@ impl<'a> Resolver<'a> { loop { // Since expanded macros may not shadow the lexical scope (enforced below), // we can ignore unresolved invocations (indicated by the penultimate argument). - match self.resolve_name_in_module(module, name, ns, true, record_used) { + match self.resolve_ident_in_module(module, ident, ns, true, record_used) { Ok(binding) => { let span = match record_used { Some(span) => span, @@ -286,6 +286,7 @@ impl<'a> Resolver<'a> { }; match potential_expanded_shadower { Some(shadower) if shadower.def() != binding.def() => { + let name = ident.name; self.ambiguity_errors.push(AmbiguityError { span: span, name: name, b1: shadower, b2: binding, lexical: true, }); @@ -383,10 +384,10 @@ impl<'a> Resolver<'a> { } } - for &(mark, name, span) in module.legacy_macro_resolutions.borrow().iter() { + for &(mark, ident, span) in module.legacy_macro_resolutions.borrow().iter() { let legacy_scope = &self.invocations[&mark].legacy_scope; - let legacy_resolution = self.resolve_legacy_scope(legacy_scope, name, true); - let resolution = self.resolve_lexical_macro_path_segment(name, MacroNS, Some(span)); + let legacy_resolution = self.resolve_legacy_scope(legacy_scope, ident.name, true); + let resolution = self.resolve_lexical_macro_path_segment(ident, MacroNS, Some(span)); let (legacy_resolution, resolution) = match (legacy_resolution, resolution) { (Some(legacy_resolution), Ok(resolution)) => (legacy_resolution, resolution), _ => continue, @@ -396,9 +397,9 @@ impl<'a> Resolver<'a> { MacroBinding::Modern(binding) => (binding.span, "imported"), MacroBinding::Legacy(binding) => (binding.span, "defined"), }; - let msg1 = format!("`{}` could resolve to the macro {} here", name, participle); - let msg2 = format!("`{}` could also resolve to the macro imported here", name); - self.session.struct_span_err(span, &format!("`{}` is ambiguous", name)) + let msg1 = format!("`{}` could resolve to the macro {} here", ident, participle); + let msg2 = format!("`{}` could also resolve to the macro imported here", ident); + self.session.struct_span_err(span, &format!("`{}` is ambiguous", ident)) .span_note(legacy_span, &msg1) .span_note(resolution.span, &msg2) .emit(); diff --git a/src/librustc_resolve/resolve_imports.rs b/src/librustc_resolve/resolve_imports.rs index 64a8e21f9e0..7c7908d2401 100644 --- a/src/librustc_resolve/resolve_imports.rs +++ b/src/librustc_resolve/resolve_imports.rs @@ -21,7 +21,7 @@ use rustc::ty; use rustc::lint::builtin::PRIVATE_IN_PUBLIC; use rustc::hir::def::*; -use syntax::ast::{Ident, NodeId, Name}; +use syntax::ast::{Ident, NodeId}; use syntax::ext::base::Determinacy::{self, Determined, Undetermined}; use syntax::ext::hygiene::Mark; use syntax::symbol::keywords; @@ -35,8 +35,8 @@ use std::mem; #[derive(Clone, Debug)] pub enum ImportDirectiveSubclass<'a> { SingleImport { - target: Name, - source: Name, + target: Ident, + source: Ident, result: PerNS, Determinacy>>>, }, GlobImport { @@ -126,31 +126,32 @@ impl<'a> NameResolution<'a> { } impl<'a> Resolver<'a> { - fn resolution(&self, module: Module<'a>, name: Name, ns: Namespace) + fn resolution(&self, module: Module<'a>, ident: Ident, ns: Namespace) -> &'a RefCell> { - *module.resolutions.borrow_mut().entry((name, ns)) + *module.resolutions.borrow_mut().entry((ident, ns)) .or_insert_with(|| self.arenas.alloc_name_resolution()) } - /// Attempts to resolve the supplied name in the given module for the given namespace. - /// If successful, returns the binding corresponding to the name. + /// Attempts to resolve `ident` in namespaces `ns` of `module`. /// Invariant: if `record_used` is `Some`, import resolution must be complete. - pub fn resolve_name_in_module(&mut self, - module: Module<'a>, - name: Name, - ns: Namespace, - ignore_unresolved_invocations: bool, - record_used: Option) - -> Result<&'a NameBinding<'a>, Determinacy> { + pub fn resolve_ident_in_module(&mut self, + module: Module<'a>, + ident: Ident, + ns: Namespace, + ignore_unresolved_invocations: bool, + record_used: Option) + -> Result<&'a NameBinding<'a>, Determinacy> { + let ident = ident.unhygienize(); self.populate_module_if_necessary(module); - let resolution = self.resolution(module, name, ns) + let resolution = self.resolution(module, ident, ns) .try_borrow_mut() .map_err(|_| Determined)?; // This happens when there is a cycle of imports if let Some(span) = record_used { if let Some(binding) = resolution.binding { if let Some(shadowed_glob) = resolution.shadows_glob { + let name = ident.name; // If we ignore unresolved invocations, we must forbid // expanded shadowing to avoid time travel. if ignore_unresolved_invocations && @@ -162,11 +163,11 @@ impl<'a> Resolver<'a> { }); } } - if self.record_use(name, ns, binding, span) { + if self.record_use(ident, ns, binding, span) { return Ok(self.dummy_binding); } if !self.is_accessible(binding.vis) { - self.privacy_errors.push(PrivacyError(span, name, binding)); + self.privacy_errors.push(PrivacyError(span, ident.name, binding)); } } @@ -194,11 +195,11 @@ impl<'a> Resolver<'a> { Some(module) => module, None => return Err(Undetermined), }; - let name = match directive.subclass { + let ident = match directive.subclass { SingleImport { source, .. } => source, _ => unreachable!(), }; - match self.resolve_name_in_module(module, name, ns, false, None) { + match self.resolve_ident_in_module(module, ident, ns, false, None) { Err(Determined) => {} _ => return Err(Undetermined), } @@ -220,7 +221,7 @@ impl<'a> Resolver<'a> { for directive in module.globs.borrow().iter() { if self.is_accessible(directive.vis.get()) { if let Some(module) = directive.imported_module.get() { - let result = self.resolve_name_in_module(module, name, ns, false, None); + let result = self.resolve_ident_in_module(module, ident, ns, false, None); if let Err(Undetermined) = result { return Err(Undetermined); } @@ -299,12 +300,13 @@ impl<'a> Resolver<'a> { } // Define the name or return the existing binding if there is a collision. - pub fn try_define(&mut self, module: Module<'a>, name: Name, ns: Namespace, binding: T) + pub fn try_define(&mut self, module: Module<'a>, ident: Ident, ns: Namespace, binding: T) -> Result<(), &'a NameBinding<'a>> where T: ToNameBinding<'a> { + let ident = ident.unhygienize(); let binding = self.arenas.alloc_name_binding(binding.to_name_binding()); - self.update_resolution(module, name, ns, |this, resolution| { + self.update_resolution(module, ident, ns, |this, resolution| { if let Some(old_binding) = resolution.binding { if binding.is_glob_import() { if !old_binding.is_glob_import() && @@ -347,13 +349,14 @@ impl<'a> Resolver<'a> { // Use `f` to mutate the resolution of the name in the module. // If the resolution becomes a success, define it in the module's glob importers. - fn update_resolution(&mut self, module: Module<'a>, name: Name, ns: Namespace, f: F) -> T + fn update_resolution(&mut self, module: Module<'a>, ident: Ident, ns: Namespace, f: F) + -> T where F: FnOnce(&mut Resolver<'a>, &mut NameResolution<'a>) -> T { // Ensure that `resolution` isn't borrowed when defining in the module's glob importers, // during which the resolution might end up getting re-defined via a glob cycle. let (binding, t) = { - let mut resolution = &mut *self.resolution(module, name, ns).borrow_mut(); + let mut resolution = &mut *self.resolution(module, ident, ns).borrow_mut(); let old_binding = resolution.binding(); let t = f(self, resolution); @@ -372,7 +375,7 @@ impl<'a> Resolver<'a> { for directive in module.glob_importers.borrow_mut().iter() { if self.is_accessible_from(binding.vis, directive.parent) { let imported_binding = self.import(binding, directive); - let _ = self.try_define(directive.parent, name, ns, imported_binding); + let _ = self.try_define(directive.parent, ident, ns, imported_binding); } } @@ -508,7 +511,7 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { let mut indeterminate = false; self.per_ns(|this, ns| { if let Err(Undetermined) = result[ns].get() { - result[ns].set(this.resolve_name_in_module(module, source, ns, false, None)); + result[ns].set(this.resolve_ident_in_module(module, source, ns, false, None)); } else { return }; @@ -564,7 +567,7 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { _ => return None, }; - let (name, result) = match directive.subclass { + let (ident, result) = match directive.subclass { SingleImport { source, ref result, .. } => (source, result), GlobImport { .. } if module.def_id() == directive.parent.def_id() => { // Importing a module into itself is not allowed. @@ -586,8 +589,8 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { self.per_ns(|this, ns| { if let Ok(binding) = result[ns].get() { all_ns_err = false; - if this.record_use(name, ns, binding, directive.span) { - this.resolution(module, name, ns).borrow_mut().binding = + if this.record_use(ident, ns, binding, directive.span) { + this.resolution(module, ident, ns).borrow_mut().binding = Some(this.dummy_binding); } } @@ -596,7 +599,7 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { if all_ns_err { let mut all_ns_failed = true; self.per_ns(|this, ns| { - match this.resolve_name_in_module(module, name, ns, false, Some(span)) { + match this.resolve_ident_in_module(module, ident, ns, false, Some(span)) { Ok(_) => all_ns_failed = false, _ => {} } @@ -604,27 +607,28 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { return if all_ns_failed { let resolutions = module.resolutions.borrow(); - let names = resolutions.iter().filter_map(|(&(ref n, _), resolution)| { - if *n == name { return None; } // Never suggest the same name + let names = resolutions.iter().filter_map(|(&(ref i, _), resolution)| { + if *i == ident { return None; } // Never suggest the same name match *resolution.borrow() { - NameResolution { binding: Some(_), .. } => Some(n), + NameResolution { binding: Some(_), .. } => Some(&i.name), NameResolution { single_imports: SingleImports::None, .. } => None, - _ => Some(n), + _ => Some(&i.name), } }); - let lev_suggestion = match find_best_match_for_name(names, &name.as_str(), None) { - Some(name) => format!(". Did you mean to use `{}`?", name), - None => "".to_owned(), - }; + let lev_suggestion = + match find_best_match_for_name(names, &ident.name.as_str(), None) { + Some(name) => format!(". Did you mean to use `{}`?", name), + None => "".to_owned(), + }; let module_str = module_to_string(module); let msg = if &module_str == "???" { - format!("no `{}` in the root{}", name, lev_suggestion) + format!("no `{}` in the root{}", ident, lev_suggestion) } else { - format!("no `{}` in `{}`{}", name, module_str, lev_suggestion) + format!("no `{}` in `{}`{}", ident, module_str, lev_suggestion) }; Some(msg) } else { - // `resolve_name_in_module` reported a privacy error. + // `resolve_ident_in_module` reported a privacy error. self.import_dummy_binding(directive); None } @@ -649,18 +653,18 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { if ns == TypeNS && binding.is_extern_crate() { let msg = format!("extern crate `{}` is private, and cannot be reexported \ (error E0364), consider declaring with `pub`", - name); + ident); self.session.add_lint(PRIVATE_IN_PUBLIC, directive.id, directive.span, msg); } else if ns == TypeNS { struct_span_err!(self.session, directive.span, E0365, - "`{}` is private, and cannot be reexported", name) - .span_label(directive.span, &format!("reexport of private `{}`", name)) - .note(&format!("consider declaring type or module `{}` with `pub`", name)) + "`{}` is private, and cannot be reexported", ident) + .span_label(directive.span, &format!("reexport of private `{}`", ident)) + .note(&format!("consider declaring type or module `{}` with `pub`", ident)) .emit(); } else { - let msg = format!("`{}` is private, and cannot be reexported", name); + let msg = format!("`{}` is private, and cannot be reexported", ident); let note_msg = - format!("consider marking `{}` as `pub` in the imported module", name); + format!("consider marking `{}` as `pub` in the imported module", ident); struct_span_err!(self.session, directive.span, E0364, "{}", &msg) .span_note(directive.span, ¬e_msg) .emit(); @@ -697,13 +701,13 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { // Ensure that `resolutions` isn't borrowed during `try_define`, // since it might get updated via a glob cycle. - let bindings = module.resolutions.borrow().iter().filter_map(|(name, resolution)| { - resolution.borrow().binding().map(|binding| (*name, binding)) + let bindings = module.resolutions.borrow().iter().filter_map(|(&ident, resolution)| { + resolution.borrow().binding().map(|binding| (ident, binding)) }).collect::>(); - for ((name, ns), binding) in bindings { + for ((ident, ns), binding) in bindings { if binding.pseudo_vis() == ty::Visibility::Public || self.is_accessible(binding.vis) { let imported_binding = self.import(binding, directive); - let _ = self.try_define(directive.parent, name, ns, imported_binding); + let _ = self.try_define(directive.parent, ident, ns, imported_binding); } } @@ -722,7 +726,7 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { reexports = mem::replace(&mut self.macro_exports, Vec::new()); } - for (&(name, ns), resolution) in module.resolutions.borrow().iter() { + for (&(ident, ns), resolution) in module.resolutions.borrow().iter() { let resolution = resolution.borrow(); let binding = match resolution.binding { Some(binding) => binding, @@ -736,7 +740,7 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { if !def.def_id().is_local() { self.session.cstore.export_macros(def.def_id().krate); } - reexports.push(Export { name: name, def: def }); + reexports.push(Export { name: ident.name, def: def }); } } @@ -745,7 +749,7 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { !orig_binding.vis.is_at_least(binding.vis, self) { let msg = format!("variant `{}` is private, and cannot be reexported \ (error E0364), consider declaring its enum as `pub`", - name); + ident); self.session.add_lint(PRIVATE_IN_PUBLIC, directive.id, binding.span, msg); } } diff --git a/src/libsyntax/ast.rs b/src/libsyntax/ast.rs index 2a911aceb9d..2369cc5714e 100644 --- a/src/libsyntax/ast.rs +++ b/src/libsyntax/ast.rs @@ -47,10 +47,14 @@ impl Ident { Ident { name: name, ctxt: SyntaxContext::empty() } } - /// Maps a string to an identifier with an empty syntax context. - pub fn from_str(s: &str) -> Ident { - Ident::with_empty_ctxt(Symbol::intern(s)) - } + /// Maps a string to an identifier with an empty syntax context. + pub fn from_str(s: &str) -> Ident { + Ident::with_empty_ctxt(Symbol::intern(s)) + } + + pub fn unhygienize(&self) -> Ident { + Ident { name: self.name, ctxt: SyntaxContext::empty() } + } } impl fmt::Debug for Ident { From aa19274b72f0f1f2a4481cdf9114e30c1b5f336d Mon Sep 17 00:00:00 2001 From: Jeffrey Seyfried Date: Tue, 29 Nov 2016 02:53:00 +0000 Subject: [PATCH 04/16] De-genericize `try_define`. --- src/librustc_resolve/build_reduced_graph.rs | 23 ++++++++-------- src/librustc_resolve/lib.rs | 6 ++--- src/librustc_resolve/resolve_imports.rs | 29 +++++++++++---------- 3 files changed, 30 insertions(+), 28 deletions(-) diff --git a/src/librustc_resolve/build_reduced_graph.rs b/src/librustc_resolve/build_reduced_graph.rs index 4aa04dc34fe..9401c77c7b8 100644 --- a/src/librustc_resolve/build_reduced_graph.rs +++ b/src/librustc_resolve/build_reduced_graph.rs @@ -16,7 +16,8 @@ use macros::{InvocationData, LegacyScope}; use resolve_imports::ImportDirective; use resolve_imports::ImportDirectiveSubclass::{self, GlobImport, SingleImport}; -use {Resolver, Module, ModuleS, ModuleKind, NameBinding, NameBindingKind, ToNameBinding}; +use {Module, ModuleS, ModuleKind, NameBinding, NameBindingKind, ToNameBinding}; +use {Resolver, ResolverArenas}; use Namespace::{self, TypeNS, ValueNS, MacroNS}; use {resolve_error, resolve_struct_error, ResolutionError}; @@ -45,24 +46,24 @@ use syntax::visit::{self, Visitor}; use syntax_pos::{Span, DUMMY_SP}; impl<'a> ToNameBinding<'a> for (Module<'a>, ty::Visibility, Span, Mark) { - fn to_name_binding(self) -> NameBinding<'a> { - NameBinding { + fn to_name_binding(self, arenas: &'a ResolverArenas<'a>) -> &'a NameBinding<'a> { + arenas.alloc_name_binding(NameBinding { kind: NameBindingKind::Module(self.0), vis: self.1, span: self.2, expansion: self.3, - } + }) } } impl<'a> ToNameBinding<'a> for (Def, ty::Visibility, Span, Mark) { - fn to_name_binding(self) -> NameBinding<'a> { - NameBinding { + fn to_name_binding(self, arenas: &'a ResolverArenas<'a>) -> &'a NameBinding<'a> { + arenas.alloc_name_binding(NameBinding { kind: NameBindingKind::Def(self.0), vis: self.1, span: self.2, expansion: self.3, - } + }) } } @@ -79,8 +80,8 @@ impl<'b> Resolver<'b> { fn define(&mut self, parent: Module<'b>, ident: Ident, ns: Namespace, def: T) where T: ToNameBinding<'b>, { - let binding = def.to_name_binding(); - if let Err(old_binding) = self.try_define(parent, ident, ns, binding.clone()) { + let binding = def.to_name_binding(self.arenas); + if let Err(old_binding) = self.try_define(parent, ident, ns, binding) { self.report_conflict(parent, ident, ns, old_binding, &binding); } } @@ -238,8 +239,8 @@ impl<'b> Resolver<'b> { // n.b. we don't need to look at the path option here, because cstore already did let crate_id = self.session.cstore.extern_mod_stmt_cnum(item.id).unwrap(); let module = self.get_extern_crate_root(crate_id); - let binding = (module, ty::Visibility::Public, sp, expansion).to_name_binding(); - let binding = self.arenas.alloc_name_binding(binding); + let binding = + (module, ty::Visibility::Public, sp, expansion).to_name_binding(self.arenas); let directive = self.arenas.alloc_import_directive(ImportDirective { id: item.id, parent: parent, diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index 4d4f4629c75..972fd8f570c 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -875,11 +875,11 @@ pub struct NameBinding<'a> { } pub trait ToNameBinding<'a> { - fn to_name_binding(self) -> NameBinding<'a>; + fn to_name_binding(self, arenas: &'a ResolverArenas<'a>) -> &'a NameBinding<'a>; } -impl<'a> ToNameBinding<'a> for NameBinding<'a> { - fn to_name_binding(self) -> NameBinding<'a> { +impl<'a> ToNameBinding<'a> for &'a NameBinding<'a> { + fn to_name_binding(self, _: &'a ResolverArenas<'a>) -> &'a NameBinding<'a> { self } } diff --git a/src/librustc_resolve/resolve_imports.rs b/src/librustc_resolve/resolve_imports.rs index 7c7908d2401..2c35d7ec442 100644 --- a/src/librustc_resolve/resolve_imports.rs +++ b/src/librustc_resolve/resolve_imports.rs @@ -12,7 +12,7 @@ use self::ImportDirectiveSubclass::*; use {AmbiguityError, Module, PerNS}; use Namespace::{self, TypeNS, MacroNS}; -use {NameBinding, NameBindingKind, PathResult, PathScope, PrivacyError, ToNameBinding}; +use {NameBinding, NameBindingKind, PathResult, PathScope, PrivacyError}; use Resolver; use {names_to_string, module_to_string}; use {resolve_error, ResolutionError}; @@ -273,7 +273,7 @@ impl<'a> Resolver<'a> { // Given a binding and an import directive that resolves to it, // return the corresponding binding defined by the import directive. pub fn import(&mut self, binding: &'a NameBinding<'a>, directive: &'a ImportDirective<'a>) - -> NameBinding<'a> { + -> &'a NameBinding<'a> { let vis = if binding.pseudo_vis().is_at_least(directive.vis.get(), self) || !directive.is_glob() && binding.is_extern_crate() { // c.f. `PRIVATE_IN_PUBLIC` directive.vis.get() @@ -287,7 +287,7 @@ impl<'a> Resolver<'a> { } } - NameBinding { + self.arenas.alloc_name_binding(NameBinding { kind: NameBindingKind::Import { binding: binding, directive: directive, @@ -296,16 +296,17 @@ impl<'a> Resolver<'a> { span: directive.span, vis: vis, expansion: directive.expansion, - } + }) } // Define the name or return the existing binding if there is a collision. - pub fn try_define(&mut self, module: Module<'a>, ident: Ident, ns: Namespace, binding: T) - -> Result<(), &'a NameBinding<'a>> - where T: ToNameBinding<'a> - { + pub fn try_define(&mut self, + module: Module<'a>, + ident: Ident, + ns: Namespace, + binding: &'a NameBinding<'a>) + -> Result<(), &'a NameBinding<'a>> { let ident = ident.unhygienize(); - let binding = self.arenas.alloc_name_binding(binding.to_name_binding()); self.update_resolution(module, ident, ns, |this, resolution| { if let Some(old_binding) = resolution.binding { if binding.is_glob_import() { @@ -389,7 +390,7 @@ impl<'a> Resolver<'a> { let dummy_binding = self.dummy_binding; let dummy_binding = self.import(dummy_binding, directive); self.per_ns(|this, ns| { - let _ = this.try_define(directive.parent, target, ns, dummy_binding.clone()); + let _ = this.try_define(directive.parent, target, ns, dummy_binding); }); } } @@ -516,10 +517,11 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { return }; + let parent = directive.parent; match result[ns].get() { Err(Undetermined) => indeterminate = true, Err(Determined) => { - this.update_resolution(directive.parent, target, ns, |_, resolution| { + this.update_resolution(parent, target, ns, |_, resolution| { resolution.single_imports.directive_failed() }); } @@ -534,10 +536,9 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { } Ok(binding) => { let imported_binding = this.import(binding, directive); - let conflict = this.try_define(directive.parent, target, ns, imported_binding); + let conflict = this.try_define(parent, target, ns, imported_binding); if let Err(old_binding) = conflict { - let binding = &this.import(binding, directive); - this.report_conflict(directive.parent, target, ns, binding, old_binding); + this.report_conflict(parent, target, ns, imported_binding, old_binding); } } } From 532b013b28adcaa6710f4ba263990debff50d860 Mon Sep 17 00:00:00 2001 From: Jeffrey Seyfried Date: Sat, 26 Nov 2016 12:47:52 +0000 Subject: [PATCH 05/16] Rename `ModuleS` -> `ModuleData`. --- src/librustc_resolve/build_reduced_graph.rs | 10 ++++---- src/librustc_resolve/lib.rs | 26 ++++++++++----------- 2 files changed, 18 insertions(+), 18 deletions(-) diff --git a/src/librustc_resolve/build_reduced_graph.rs b/src/librustc_resolve/build_reduced_graph.rs index 9401c77c7b8..2930e57422b 100644 --- a/src/librustc_resolve/build_reduced_graph.rs +++ b/src/librustc_resolve/build_reduced_graph.rs @@ -16,7 +16,7 @@ use macros::{InvocationData, LegacyScope}; use resolve_imports::ImportDirective; use resolve_imports::ImportDirectiveSubclass::{self, GlobImport, SingleImport}; -use {Module, ModuleS, ModuleKind, NameBinding, NameBindingKind, ToNameBinding}; +use {Module, ModuleData, ModuleKind, NameBinding, NameBindingKind, ToNameBinding}; use {Resolver, ResolverArenas}; use Namespace::{self, TypeNS, ValueNS, MacroNS}; use {resolve_error, resolve_struct_error, ResolutionError}; @@ -261,12 +261,12 @@ impl<'b> Resolver<'b> { ItemKind::Mod(..) => { let def = Def::Mod(self.definitions.local_def_id(item.id)); - let module = self.arenas.alloc_module(ModuleS { + let module = self.arenas.alloc_module(ModuleData { no_implicit_prelude: parent.no_implicit_prelude || { attr::contains_name(&item.attrs, "no_implicit_prelude") }, normal_ancestor_id: Some(item.id), - ..ModuleS::new(Some(parent), ModuleKind::Def(def, ident.name)) + ..ModuleData::new(Some(parent), ModuleKind::Def(def, ident.name)) }); self.define(parent, ident, TypeNS, (module, vis, sp, expansion)); self.module_map.insert(item.id, module); @@ -493,9 +493,9 @@ impl<'b> Resolver<'b> { let macros_only = self.session.cstore.dep_kind(cnum).macros_only(); let arenas = self.arenas; *self.extern_crate_roots.entry((cnum, macros_only)).or_insert_with(|| { - arenas.alloc_module(ModuleS { + arenas.alloc_module(ModuleData { populated: Cell::new(false), - ..ModuleS::new(None, ModuleKind::Def(Def::Mod(def_id), keywords::Invalid.name())) + ..ModuleData::new(None, ModuleKind::Def(Def::Mod(def_id), keywords::Invalid.name())) }) }) } diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index 972fd8f570c..d87e04f7b97 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -774,7 +774,7 @@ enum ModuleKind { } /// One node in the tree of modules. -pub struct ModuleS<'a> { +pub struct ModuleData<'a> { parent: Option>, kind: ModuleKind, @@ -802,11 +802,11 @@ pub struct ModuleS<'a> { populated: Cell, } -pub type Module<'a> = &'a ModuleS<'a>; +pub type Module<'a> = &'a ModuleData<'a>; -impl<'a> ModuleS<'a> { +impl<'a> ModuleData<'a> { fn new(parent: Option>, kind: ModuleKind) -> Self { - ModuleS { + ModuleData { parent: parent, kind: kind, normal_ancestor_id: None, @@ -859,7 +859,7 @@ impl<'a> ModuleS<'a> { } } -impl<'a> fmt::Debug for ModuleS<'a> { +impl<'a> fmt::Debug for ModuleData<'a> { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { write!(f, "{:?}", self.def()) } @@ -1116,7 +1116,7 @@ pub struct Resolver<'a> { } pub struct ResolverArenas<'a> { - modules: arena::TypedArena>, + modules: arena::TypedArena>, local_modules: RefCell>>, name_bindings: arena::TypedArena>, import_directives: arena::TypedArena>, @@ -1126,7 +1126,7 @@ pub struct ResolverArenas<'a> { } impl<'a> ResolverArenas<'a> { - fn alloc_module(&'a self, module: ModuleS<'a>) -> Module<'a> { + fn alloc_module(&'a self, module: ModuleData<'a>) -> Module<'a> { let module = self.modules.alloc(module); if module.def_id().map(|def_id| def_id.is_local()).unwrap_or(true) { self.local_modules.borrow_mut().push(module); @@ -1206,10 +1206,10 @@ impl<'a> Resolver<'a> { arenas: &'a ResolverArenas<'a>) -> Resolver<'a> { let root_def = Def::Mod(DefId::local(CRATE_DEF_INDEX)); - let graph_root = arenas.alloc_module(ModuleS { + let graph_root = arenas.alloc_module(ModuleData { normal_ancestor_id: Some(CRATE_NODE_ID), no_implicit_prelude: attr::contains_name(&krate.attrs, "no_implicit_prelude"), - ..ModuleS::new(None, ModuleKind::Def(root_def, keywords::Invalid.name())) + ..ModuleData::new(None, ModuleKind::Def(root_def, keywords::Invalid.name())) }); let mut module_map = NodeMap(); module_map.insert(CRATE_NODE_ID, graph_root); @@ -1327,17 +1327,17 @@ impl<'a> Resolver<'a> { } fn new_module(&self, parent: Module<'a>, kind: ModuleKind, local: bool) -> Module<'a> { - self.arenas.alloc_module(ModuleS { + self.arenas.alloc_module(ModuleData { normal_ancestor_id: if local { self.current_module.normal_ancestor_id } else { None }, populated: Cell::new(local), - ..ModuleS::new(Some(parent), kind) + ..ModuleData::new(Some(parent), kind) }) } fn record_use(&mut self, ident: Ident, ns: Namespace, binding: &'a NameBinding<'a>, span: Span) -> bool /* true if an error was reported */ { // track extern crates for unused_extern_crate lint - if let Some(DefId { krate, .. }) = binding.module().and_then(ModuleS::def_id) { + if let Some(DefId { krate, .. }) = binding.module().and_then(ModuleData::def_id) { self.used_crates.insert(krate); } @@ -2403,7 +2403,7 @@ impl<'a> Resolver<'a> { }); } } - let msg = if module.and_then(ModuleS::def) == self.graph_root.def() { + let msg = if module.and_then(ModuleData::def) == self.graph_root.def() { let is_mod = |def| match def { Def::Mod(..) => true, _ => false }; let mut candidates = self.lookup_candidates(ident.name, TypeNS, is_mod).candidates; From 4d638fd1131c3f909d6e328c20d88e361a444a99 Mon Sep 17 00:00:00 2001 From: Jeffrey Seyfried Date: Sat, 26 Nov 2016 12:48:46 +0000 Subject: [PATCH 06/16] Canonicalize lifetime names. --- src/librustc_resolve/build_reduced_graph.rs | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/librustc_resolve/build_reduced_graph.rs b/src/librustc_resolve/build_reduced_graph.rs index 2930e57422b..02a4d8db095 100644 --- a/src/librustc_resolve/build_reduced_graph.rs +++ b/src/librustc_resolve/build_reduced_graph.rs @@ -74,11 +74,11 @@ struct LegacyMacroImports { reexports: Vec<(Name, Span)>, } -impl<'b> Resolver<'b> { +impl<'a> Resolver<'a> { /// Defines `name` in namespace `ns` of module `parent` to be `def` if it is not yet defined; /// otherwise, reports an error. - fn define(&mut self, parent: Module<'b>, ident: Ident, ns: Namespace, def: T) - where T: ToNameBinding<'b>, + fn define(&mut self, parent: Module<'a>, ident: Ident, ns: Namespace, def: T) + where T: ToNameBinding<'a>, { let binding = def.to_name_binding(self.arenas); if let Err(old_binding) = self.try_define(parent, ident, ns, binding) { @@ -363,7 +363,7 @@ impl<'b> Resolver<'b> { // type and value namespaces. fn build_reduced_graph_for_variant(&mut self, variant: &Variant, - parent: Module<'b>, + parent: Module<'a>, vis: ty::Visibility, expansion: Mark) { let ident = variant.node.name; @@ -412,7 +412,7 @@ impl<'b> Resolver<'b> { } /// Builds the reduced graph for a single item in an external crate. - fn build_reduced_graph_for_external_crate_def(&mut self, parent: Module<'b>, child: Export) { + fn build_reduced_graph_for_external_crate_def(&mut self, parent: Module<'a>, child: Export) { let ident = Ident::with_empty_ctxt(child.name); let def = child.def; let def_id = def.def_id(); @@ -488,7 +488,7 @@ impl<'b> Resolver<'b> { } } - fn get_extern_crate_root(&mut self, cnum: CrateNum) -> Module<'b> { + fn get_extern_crate_root(&mut self, cnum: CrateNum) -> Module<'a> { let def_id = DefId { krate: cnum, index: CRATE_DEF_INDEX }; let macros_only = self.session.cstore.dep_kind(cnum).macros_only(); let arenas = self.arenas; @@ -531,7 +531,7 @@ impl<'b> Resolver<'b> { /// Ensures that the reduced graph rooted at the given external module /// is built, building it if it is not. - pub fn populate_module_if_necessary(&mut self, module: Module<'b>) { + pub fn populate_module_if_necessary(&mut self, module: Module<'a>) { if module.populated.get() { return } for child in self.session.cstore.item_children(module.def_id().unwrap()) { self.build_reduced_graph_for_external_crate_def(module, child); @@ -541,7 +541,7 @@ impl<'b> Resolver<'b> { fn legacy_import_macro(&mut self, name: Name, - binding: &'b NameBinding<'b>, + binding: &'a NameBinding<'a>, span: Span, allow_shadowing: bool) { self.used_crates.insert(binding.def().def_id().krate); @@ -554,7 +554,7 @@ impl<'b> Resolver<'b> { } } - fn process_legacy_macro_imports(&mut self, item: &Item, module: Module<'b>, expansion: Mark) { + fn process_legacy_macro_imports(&mut self, item: &Item, module: Module<'a>, expansion: Mark) { let allow_shadowing = expansion == Mark::root(); let legacy_imports = self.legacy_macro_imports(&item.attrs); let cnum = module.def_id().unwrap().krate; From e80d1a8faf2da8df494828e2772e2d2043282fed Mon Sep 17 00:00:00 2001 From: Jeffrey Seyfried Date: Tue, 29 Nov 2016 23:36:27 +0000 Subject: [PATCH 07/16] Remove `MacroDef`'s fields `imported_from` and `allow_internal_unstable`, remove `export` argument of `resolver.add_macro()`. --- src/librustc/hir/intravisit.rs | 1 - src/librustc/hir/lowering.rs | 2 -- src/librustc/hir/mod.rs | 2 -- src/librustc/middle/stability.rs | 8 ++------ src/librustc_metadata/cstore_impl.rs | 2 -- src/librustc_resolve/macros.rs | 5 +++-- src/librustdoc/visit_ast.rs | 5 +++-- src/libsyntax/ast.rs | 2 -- src/libsyntax/ext/base.rs | 4 ++-- src/libsyntax/ext/tt/macro_rules.rs | 7 ++----- src/libsyntax/visit.rs | 1 - 11 files changed, 12 insertions(+), 27 deletions(-) diff --git a/src/librustc/hir/intravisit.rs b/src/librustc/hir/intravisit.rs index 625bde2ca8b..186d6f62650 100644 --- a/src/librustc/hir/intravisit.rs +++ b/src/librustc/hir/intravisit.rs @@ -365,7 +365,6 @@ pub fn walk_crate<'v, V: Visitor<'v>>(visitor: &mut V, krate: &'v Crate) { pub fn walk_macro_def<'v, V: Visitor<'v>>(visitor: &mut V, macro_def: &'v MacroDef) { visitor.visit_id(macro_def.id); visitor.visit_name(macro_def.span, macro_def.name); - walk_opt_name(visitor, macro_def.span, macro_def.imported_from); walk_list!(visitor, visit_attribute, ¯o_def.attrs); } diff --git a/src/librustc/hir/lowering.rs b/src/librustc/hir/lowering.rs index 74876eb59ee..f5773d35178 100644 --- a/src/librustc/hir/lowering.rs +++ b/src/librustc/hir/lowering.rs @@ -987,8 +987,6 @@ impl<'a> LoweringContext<'a> { attrs: self.lower_attrs(&m.attrs), id: m.id, span: m.span, - imported_from: m.imported_from.map(|x| x.name), - allow_internal_unstable: m.allow_internal_unstable, body: m.body.clone().into(), } } diff --git a/src/librustc/hir/mod.rs b/src/librustc/hir/mod.rs index 4fd8f96ba04..f52ee35e175 100644 --- a/src/librustc/hir/mod.rs +++ b/src/librustc/hir/mod.rs @@ -475,8 +475,6 @@ pub struct MacroDef { pub attrs: HirVec, pub id: NodeId, pub span: Span, - pub imported_from: Option, - pub allow_internal_unstable: bool, pub body: HirVec, } diff --git a/src/librustc/middle/stability.rs b/src/librustc/middle/stability.rs index 3e32957aecf..f45e86f2f4b 100644 --- a/src/librustc/middle/stability.rs +++ b/src/librustc/middle/stability.rs @@ -302,9 +302,7 @@ impl<'a, 'tcx> Visitor<'tcx> for Annotator<'a, 'tcx> { } fn visit_macro_def(&mut self, md: &'tcx hir::MacroDef) { - if md.imported_from.is_none() { - self.annotate(md.id, &md.attrs, md.span, AnnotationKind::Required, |_| {}); - } + self.annotate(md.id, &md.attrs, md.span, AnnotationKind::Required, |_| {}); } } @@ -373,9 +371,7 @@ impl<'a, 'tcx> Visitor<'tcx> for MissingStabilityAnnotations<'a, 'tcx> { } fn visit_macro_def(&mut self, md: &'tcx hir::MacroDef) { - if md.imported_from.is_none() { - self.check_missing_stability(md.id, md.span); - } + self.check_missing_stability(md.id, md.span); } } diff --git a/src/librustc_metadata/cstore_impl.rs b/src/librustc_metadata/cstore_impl.rs index 1a1bb1432ee..ac830318ce9 100644 --- a/src/librustc_metadata/cstore_impl.rs +++ b/src/librustc_metadata/cstore_impl.rs @@ -418,8 +418,6 @@ impl<'tcx> CrateStore<'tcx> for cstore::CStore { ident: ast::Ident::with_empty_ctxt(name), id: ast::DUMMY_NODE_ID, span: local_span, - imported_from: None, // FIXME - allow_internal_unstable: attr::contains_name(&attrs, "allow_internal_unstable"), attrs: attrs, body: body, }) diff --git a/src/librustc_resolve/macros.rs b/src/librustc_resolve/macros.rs index b2e31d54909..5e356878ba8 100644 --- a/src/librustc_resolve/macros.rs +++ b/src/librustc_resolve/macros.rs @@ -20,6 +20,7 @@ use rustc::ty; use std::cell::Cell; use std::rc::Rc; use syntax::ast::{self, Name, Ident}; +use syntax::attr; use syntax::errors::DiagnosticBuilder; use syntax::ext::base::{self, Determinacy, MultiModifier, MultiDecorator}; use syntax::ext::base::{NormalTT, SyntaxExtension}; @@ -138,7 +139,7 @@ impl<'a> base::Resolver for Resolver<'a> { invocation.expansion.set(visitor.legacy_scope); } - fn add_macro(&mut self, scope: Mark, mut def: ast::MacroDef, export: bool) { + fn add_macro(&mut self, scope: Mark, mut def: ast::MacroDef) { if def.ident.name == "macro_rules" { self.session.span_err(def.span, "user-defined macros may not be named `macro_rules`"); } @@ -153,7 +154,7 @@ impl<'a> base::Resolver for Resolver<'a> { invocation.legacy_scope.set(LegacyScope::Binding(binding)); self.macro_names.insert(def.ident.name); - if export { + if attr::contains_name(&def.attrs, "macro_export") { def.id = self.next_node_id(); DefCollector::new(&mut self.definitions).with_parent(CRATE_DEF_INDEX, |collector| { collector.visit_macro_def(&def) diff --git a/src/librustdoc/visit_ast.rs b/src/librustdoc/visit_ast.rs index 4087b9a761f..e5410c6341f 100644 --- a/src/librustdoc/visit_ast.rs +++ b/src/librustdoc/visit_ast.rs @@ -201,6 +201,7 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> { if def_id.krate == LOCAL_CRATE { continue // These are `krate.exported_macros`, handled in `self.visit()`. } + let imported_from = self.cx.sess().cstore.original_crate_name(def_id.krate); let def = match self.cx.sess().cstore.load_macro(def_id, self.cx.sess()) { LoadedMacro::MacroRules(macro_rules) => macro_rules, // FIXME(jseyfried): document proc macro reexports @@ -217,7 +218,7 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> { matchers: matchers, stab: self.stability(def.id), depr: self.deprecation(def.id), - imported_from: def.imported_from.map(|ident| ident.name), + imported_from: Some(imported_from), }) } } @@ -525,7 +526,7 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> { matchers: matchers, stab: self.stability(def.id), depr: self.deprecation(def.id), - imported_from: def.imported_from, + imported_from: None, } } } diff --git a/src/libsyntax/ast.rs b/src/libsyntax/ast.rs index 2369cc5714e..39d78cd8776 100644 --- a/src/libsyntax/ast.rs +++ b/src/libsyntax/ast.rs @@ -1972,8 +1972,6 @@ pub struct MacroDef { pub attrs: Vec, pub id: NodeId, pub span: Span, - pub imported_from: Option, - pub allow_internal_unstable: bool, pub body: Vec, } diff --git a/src/libsyntax/ext/base.rs b/src/libsyntax/ext/base.rs index ddbca47429d..508c5eaed8c 100644 --- a/src/libsyntax/ext/base.rs +++ b/src/libsyntax/ext/base.rs @@ -520,7 +520,7 @@ pub trait Resolver { fn eliminate_crate_var(&mut self, item: P) -> P; fn visit_expansion(&mut self, mark: Mark, expansion: &Expansion); - fn add_macro(&mut self, scope: Mark, def: ast::MacroDef, export: bool); + fn add_macro(&mut self, scope: Mark, def: ast::MacroDef); fn add_ext(&mut self, ident: ast::Ident, ext: Rc); fn add_expansions_at_stmt(&mut self, id: ast::NodeId, macros: Vec); @@ -544,7 +544,7 @@ impl Resolver for DummyResolver { fn eliminate_crate_var(&mut self, item: P) -> P { item } fn visit_expansion(&mut self, _invoc: Mark, _expansion: &Expansion) {} - fn add_macro(&mut self, _scope: Mark, _def: ast::MacroDef, _export: bool) {} + fn add_macro(&mut self, _scope: Mark, _def: ast::MacroDef) {} fn add_ext(&mut self, _ident: ast::Ident, _ext: Rc) {} fn add_expansions_at_stmt(&mut self, _id: ast::NodeId, _macros: Vec) {} diff --git a/src/libsyntax/ext/tt/macro_rules.rs b/src/libsyntax/ext/tt/macro_rules.rs index ca18e580ecd..5a028594a21 100644 --- a/src/libsyntax/ext/tt/macro_rules.rs +++ b/src/libsyntax/ext/tt/macro_rules.rs @@ -160,14 +160,11 @@ impl IdentMacroExpander for MacroRulesExpander { tts: Vec, attrs: Vec) -> Box { - let export = attr::contains_name(&attrs, "macro_export"); let def = ast::MacroDef { ident: ident, id: ast::DUMMY_NODE_ID, span: span, - imported_from: None, body: tts, - allow_internal_unstable: attr::contains_name(&attrs, "allow_internal_unstable"), attrs: attrs, }; @@ -178,7 +175,7 @@ impl IdentMacroExpander for MacroRulesExpander { MacEager::items(placeholders::macro_scope_placeholder().make_items()) }; - cx.resolver.add_macro(cx.current_expansion.mark, def, export); + cx.resolver.add_macro(cx.current_expansion.mark, def); result } } @@ -282,7 +279,7 @@ pub fn compile(sess: &ParseSess, def: &ast::MacroDef) -> SyntaxExtension { valid: valid, }); - NormalTT(exp, Some(def.span), def.allow_internal_unstable) + NormalTT(exp, Some(def.span), attr::contains_name(&def.attrs, "allow_internal_unstable")) } fn check_lhs_nt_follows(sess: &ParseSess, lhs: &TokenTree) -> bool { diff --git a/src/libsyntax/visit.rs b/src/libsyntax/visit.rs index 3e0353d532d..c1391d0b1c2 100644 --- a/src/libsyntax/visit.rs +++ b/src/libsyntax/visit.rs @@ -178,7 +178,6 @@ pub fn walk_crate<'a, V: Visitor<'a>>(visitor: &mut V, krate: &'a Crate) { pub fn walk_macro_def<'a, V: Visitor<'a>>(visitor: &mut V, macro_def: &'a MacroDef) { visitor.visit_ident(macro_def.span, macro_def.ident); - walk_opt_ident(visitor, macro_def.span, macro_def.imported_from); walk_list!(visitor, visit_attribute, ¯o_def.attrs); } From 421c5d11c1b4bb591bb429577c7b89cba59acefa Mon Sep 17 00:00:00 2001 From: Jeffrey Seyfried Date: Thu, 1 Dec 2016 11:20:04 +0000 Subject: [PATCH 08/16] Remove scope placeholders, remove method `add_macro` of `ext::base::Resolver`. --- src/librustc_resolve/build_reduced_graph.rs | 10 ++- src/librustc_resolve/macros.rs | 75 ++++++++++++--------- src/libsyntax/ext/base.rs | 2 - src/libsyntax/ext/expand.rs | 37 +++++----- src/libsyntax/ext/placeholders.rs | 50 ++++---------- src/libsyntax/ext/tt/macro_rules.rs | 34 +--------- src/libsyntax_ext/lib.rs | 5 +- 7 files changed, 91 insertions(+), 122 deletions(-) diff --git a/src/librustc_resolve/build_reduced_graph.rs b/src/librustc_resolve/build_reduced_graph.rs index 02a4d8db095..9f85580b93e 100644 --- a/src/librustc_resolve/build_reduced_graph.rs +++ b/src/librustc_resolve/build_reduced_graph.rs @@ -697,9 +697,13 @@ impl<'a, 'b> Visitor<'a> for BuildReducedGraphVisitor<'a, 'b> { fn visit_item(&mut self, item: &'a Item) { let macro_use = match item.node { - ItemKind::Mac(..) if item.id == ast::DUMMY_NODE_ID => return, // Scope placeholder - ItemKind::Mac(..) => { - return self.legacy_scope = LegacyScope::Expansion(self.visit_invoc(item.id)); + ItemKind::Mac(ref mac) => { + if mac.node.path.segments.is_empty() { + self.legacy_scope = LegacyScope::Expansion(self.visit_invoc(item.id)); + } else { + self.resolver.define_macro(item, &mut self.legacy_scope); + } + return } ItemKind::Mod(..) => self.resolver.contains_macro_use(&item.attrs), _ => false, diff --git a/src/librustc_resolve/macros.rs b/src/librustc_resolve/macros.rs index 5e356878ba8..ce92a4446f9 100644 --- a/src/librustc_resolve/macros.rs +++ b/src/librustc_resolve/macros.rs @@ -23,8 +23,8 @@ use syntax::ast::{self, Name, Ident}; use syntax::attr; use syntax::errors::DiagnosticBuilder; use syntax::ext::base::{self, Determinacy, MultiModifier, MultiDecorator}; -use syntax::ext::base::{NormalTT, SyntaxExtension}; -use syntax::ext::expand::Expansion; +use syntax::ext::base::{NormalTT, Resolver as SyntaxResolver, SyntaxExtension}; +use syntax::ext::expand::{Expansion, mark_tts}; use syntax::ext::hygiene::Mark; use syntax::ext::tt::macro_rules; use syntax::feature_gate::{emit_feature_err, GateIssue}; @@ -139,34 +139,6 @@ impl<'a> base::Resolver for Resolver<'a> { invocation.expansion.set(visitor.legacy_scope); } - fn add_macro(&mut self, scope: Mark, mut def: ast::MacroDef) { - if def.ident.name == "macro_rules" { - self.session.span_err(def.span, "user-defined macros may not be named `macro_rules`"); - } - - let invocation = self.invocations[&scope]; - let binding = self.arenas.alloc_legacy_binding(LegacyBinding { - parent: Cell::new(invocation.legacy_scope.get()), - name: def.ident.name, - ext: Rc::new(macro_rules::compile(&self.session.parse_sess, &def)), - span: def.span, - }); - invocation.legacy_scope.set(LegacyScope::Binding(binding)); - self.macro_names.insert(def.ident.name); - - if attr::contains_name(&def.attrs, "macro_export") { - def.id = self.next_node_id(); - DefCollector::new(&mut self.definitions).with_parent(CRATE_DEF_INDEX, |collector| { - collector.visit_macro_def(&def) - }); - self.macro_exports.push(Export { - name: def.ident.name, - def: Def::Macro(self.definitions.local_def_id(def.id)), - }); - self.exported_macros.push(def); - } - } - fn add_ext(&mut self, ident: ast::Ident, ext: Rc) { if let NormalTT(..) = *ext { self.macro_names.insert(ident.name); @@ -444,4 +416,47 @@ impl<'a> Resolver<'a> { expansion.visit_with(def_collector) }); } + + pub fn define_macro(&mut self, item: &ast::Item, legacy_scope: &mut LegacyScope<'a>) { + let tts = match item.node { + ast::ItemKind::Mac(ref mac) => &mac.node.tts, + _ => unreachable!(), + }; + + if item.ident.name == "macro_rules" { + self.session.span_err(item.span, "user-defined macros may not be named `macro_rules`"); + } + + let mark = Mark::from_placeholder_id(item.id); + let invocation = self.invocations[&mark]; + invocation.module.set(self.current_module); + + let mut def = ast::MacroDef { + ident: item.ident, + attrs: item.attrs.clone(), + id: ast::DUMMY_NODE_ID, + span: item.span, + body: mark_tts(tts, mark), + }; + + *legacy_scope = LegacyScope::Binding(self.arenas.alloc_legacy_binding(LegacyBinding { + parent: Cell::new(*legacy_scope), + name: def.ident.name, + ext: Rc::new(macro_rules::compile(&self.session.parse_sess, &def)), + span: def.span, + })); + self.macro_names.insert(def.ident.name); + + if attr::contains_name(&def.attrs, "macro_export") { + def.id = self.next_node_id(); + DefCollector::new(&mut self.definitions).with_parent(CRATE_DEF_INDEX, |collector| { + collector.visit_macro_def(&def) + }); + self.macro_exports.push(Export { + name: def.ident.name, + def: Def::Macro(self.definitions.local_def_id(def.id)), + }); + self.exported_macros.push(def); + } + } } diff --git a/src/libsyntax/ext/base.rs b/src/libsyntax/ext/base.rs index 508c5eaed8c..f9364f39ab7 100644 --- a/src/libsyntax/ext/base.rs +++ b/src/libsyntax/ext/base.rs @@ -520,7 +520,6 @@ pub trait Resolver { fn eliminate_crate_var(&mut self, item: P) -> P; fn visit_expansion(&mut self, mark: Mark, expansion: &Expansion); - fn add_macro(&mut self, scope: Mark, def: ast::MacroDef); fn add_ext(&mut self, ident: ast::Ident, ext: Rc); fn add_expansions_at_stmt(&mut self, id: ast::NodeId, macros: Vec); @@ -544,7 +543,6 @@ impl Resolver for DummyResolver { fn eliminate_crate_var(&mut self, item: P) -> P { item } fn visit_expansion(&mut self, _invoc: Mark, _expansion: &Expansion) {} - fn add_macro(&mut self, _scope: Mark, _def: ast::MacroDef) {} fn add_ext(&mut self, _ident: ast::Ident, _ext: Rc) {} fn add_expansions_at_stmt(&mut self, _id: ast::NodeId, _macros: Vec) {} diff --git a/src/libsyntax/ext/expand.rs b/src/libsyntax/ext/expand.rs index 19545e2e642..05501b5434a 100644 --- a/src/libsyntax/ext/expand.rs +++ b/src/libsyntax/ext/expand.rs @@ -392,14 +392,6 @@ impl<'a, 'b> MacroExpander<'a, 'b> { }; let Mac_ { path, tts, .. } = mac.node; - // Detect use of feature-gated or invalid attributes on macro invoations - // since they will not be detected after macro expansion. - for attr in attrs.iter() { - feature_gate::check_attribute(&attr, &self.cx.parse_sess, - &self.cx.parse_sess.codemap(), - &self.cx.ecfg.features.unwrap()); - } - let extname = path.segments.last().unwrap().identifier.name; let ident = ident.unwrap_or(keywords::Invalid.ident()); let marked_tts = mark_tts(&tts, mark); @@ -601,6 +593,7 @@ impl<'a, 'b> InvocationCollector<'a, 'b> { fn collect_bang( &mut self, mac: ast::Mac, attrs: Vec, span: Span, kind: ExpansionKind, ) -> Expansion { + self.check_attributes(&attrs); self.collect(kind, InvocationKind::Bang { attrs: attrs, mac: mac, ident: None, span: span }) } @@ -622,6 +615,16 @@ impl<'a, 'b> InvocationCollector<'a, 'b> { fn configure(&mut self, node: T) -> Option { self.cfg.configure(node) } + + // Detect use of feature-gated or invalid attributes on macro invocations + // since they will not be detected after macro expansion. + fn check_attributes(&mut self, attrs: &[ast::Attribute]) { + let codemap = &self.cx.parse_sess.codemap(); + let features = self.cx.ecfg.features.unwrap(); + for attr in attrs.iter() { + feature_gate::check_attribute(&attr, &self.cx.parse_sess, codemap, features); + } + } } // These are pretty nasty. Ideally, we would keep the tokens around, linked from @@ -740,14 +743,18 @@ impl<'a, 'b> Folder for InvocationCollector<'a, 'b> { match item.node { ast::ItemKind::Mac(..) => { - if match item.node { - ItemKind::Mac(ref mac) => mac.node.path.segments.is_empty(), - _ => unreachable!(), - } { - return SmallVector::one(item); - } + self.check_attributes(&item.attrs); + let is_macro_def = if let ItemKind::Mac(ref mac) = item.node { + mac.node.path.segments[0].identifier.name == "macro_rules" + } else { + unreachable!() + }; - item.and_then(|item| match item.node { + item.and_then(|mut item| match item.node { + ItemKind::Mac(_) if is_macro_def => { + item.id = ast::NodeId::from_u32(Mark::fresh().as_u32()); + SmallVector::one(P(item)) + } ItemKind::Mac(mac) => { self.collect(ExpansionKind::Items, InvocationKind::Bang { mac: mac, diff --git a/src/libsyntax/ext/placeholders.rs b/src/libsyntax/ext/placeholders.rs index 4fe57a8345e..eb4b6144c8d 100644 --- a/src/libsyntax/ext/placeholders.rs +++ b/src/libsyntax/ext/placeholders.rs @@ -12,9 +12,10 @@ use ast; use codemap::{DUMMY_SP, dummy_spanned}; use ext::base::ExtCtxt; use ext::expand::{Expansion, ExpansionKind}; +use ext::hygiene::Mark; use fold::*; use ptr::P; -use symbol::{Symbol, keywords}; +use symbol::keywords; use util::move_map::MoveMap; use util::small_vector::SmallVector; @@ -68,10 +69,6 @@ pub fn placeholder(kind: ExpansionKind, id: ast::NodeId) -> Expansion { } } -pub fn macro_scope_placeholder() -> Expansion { - placeholder(ExpansionKind::Items, ast::DUMMY_NODE_ID) -} - pub struct PlaceholderExpander<'a, 'b: 'a> { expansions: HashMap, cx: &'a mut ExtCtxt<'b>, @@ -100,11 +97,12 @@ impl<'a, 'b> PlaceholderExpander<'a, 'b> { impl<'a, 'b> Folder for PlaceholderExpander<'a, 'b> { fn fold_item(&mut self, item: P) -> SmallVector> { match item.node { - // Scope placeholder - ast::ItemKind::Mac(_) if item.id == ast::DUMMY_NODE_ID => SmallVector::one(item), - ast::ItemKind::Mac(_) => self.remove(item.id).make_items(), - _ => noop_fold_item(item, self), + ast::ItemKind::Mac(ref mac) if !mac.node.path.segments.is_empty() => {} + ast::ItemKind::Mac(_) => return self.remove(item.id).make_items(), + _ => {} } + + noop_fold_item(item, self) } fn fold_trait_item(&mut self, item: ast::TraitItem) -> SmallVector { @@ -172,10 +170,10 @@ impl<'a, 'b> Folder for PlaceholderExpander<'a, 'b> { block.stmts = block.stmts.move_flat_map(|mut stmt| { remaining_stmts -= 1; - // Scope placeholder + // `macro_rules!` macro definition if let ast::StmtKind::Item(ref item) = stmt.node { - if let ast::ItemKind::Mac(..) = item.node { - macros.push(item.ident.ctxt.data().outer_mark); + if let ast::ItemKind::Mac(_) = item.node { + macros.push(Mark::from_placeholder_id(item.id)); return None; } } @@ -208,33 +206,13 @@ impl<'a, 'b> Folder for PlaceholderExpander<'a, 'b> { fn fold_mod(&mut self, module: ast::Mod) -> ast::Mod { let mut module = noop_fold_mod(module, self); module.items = module.items.move_flat_map(|item| match item.node { - ast::ItemKind::Mac(_) => None, // remove scope placeholders from modules + ast::ItemKind::Mac(_) if !self.cx.ecfg.keep_macs => None, // remove macro definitions _ => Some(item), }); module } -} -pub fn reconstructed_macro_rules(def: &ast::MacroDef) -> Expansion { - Expansion::Items(SmallVector::one(P(ast::Item { - ident: def.ident, - attrs: def.attrs.clone(), - id: ast::DUMMY_NODE_ID, - node: ast::ItemKind::Mac(ast::Mac { - span: def.span, - node: ast::Mac_ { - path: ast::Path { - span: DUMMY_SP, - global: false, - segments: vec![ast::PathSegment { - identifier: ast::Ident::with_empty_ctxt(Symbol::intern("macro_rules")), - parameters: ast::PathParameters::none(), - }], - }, - tts: def.body.clone(), - } - }), - vis: ast::Visibility::Inherited, - span: def.span, - }))) + fn fold_mac(&mut self, mac: ast::Mac) -> ast::Mac { + mac + } } diff --git a/src/libsyntax/ext/tt/macro_rules.rs b/src/libsyntax/ext/tt/macro_rules.rs index 5a028594a21..3abd24b50ba 100644 --- a/src/libsyntax/ext/tt/macro_rules.rs +++ b/src/libsyntax/ext/tt/macro_rules.rs @@ -10,10 +10,9 @@ use {ast, attr}; use syntax_pos::{Span, DUMMY_SP}; -use ext::base::{DummyResult, ExtCtxt, MacEager, MacResult, SyntaxExtension}; -use ext::base::{IdentMacroExpander, NormalTT, TTMacroExpander}; +use ext::base::{DummyResult, ExtCtxt, MacResult, SyntaxExtension}; +use ext::base::{NormalTT, TTMacroExpander}; use ext::expand::{Expansion, ExpansionKind}; -use ext::placeholders; use ext::tt::macro_parser::{Success, Error, Failure}; use ext::tt::macro_parser::{MatchedSeq, MatchedNonterminal}; use ext::tt::macro_parser::{parse, parse_failure_msg}; @@ -151,35 +150,6 @@ fn generic_extension<'cx>(cx: &'cx ExtCtxt, cx.span_fatal(best_fail_spot.substitute_dummy(sp), &best_fail_msg); } -pub struct MacroRulesExpander; -impl IdentMacroExpander for MacroRulesExpander { - fn expand(&self, - cx: &mut ExtCtxt, - span: Span, - ident: ast::Ident, - tts: Vec, - attrs: Vec) - -> Box { - let def = ast::MacroDef { - ident: ident, - id: ast::DUMMY_NODE_ID, - span: span, - body: tts, - attrs: attrs, - }; - - // If keep_macs is true, expands to a MacEager::items instead. - let result = if cx.ecfg.keep_macs { - MacEager::items(placeholders::reconstructed_macro_rules(&def).make_items()) - } else { - MacEager::items(placeholders::macro_scope_placeholder().make_items()) - }; - - cx.resolver.add_macro(cx.current_expansion.mark, def); - result - } -} - // Note that macro-by-example's input is also matched against a token tree: // $( $lhs:tt => $rhs:tt );+ // diff --git a/src/libsyntax_ext/lib.rs b/src/libsyntax_ext/lib.rs index 66d6c0570ac..e31b29d5cc1 100644 --- a/src/libsyntax_ext/lib.rs +++ b/src/libsyntax_ext/lib.rs @@ -50,8 +50,7 @@ pub mod deriving; use std::rc::Rc; use syntax::ast; -use syntax::ext::base::{MacroExpanderFn, NormalTT, IdentTT, MultiModifier, NamedSyntaxExtension}; -use syntax::ext::tt::macro_rules::MacroRulesExpander; +use syntax::ext::base::{MacroExpanderFn, NormalTT, MultiModifier, NamedSyntaxExtension}; use syntax::symbol::Symbol; pub fn register_builtins(resolver: &mut syntax::ext::base::Resolver, @@ -61,8 +60,6 @@ pub fn register_builtins(resolver: &mut syntax::ext::base::Resolver, resolver.add_ext(ast::Ident::with_empty_ctxt(name), Rc::new(ext)); }; - register(Symbol::intern("macro_rules"), IdentTT(Box::new(MacroRulesExpander), None, false)); - macro_rules! register { ($( $name:ident: $f:expr, )*) => { $( register(Symbol::intern(stringify!($name)), From 6f040b48ef3f8bce0706d838f5672ca8a3e07880 Mon Sep 17 00:00:00 2001 From: Jeffrey Seyfried Date: Thu, 1 Dec 2016 11:54:01 +0000 Subject: [PATCH 09/16] Avoid including attributes in bang macro invocations. --- src/libsyntax/ext/base.rs | 6 ++---- src/libsyntax/ext/expand.rs | 38 +++++++++++++++++-------------------- 2 files changed, 19 insertions(+), 25 deletions(-) diff --git a/src/libsyntax/ext/base.rs b/src/libsyntax/ext/base.rs index f9364f39ab7..8e63f73fdaa 100644 --- a/src/libsyntax/ext/base.rs +++ b/src/libsyntax/ext/base.rs @@ -217,8 +217,7 @@ pub trait IdentMacroExpander { cx: &'cx mut ExtCtxt, sp: Span, ident: ast::Ident, - token_tree: Vec, - attrs: Vec) + token_tree: Vec) -> Box; } @@ -234,8 +233,7 @@ impl IdentMacroExpander for F cx: &'cx mut ExtCtxt, sp: Span, ident: ast::Ident, - token_tree: Vec, - _attrs: Vec) + token_tree: Vec) -> Box { (*self)(cx, sp, ident, token_tree) diff --git a/src/libsyntax/ext/expand.rs b/src/libsyntax/ext/expand.rs index 05501b5434a..e6782884f38 100644 --- a/src/libsyntax/ext/expand.rs +++ b/src/libsyntax/ext/expand.rs @@ -158,7 +158,6 @@ pub struct Invocation { pub enum InvocationKind { Bang { - attrs: Vec, mac: ast::Mac, ident: Option, span: Span, @@ -386,8 +385,8 @@ impl<'a, 'b> MacroExpander<'a, 'b> { /// Expand a macro invocation. Returns the result of expansion. fn expand_bang_invoc(&mut self, invoc: Invocation, ext: Rc) -> Expansion { let (mark, kind) = (invoc.expansion_data.mark, invoc.expansion_kind); - let (attrs, mac, ident, span) = match invoc.kind { - InvocationKind::Bang { attrs, mac, ident, span } => (attrs, mac, ident, span), + let (mac, ident, span) = match invoc.kind { + InvocationKind::Bang { mac, ident, span } => (mac, ident, span), _ => unreachable!(), }; let Mac_ { path, tts, .. } = mac.node; @@ -432,7 +431,7 @@ impl<'a, 'b> MacroExpander<'a, 'b> { } }); - kind.make_from(expander.expand(self.cx, span, ident, marked_tts, attrs)) + kind.make_from(expander.expand(self.cx, span, ident, marked_tts)) } MultiDecorator(..) | MultiModifier(..) | SyntaxExtension::AttrProcMacro(..) => { @@ -590,11 +589,8 @@ impl<'a, 'b> InvocationCollector<'a, 'b> { placeholder(expansion_kind, ast::NodeId::from_u32(mark.as_u32())) } - fn collect_bang( - &mut self, mac: ast::Mac, attrs: Vec, span: Span, kind: ExpansionKind, - ) -> Expansion { - self.check_attributes(&attrs); - self.collect(kind, InvocationKind::Bang { attrs: attrs, mac: mac, ident: None, span: span }) + fn collect_bang(&mut self, mac: ast::Mac, span: Span, kind: ExpansionKind) -> Expansion { + self.collect(kind, InvocationKind::Bang { mac: mac, ident: None, span: span }) } fn collect_attr(&mut self, attr: ast::Attribute, item: Annotatable, kind: ExpansionKind) @@ -663,7 +659,8 @@ impl<'a, 'b> Folder for InvocationCollector<'a, 'b> { expr.node = self.cfg.configure_expr_kind(expr.node); if let ast::ExprKind::Mac(mac) = expr.node { - self.collect_bang(mac, expr.attrs.into(), expr.span, ExpansionKind::Expr).make_expr() + self.check_attributes(&expr.attrs); + self.collect_bang(mac, expr.span, ExpansionKind::Expr).make_expr() } else { P(noop_fold_expr(expr, self)) } @@ -674,8 +671,8 @@ impl<'a, 'b> Folder for InvocationCollector<'a, 'b> { expr.node = self.cfg.configure_expr_kind(expr.node); if let ast::ExprKind::Mac(mac) = expr.node { - self.collect_bang(mac, expr.attrs.into(), expr.span, ExpansionKind::OptExpr) - .make_opt_expr() + self.check_attributes(&expr.attrs); + self.collect_bang(mac, expr.span, ExpansionKind::OptExpr).make_opt_expr() } else { Some(P(noop_fold_expr(expr, self))) } @@ -688,8 +685,7 @@ impl<'a, 'b> Folder for InvocationCollector<'a, 'b> { } pat.and_then(|pat| match pat.node { - PatKind::Mac(mac) => - self.collect_bang(mac, Vec::new(), pat.span, ExpansionKind::Pat).make_pat(), + PatKind::Mac(mac) => self.collect_bang(mac, pat.span, ExpansionKind::Pat).make_pat(), _ => unreachable!(), }) } @@ -710,8 +706,8 @@ impl<'a, 'b> Folder for InvocationCollector<'a, 'b> { }).collect() }; - let mut placeholder = - self.collect_bang(mac, attrs.into(), stmt.span, ExpansionKind::Stmts).make_stmts(); + self.check_attributes(&attrs); + let mut placeholder = self.collect_bang(mac, stmt.span, ExpansionKind::Stmts).make_stmts(); // If this is a macro invocation with a semicolon, then apply that // semicolon to the final statement produced by expansion. @@ -758,7 +754,6 @@ impl<'a, 'b> Folder for InvocationCollector<'a, 'b> { ItemKind::Mac(mac) => { self.collect(ExpansionKind::Items, InvocationKind::Bang { mac: mac, - attrs: item.attrs, ident: Some(item.ident), span: item.span, }).make_items() @@ -830,7 +825,8 @@ impl<'a, 'b> Folder for InvocationCollector<'a, 'b> { match item.node { ast::TraitItemKind::Macro(mac) => { let ast::TraitItem { attrs, span, .. } = item; - self.collect_bang(mac, attrs, span, ExpansionKind::TraitItems).make_trait_items() + self.check_attributes(&attrs); + self.collect_bang(mac, span, ExpansionKind::TraitItems).make_trait_items() } _ => fold::noop_fold_trait_item(item, self), } @@ -848,7 +844,8 @@ impl<'a, 'b> Folder for InvocationCollector<'a, 'b> { match item.node { ast::ImplItemKind::Macro(mac) => { let ast::ImplItem { attrs, span, .. } = item; - self.collect_bang(mac, attrs, span, ExpansionKind::ImplItems).make_impl_items() + self.check_attributes(&attrs); + self.collect_bang(mac, span, ExpansionKind::ImplItems).make_impl_items() } _ => fold::noop_fold_impl_item(item, self), } @@ -861,8 +858,7 @@ impl<'a, 'b> Folder for InvocationCollector<'a, 'b> { }; match ty.node { - ast::TyKind::Mac(mac) => - self.collect_bang(mac, Vec::new(), ty.span, ExpansionKind::Ty).make_ty(), + ast::TyKind::Mac(mac) => self.collect_bang(mac, ty.span, ExpansionKind::Ty).make_ty(), _ => unreachable!(), } } From 745ddf2aa78e4515d6a41e34a301d97afbde33ba Mon Sep 17 00:00:00 2001 From: Jeffrey Seyfried Date: Thu, 1 Dec 2016 22:49:32 +0000 Subject: [PATCH 10/16] Refactor out `mark.as_placeholder_id()`. --- src/libsyntax/ext/expand.rs | 8 ++++---- src/libsyntax/ext/hygiene.rs | 6 +++++- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/src/libsyntax/ext/expand.rs b/src/libsyntax/ext/expand.rs index e6782884f38..5d62175fbf2 100644 --- a/src/libsyntax/ext/expand.rs +++ b/src/libsyntax/ext/expand.rs @@ -275,7 +275,7 @@ impl<'a, 'b> MacroExpander<'a, 'b> { if expansions.len() < depth { expansions.push(Vec::new()); } - expansions[depth - 1].push((mark.as_u32(), expansion)); + expansions[depth - 1].push((mark, expansion)); if !self.cx.ecfg.single_step { invocations.extend(new_invocations.into_iter().rev()); } @@ -286,7 +286,7 @@ impl<'a, 'b> MacroExpander<'a, 'b> { let mut placeholder_expander = PlaceholderExpander::new(self.cx, self.monotonic); while let Some(expansions) = expansions.pop() { for (mark, expansion) in expansions.into_iter().rev() { - placeholder_expander.add(ast::NodeId::from_u32(mark), expansion); + placeholder_expander.add(mark.as_placeholder_id(), expansion); } } @@ -586,7 +586,7 @@ impl<'a, 'b> InvocationCollector<'a, 'b> { ..self.cx.current_expansion.clone() }, }); - placeholder(expansion_kind, ast::NodeId::from_u32(mark.as_u32())) + placeholder(expansion_kind, mark.as_placeholder_id()) } fn collect_bang(&mut self, mac: ast::Mac, span: Span, kind: ExpansionKind) -> Expansion { @@ -748,7 +748,7 @@ impl<'a, 'b> Folder for InvocationCollector<'a, 'b> { item.and_then(|mut item| match item.node { ItemKind::Mac(_) if is_macro_def => { - item.id = ast::NodeId::from_u32(Mark::fresh().as_u32()); + item.id = Mark::fresh().as_placeholder_id(); SmallVector::one(P(item)) } ItemKind::Mac(mac) => { diff --git a/src/libsyntax/ext/hygiene.rs b/src/libsyntax/ext/hygiene.rs index 8842cb55b1e..2af5c2ea999 100644 --- a/src/libsyntax/ext/hygiene.rs +++ b/src/libsyntax/ext/hygiene.rs @@ -51,7 +51,11 @@ impl Mark { Mark(id.as_u32()) } - pub fn as_u32(&self) -> u32 { + pub fn as_placeholder_id(self) -> NodeId { + NodeId::from_u32(self.0) + } + + pub fn as_u32(self) -> u32 { self.0 } } From dcae8bfb409c6b4f67b57a52b36bcecd4eafa3a4 Mon Sep 17 00:00:00 2001 From: Jeffrey Seyfried Date: Tue, 6 Dec 2016 06:25:41 +0000 Subject: [PATCH 11/16] Give extern crates' root modules a better name. --- src/librustc_resolve/build_reduced_graph.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/librustc_resolve/build_reduced_graph.rs b/src/librustc_resolve/build_reduced_graph.rs index 9f85580b93e..cd2a2767979 100644 --- a/src/librustc_resolve/build_reduced_graph.rs +++ b/src/librustc_resolve/build_reduced_graph.rs @@ -490,12 +490,13 @@ impl<'a> Resolver<'a> { fn get_extern_crate_root(&mut self, cnum: CrateNum) -> Module<'a> { let def_id = DefId { krate: cnum, index: CRATE_DEF_INDEX }; + let name = self.session.cstore.crate_name(cnum); let macros_only = self.session.cstore.dep_kind(cnum).macros_only(); let arenas = self.arenas; *self.extern_crate_roots.entry((cnum, macros_only)).or_insert_with(|| { arenas.alloc_module(ModuleData { populated: Cell::new(false), - ..ModuleData::new(None, ModuleKind::Def(Def::Mod(def_id), keywords::Invalid.name())) + ..ModuleData::new(None, ModuleKind::Def(Def::Mod(def_id), name)) }) }) } From 8e61ff25d85dcdc81c55f51ba2a777e13e561a25 Mon Sep 17 00:00:00 2001 From: Jeffrey Seyfried Date: Sat, 10 Dec 2016 06:45:58 +0000 Subject: [PATCH 12/16] Optimize `ast::PathSegment`. --- src/librustc/hir/lowering.rs | 18 +++-- src/librustc_passes/ast_validation.rs | 4 +- src/librustc_resolve/lib.rs | 14 +--- src/librustc_resolve/macros.rs | 4 +- src/libsyntax/ast.rs | 97 +++++---------------------- src/libsyntax/ext/build.rs | 31 ++++----- src/libsyntax/fold.rs | 2 +- src/libsyntax/parse/mod.rs | 53 +++------------ src/libsyntax/parse/parser.rs | 40 ++++------- src/libsyntax/print/pprust.rs | 13 ++-- src/libsyntax/std_inject.rs | 5 +- src/libsyntax/test.rs | 5 +- src/libsyntax/visit.rs | 4 +- src/libsyntax_ext/concat_idents.rs | 6 +- 14 files changed, 86 insertions(+), 210 deletions(-) diff --git a/src/librustc/hir/lowering.rs b/src/librustc/hir/lowering.rs index f5773d35178..e8c3492705a 100644 --- a/src/librustc/hir/lowering.rs +++ b/src/librustc/hir/lowering.rs @@ -433,13 +433,19 @@ impl<'a> LoweringContext<'a> { segment: &PathSegment, param_mode: ParamMode) -> hir::PathSegment { - let parameters = match segment.parameters { - PathParameters::AngleBracketed(ref data) => { - let data = self.lower_angle_bracketed_parameter_data(data, param_mode); - hir::AngleBracketedParameters(data) + let parameters = if let Some(ref parameters) = segment.parameters { + match **parameters { + PathParameters::AngleBracketed(ref data) => { + let data = self.lower_angle_bracketed_parameter_data(data, param_mode); + hir::AngleBracketedParameters(data) + } + PathParameters::Parenthesized(ref data) => { + hir::ParenthesizedParameters(self.lower_parenthesized_parameter_data(data)) + } } - PathParameters::Parenthesized(ref data) => - hir::ParenthesizedParameters(self.lower_parenthesized_parameter_data(data)), + } else { + let data = self.lower_angle_bracketed_parameter_data(&Default::default(), param_mode); + hir::AngleBracketedParameters(data) }; hir::PathSegment { diff --git a/src/librustc_passes/ast_validation.rs b/src/librustc_passes/ast_validation.rs index 2d0f0864752..bc150b84778 100644 --- a/src/librustc_passes/ast_validation.rs +++ b/src/librustc_passes/ast_validation.rs @@ -171,7 +171,7 @@ impl<'a> Visitor<'a> for AstValidator<'a> { match item.node { ItemKind::Use(ref view_path) => { let path = view_path.node.path(); - if !path.segments.iter().all(|segment| segment.parameters.is_empty()) { + if path.segments.iter().any(|segment| segment.parameters.is_some()) { self.err_handler() .span_err(path.span, "type or lifetime parameters in import path"); } @@ -275,7 +275,7 @@ impl<'a> Visitor<'a> for AstValidator<'a> { fn visit_vis(&mut self, vis: &'a Visibility) { match *vis { Visibility::Restricted { ref path, .. } => { - if !path.segments.iter().all(|segment| segment.parameters.is_empty()) { + if !path.segments.iter().all(|segment| segment.parameters.is_none()) { self.err_handler() .span_err(path.span, "type or lifetime parameters in visibility path"); } diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index d87e04f7b97..821820df838 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -62,7 +62,7 @@ use syntax::ast::{Arm, BindingMode, Block, Crate, Expr, ExprKind}; use syntax::ast::{FnDecl, ForeignItem, ForeignItemKind, Generics}; use syntax::ast::{Item, ItemKind, ImplItem, ImplItemKind}; use syntax::ast::{Local, Mutability, Pat, PatKind, Path}; -use syntax::ast::{PathSegment, PathParameters, QSelf, TraitItemKind, TraitRef, Ty, TyKind}; +use syntax::ast::{QSelf, TraitItemKind, TraitRef, Ty, TyKind}; use syntax_pos::{Span, DUMMY_SP}; use errors::DiagnosticBuilder; @@ -2960,14 +2960,9 @@ impl<'a> Resolver<'a> { if ident.name == lookup_name && ns == namespace { if filter_fn(name_binding.def()) { // create the path - let params = PathParameters::none(); - let segment = PathSegment { - identifier: ident, - parameters: params, - }; let span = name_binding.span; let mut segms = path_segments.clone(); - segms.push(segment); + segms.push(ident.into()); let path = Path { span: span, global: false, @@ -2990,10 +2985,7 @@ impl<'a> Resolver<'a> { if let Some(module) = name_binding.module() { // form the path let mut path_segments = path_segments.clone(); - path_segments.push(PathSegment { - identifier: ident, - parameters: PathParameters::none(), - }); + path_segments.push(ident.into()); if !in_module_is_extern || name_binding.vis == ty::Visibility::Public { // add the module to the lookup diff --git a/src/librustc_resolve/macros.rs b/src/librustc_resolve/macros.rs index ce92a4446f9..6399a266fcf 100644 --- a/src/librustc_resolve/macros.rs +++ b/src/librustc_resolve/macros.rs @@ -183,9 +183,9 @@ impl<'a> base::Resolver for Resolver<'a> { fn resolve_macro(&mut self, scope: Mark, path: &ast::Path, force: bool) -> Result, Determinacy> { let ast::Path { ref segments, global, span } = *path; - if segments.iter().any(|segment| !segment.parameters.is_empty()) { + if segments.iter().any(|segment| segment.parameters.is_some()) { let kind = - if segments.last().unwrap().parameters.is_empty() { "module" } else { "macro" }; + if segments.last().unwrap().parameters.is_some() { "macro" } else { "module" }; let msg = format!("type parameters are not allowed on {}s", kind); self.session.span_err(path.span, &msg); return Err(Determinacy::Determined); diff --git a/src/libsyntax/ast.rs b/src/libsyntax/ast.rs index 39d78cd8776..fdd82225b97 100644 --- a/src/libsyntax/ast.rs +++ b/src/libsyntax/ast.rs @@ -137,12 +137,7 @@ impl Path { Path { span: s, global: false, - segments: vec![ - PathSegment { - identifier: identifier, - parameters: PathParameters::none() - } - ], + segments: vec![identifier.into()], } } } @@ -160,7 +155,15 @@ pub struct PathSegment { /// this is more than just simple syntactic sugar; the use of /// parens affects the region binding rules, so we preserve the /// distinction. - pub parameters: PathParameters, + /// The `Option>` wrapper is purely a size optimization; + /// `None` is used to represent both `Path` and `Path<>`. + pub parameters: Option>, +} + +impl From for PathSegment { + fn from(id: Ident) -> Self { + PathSegment { identifier: id, parameters: None } + } } /// Parameters of a path segment. @@ -174,79 +177,8 @@ pub enum PathParameters { Parenthesized(ParenthesizedParameterData), } -impl PathParameters { - pub fn none() -> PathParameters { - PathParameters::AngleBracketed(AngleBracketedParameterData { - lifetimes: Vec::new(), - types: P::new(), - bindings: P::new(), - }) - } - - pub fn is_empty(&self) -> bool { - match *self { - PathParameters::AngleBracketed(ref data) => data.is_empty(), - - // Even if the user supplied no types, something like - // `X()` is equivalent to `X<(),()>`. - PathParameters::Parenthesized(..) => false, - } - } - - pub fn has_lifetimes(&self) -> bool { - match *self { - PathParameters::AngleBracketed(ref data) => !data.lifetimes.is_empty(), - PathParameters::Parenthesized(_) => false, - } - } - - pub fn has_types(&self) -> bool { - match *self { - PathParameters::AngleBracketed(ref data) => !data.types.is_empty(), - PathParameters::Parenthesized(..) => true, - } - } - - /// Returns the types that the user wrote. Note that these do not necessarily map to the type - /// parameters in the parenthesized case. - pub fn types(&self) -> Vec<&P> { - match *self { - PathParameters::AngleBracketed(ref data) => { - data.types.iter().collect() - } - PathParameters::Parenthesized(ref data) => { - data.inputs.iter() - .chain(data.output.iter()) - .collect() - } - } - } - - pub fn lifetimes(&self) -> Vec<&Lifetime> { - match *self { - PathParameters::AngleBracketed(ref data) => { - data.lifetimes.iter().collect() - } - PathParameters::Parenthesized(_) => { - Vec::new() - } - } - } - - pub fn bindings(&self) -> Vec<&TypeBinding> { - match *self { - PathParameters::AngleBracketed(ref data) => { - data.bindings.iter().collect() - } - PathParameters::Parenthesized(_) => { - Vec::new() - } - } - } -} - /// A path like `Foo<'a, T>` -#[derive(Clone, PartialEq, Eq, RustcEncodable, RustcDecodable, Hash, Debug)] +#[derive(Clone, PartialEq, Eq, RustcEncodable, RustcDecodable, Hash, Debug, Default)] pub struct AngleBracketedParameterData { /// The lifetime parameters for this path segment. pub lifetimes: Vec, @@ -258,9 +190,10 @@ pub struct AngleBracketedParameterData { pub bindings: P<[TypeBinding]>, } -impl AngleBracketedParameterData { - fn is_empty(&self) -> bool { - self.lifetimes.is_empty() && self.types.is_empty() && self.bindings.is_empty() +impl Into>> for AngleBracketedParameterData { + fn into(self) -> Option> { + let empty = self.lifetimes.is_empty() && self.types.is_empty() && self.bindings.is_empty(); + if empty { None } else { Some(P(PathParameters::AngleBracketed(self))) } } } diff --git a/src/libsyntax/ext/build.rs b/src/libsyntax/ext/build.rs index a208b934d79..c0dfb900240 100644 --- a/src/libsyntax/ext/build.rs +++ b/src/libsyntax/ext/build.rs @@ -322,21 +322,17 @@ impl<'a> AstBuilder for ExtCtxt<'a> { bindings: Vec ) -> ast::Path { let last_identifier = idents.pop().unwrap(); - let mut segments: Vec = idents.into_iter() - .map(|ident| { - ast::PathSegment { - identifier: ident, - parameters: ast::PathParameters::none(), - } - }).collect(); - segments.push(ast::PathSegment { - identifier: last_identifier, - parameters: ast::PathParameters::AngleBracketed(ast::AngleBracketedParameterData { + let mut segments: Vec = idents.into_iter().map(Into::into).collect(); + let parameters = if lifetimes.is_empty() && types.is_empty() && bindings.is_empty() { + None + } else { + Some(P(ast::PathParameters::AngleBracketed(ast::AngleBracketedParameterData { lifetimes: lifetimes, types: P::from_vec(types), bindings: P::from_vec(bindings), - }) - }); + }))) + }; + segments.push(ast::PathSegment { identifier: last_identifier, parameters: parameters }); ast::Path { span: sp, global: global, @@ -367,13 +363,14 @@ impl<'a> AstBuilder for ExtCtxt<'a> { bindings: Vec) -> (ast::QSelf, ast::Path) { let mut path = trait_path; + let parameters = ast::AngleBracketedParameterData { + lifetimes: lifetimes, + types: P::from_vec(types), + bindings: P::from_vec(bindings), + }; path.segments.push(ast::PathSegment { identifier: ident, - parameters: ast::PathParameters::AngleBracketed(ast::AngleBracketedParameterData { - lifetimes: lifetimes, - types: P::from_vec(types), - bindings: P::from_vec(bindings), - }) + parameters: Some(P(ast::PathParameters::AngleBracketed(parameters))), }); (ast::QSelf { diff --git a/src/libsyntax/fold.rs b/src/libsyntax/fold.rs index 6af8efb2a19..b3753e3e977 100644 --- a/src/libsyntax/fold.rs +++ b/src/libsyntax/fold.rs @@ -438,7 +438,7 @@ pub fn noop_fold_path(Path {global, segments, span}: Path, fld: &mut global: global, segments: segments.move_map(|PathSegment {identifier, parameters}| PathSegment { identifier: fld.fold_ident(identifier), - parameters: fld.fold_path_parameters(parameters), + parameters: parameters.map(|ps| ps.map(|ps| fld.fold_path_parameters(ps))), }), span: fld.new_span(span) } diff --git a/src/libsyntax/parse/mod.rs b/src/libsyntax/parse/mod.rs index c982205f0ec..b9e6605639e 100644 --- a/src/libsyntax/parse/mod.rs +++ b/src/libsyntax/parse/mod.rs @@ -634,12 +634,7 @@ mod tests { node: ast::ExprKind::Path(None, ast::Path { span: sp(0, 1), global: false, - segments: vec![ - ast::PathSegment { - identifier: Ident::from_str("a"), - parameters: ast::PathParameters::none(), - } - ], + segments: vec![Ident::from_str("a").into()], }), span: sp(0, 1), attrs: ThinVec::new(), @@ -651,19 +646,10 @@ mod tests { P(ast::Expr { id: ast::DUMMY_NODE_ID, node: ast::ExprKind::Path(None, ast::Path { - span: sp(0, 6), - global: true, - segments: vec![ - ast::PathSegment { - identifier: Ident::from_str("a"), - parameters: ast::PathParameters::none(), - }, - ast::PathSegment { - identifier: Ident::from_str("b"), - parameters: ast::PathParameters::none(), - } - ] - }), + span: sp(0, 6), + global: true, + segments: vec![Ident::from_str("a").into(), Ident::from_str("b").into()], + }), span: sp(0, 6), attrs: ThinVec::new(), })) @@ -772,12 +758,7 @@ mod tests { node:ast::ExprKind::Path(None, ast::Path{ span: sp(7, 8), global: false, - segments: vec![ - ast::PathSegment { - identifier: Ident::from_str("d"), - parameters: ast::PathParameters::none(), - } - ], + segments: vec![Ident::from_str("d").into()], }), span:sp(7,8), attrs: ThinVec::new(), @@ -795,12 +776,7 @@ mod tests { node: ast::ExprKind::Path(None, ast::Path { span:sp(0,1), global:false, - segments: vec![ - ast::PathSegment { - identifier: Ident::from_str("b"), - parameters: ast::PathParameters::none(), - } - ], + segments: vec![Ident::from_str("b").into()], }), span: sp(0,1), attrs: ThinVec::new()})), @@ -842,12 +818,7 @@ mod tests { node: ast::TyKind::Path(None, ast::Path{ span:sp(10,13), global:false, - segments: vec![ - ast::PathSegment { - identifier: Ident::from_str("i32"), - parameters: ast::PathParameters::none(), - } - ], + segments: vec![Ident::from_str("i32").into()], }), span:sp(10,13) }), @@ -890,13 +861,7 @@ mod tests { ast::Path{ span:sp(17,18), global:false, - segments: vec![ - ast::PathSegment { - identifier: Ident::from_str("b"), - parameters: - ast::PathParameters::none(), - } - ], + segments: vec![Ident::from_str("b").into()], }), span: sp(17,18), attrs: ThinVec::new()})), diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index a1d4ad9d629..72462b74e68 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -1705,12 +1705,11 @@ impl<'a> Parser<'a> { // Parse types, optionally. let parameters = if self.eat_lt() { let (lifetimes, types, bindings) = self.parse_generic_values_after_lt()?; - - ast::PathParameters::AngleBracketed(ast::AngleBracketedParameterData { + ast::AngleBracketedParameterData { lifetimes: lifetimes, types: P::from_vec(types), bindings: P::from_vec(bindings), - }) + }.into() } else if self.eat(&token::OpenDelim(token::Paren)) { let lo = self.prev_span.lo; @@ -1727,18 +1726,17 @@ impl<'a> Parser<'a> { let hi = self.prev_span.hi; - ast::PathParameters::Parenthesized(ast::ParenthesizedParameterData { + Some(P(ast::PathParameters::Parenthesized(ast::ParenthesizedParameterData { span: mk_sp(lo, hi), inputs: inputs, output: output_ty, - }) + }))) } else { - ast::PathParameters::none() + None }; // Assemble and push the result. - segments.push(ast::PathSegment { identifier: identifier, - parameters: parameters }); + segments.push(ast::PathSegment { identifier: identifier, parameters: parameters }); // Continue only if we see a `::` if !self.eat(&token::ModSep) { @@ -1757,10 +1755,7 @@ impl<'a> Parser<'a> { // If we do not see a `::`, stop. if !self.eat(&token::ModSep) { - segments.push(ast::PathSegment { - identifier: identifier, - parameters: ast::PathParameters::none() - }); + segments.push(identifier.into()); return Ok(segments); } @@ -1768,14 +1763,13 @@ impl<'a> Parser<'a> { if self.eat_lt() { // Consumed `a::b::<`, go look for types let (lifetimes, types, bindings) = self.parse_generic_values_after_lt()?; - let parameters = ast::AngleBracketedParameterData { - lifetimes: lifetimes, - types: P::from_vec(types), - bindings: P::from_vec(bindings), - }; segments.push(ast::PathSegment { identifier: identifier, - parameters: ast::PathParameters::AngleBracketed(parameters), + parameters: ast::AngleBracketedParameterData { + lifetimes: lifetimes, + types: P::from_vec(types), + bindings: P::from_vec(bindings), + }.into(), }); // Consumed `a::b::`, check for `::` before proceeding @@ -1784,10 +1778,7 @@ impl<'a> Parser<'a> { } } else { // Consumed `a::`, go look for `b` - segments.push(ast::PathSegment { - identifier: identifier, - parameters: ast::PathParameters::none(), - }); + segments.push(identifier.into()); } } } @@ -1802,10 +1793,7 @@ impl<'a> Parser<'a> { let identifier = self.parse_path_segment_ident()?; // Assemble and push the result. - segments.push(ast::PathSegment { - identifier: identifier, - parameters: ast::PathParameters::none() - }); + segments.push(identifier.into()); // If we do not see a `::` or see `::{`/`::*`, stop. if !self.check(&token::ModSep) || self.is_import_coupler() { diff --git a/src/libsyntax/print/pprust.rs b/src/libsyntax/print/pprust.rs index c28b9d00501..22e8391de93 100644 --- a/src/libsyntax/print/pprust.rs +++ b/src/libsyntax/print/pprust.rs @@ -2349,7 +2349,9 @@ impl<'a> State<'a> { try!(self.print_ident(segment.identifier)); - try!(self.print_path_parameters(&segment.parameters, colons_before_params)); + if let Some(ref parameters) = segment.parameters { + try!(self.print_path_parameters(parameters, colons_before_params)) + } } Ok(()) @@ -2373,7 +2375,10 @@ impl<'a> State<'a> { try!(word(&mut self.s, "::")); let item_segment = path.segments.last().unwrap(); try!(self.print_ident(item_segment.identifier)); - self.print_path_parameters(&item_segment.parameters, colons_before_params) + match item_segment.parameters { + Some(ref parameters) => self.print_path_parameters(parameters, colons_before_params), + None => Ok(()), + } } fn print_path_parameters(&mut self, @@ -2381,10 +2386,6 @@ impl<'a> State<'a> { colons_before_params: bool) -> io::Result<()> { - if parameters.is_empty() { - return Ok(()); - } - if colons_before_params { try!(word(&mut self.s, "::")) } diff --git a/src/libsyntax/std_inject.rs b/src/libsyntax/std_inject.rs index 6a291ad9c40..4ad760a3caf 100644 --- a/src/libsyntax/std_inject.rs +++ b/src/libsyntax/std_inject.rs @@ -81,9 +81,8 @@ pub fn maybe_inject_crates_ref(sess: &ParseSess, vis: ast::Visibility::Inherited, node: ast::ItemKind::Use(P(codemap::dummy_spanned(ast::ViewPathGlob(ast::Path { global: false, - segments: vec![name, "prelude", "v1"].into_iter().map(|name| ast::PathSegment { - identifier: ast::Ident::from_str(name), - parameters: ast::PathParameters::none(), + segments: vec![name, "prelude", "v1"].into_iter().map(|name| { + ast::Ident::from_str(name).into() }).collect(), span: span, })))), diff --git a/src/libsyntax/test.rs b/src/libsyntax/test.rs index fca89e265e4..7709d3bd1cf 100644 --- a/src/libsyntax/test.rs +++ b/src/libsyntax/test.rs @@ -580,10 +580,7 @@ fn path_node(ids: Vec) -> ast::Path { ast::Path { span: DUMMY_SP, global: false, - segments: ids.into_iter().map(|identifier| ast::PathSegment { - identifier: identifier, - parameters: ast::PathParameters::none(), - }).collect() + segments: ids.into_iter().map(Into::into).collect(), } } diff --git a/src/libsyntax/visit.rs b/src/libsyntax/visit.rs index c1391d0b1c2..ad29cb50a84 100644 --- a/src/libsyntax/visit.rs +++ b/src/libsyntax/visit.rs @@ -383,7 +383,9 @@ pub fn walk_path_segment<'a, V: Visitor<'a>>(visitor: &mut V, path_span: Span, segment: &'a PathSegment) { visitor.visit_ident(path_span, segment.identifier); - visitor.visit_path_parameters(path_span, &segment.parameters); + if let Some(ref parameters) = segment.parameters { + visitor.visit_path_parameters(path_span, parameters); + } } pub fn walk_path_parameters<'a, V>(visitor: &mut V, diff --git a/src/libsyntax_ext/concat_idents.rs b/src/libsyntax_ext/concat_idents.rs index b26e33eb384..1381490efa1 100644 --- a/src/libsyntax_ext/concat_idents.rs +++ b/src/libsyntax_ext/concat_idents.rs @@ -59,14 +59,10 @@ pub fn expand_syntax_ext<'cx>(cx: &'cx mut ExtCtxt, impl Result { fn path(&self) -> ast::Path { - let segment = ast::PathSegment { - identifier: self.ident, - parameters: ast::PathParameters::none(), - }; ast::Path { span: self.span, global: false, - segments: vec![segment], + segments: vec![self.ident.into()], } } } From 51f25b3cfcb8137e7a95e35a89e5b3a7f33858b2 Mon Sep 17 00:00:00 2001 From: Jeffrey Seyfried Date: Mon, 12 Dec 2016 08:17:47 +0000 Subject: [PATCH 13/16] resolve: clean up diagnostics for name conflicts. --- src/librustc_resolve/lib.rs | 37 ++++--------------- src/test/compile-fail/E0259.rs | 2 +- .../compile-fail/blind-item-item-shadow.rs | 2 +- src/test/compile-fail/issue-19498.rs | 6 +-- src/test/compile-fail/issue-24081.rs | 10 ++--- .../resolve-conflict-item-vs-import.rs | 2 +- 6 files changed, 19 insertions(+), 40 deletions(-) diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index 821820df838..3535aab9a1b 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -3156,40 +3156,19 @@ impl<'a> Resolver<'a> { }; let mut err = match (old_binding.is_extern_crate(), binding.is_extern_crate()) { - (true, true) => { - let mut e = struct_span_err!(self.session, span, E0259, "{}", msg); - e.span_label(span, &format!("`{}` was already imported", name)); - e - }, - (true, _) | (_, true) if binding.is_import() && old_binding.is_import() => { - let mut e = struct_span_err!(self.session, span, E0254, "{}", msg); - e.span_label(span, &"already imported"); - e - }, - (true, _) | (_, true) => { - let mut e = struct_span_err!(self.session, span, E0260, "{}", msg); - e.span_label(span, &format!("`{}` already imported", name)); - e + (true, true) => struct_span_err!(self.session, span, E0259, "{}", msg), + (true, _) | (_, true) => match binding.is_import() && old_binding.is_import() { + true => struct_span_err!(self.session, span, E0254, "{}", msg), + false => struct_span_err!(self.session, span, E0260, "{}", msg), }, _ => match (old_binding.is_import(), binding.is_import()) { - (false, false) => { - let mut e = struct_span_err!(self.session, span, E0428, "{}", msg); - e.span_label(span, &format!("already defined")); - e - }, - (true, true) => { - let mut e = struct_span_err!(self.session, span, E0252, "{}", msg); - e.span_label(span, &format!("already imported")); - e - }, - _ => { - let mut e = struct_span_err!(self.session, span, E0255, "{}", msg); - e.span_label(span, &format!("`{}` was already imported", name)); - e - } + (false, false) => struct_span_err!(self.session, span, E0428, "{}", msg), + (true, true) => struct_span_err!(self.session, span, E0252, "{}", msg), + _ => struct_span_err!(self.session, span, E0255, "{}", msg), }, }; + err.span_label(span, &format!("`{}` already {}", name, participle)); if old_binding.span != syntax_pos::DUMMY_SP { err.span_label(old_binding.span, &format!("previous {} of `{}` here", noun, name)); } diff --git a/src/test/compile-fail/E0259.rs b/src/test/compile-fail/E0259.rs index 95be48b5ff1..b2129902ef9 100644 --- a/src/test/compile-fail/E0259.rs +++ b/src/test/compile-fail/E0259.rs @@ -15,6 +15,6 @@ extern crate collections; extern crate libc as collections; //~^ ERROR E0259 -//~| NOTE `collections` was already imported +//~| NOTE `collections` already imported fn main() {} diff --git a/src/test/compile-fail/blind-item-item-shadow.rs b/src/test/compile-fail/blind-item-item-shadow.rs index 853282ff014..e9df8868a1e 100644 --- a/src/test/compile-fail/blind-item-item-shadow.rs +++ b/src/test/compile-fail/blind-item-item-shadow.rs @@ -12,6 +12,6 @@ mod foo { pub mod foo { } } //~ NOTE previous definition of `foo` here use foo::foo; //~^ ERROR a module named `foo` has already been defined in this module -//~| was already imported +//~| `foo` already defined fn main() {} diff --git a/src/test/compile-fail/issue-19498.rs b/src/test/compile-fail/issue-19498.rs index 2e2115b7110..88e804fb8aa 100644 --- a/src/test/compile-fail/issue-19498.rs +++ b/src/test/compile-fail/issue-19498.rs @@ -11,13 +11,13 @@ use self::A; //~ NOTE previous import of `A` here use self::B; //~ NOTE previous import of `B` here mod A {} //~ ERROR a module named `A` has already been imported in this module -//~| `A` was already imported +//~| `A` already imported pub mod B {} //~ ERROR a module named `B` has already been imported in this module -//~| `B` was already imported +//~| `B` already imported mod C { use C::D; //~ NOTE previous import of `D` here mod D {} //~ ERROR a module named `D` has already been imported in this module - //~| `D` was already imported + //~| `D` already imported } fn main() {} diff --git a/src/test/compile-fail/issue-24081.rs b/src/test/compile-fail/issue-24081.rs index 188716c5e93..26bb72b862f 100644 --- a/src/test/compile-fail/issue-24081.rs +++ b/src/test/compile-fail/issue-24081.rs @@ -15,14 +15,14 @@ use std::ops::Div; //~ NOTE previous import use std::ops::Rem; //~ NOTE previous import type Add = bool; //~ ERROR a trait named `Add` has already been imported in this module -//~| was already imported +//~| `Add` already imported struct Sub { x: f32 } //~ ERROR a trait named `Sub` has already been imported in this module -//~| was already imported +//~| `Sub` already imported enum Mul { A, B } //~ ERROR a trait named `Mul` has already been imported in this module -//~| was already imported +//~| `Mul` already imported mod Div { } //~ ERROR a trait named `Div` has already been imported in this module -//~| was already imported +//~| `Div` already imported trait Rem { } //~ ERROR a trait named `Rem` has already been imported in this module -//~| was already imported +//~| `Rem` already imported fn main() {} diff --git a/src/test/compile-fail/resolve-conflict-item-vs-import.rs b/src/test/compile-fail/resolve-conflict-item-vs-import.rs index 5a068ce4214..2083d98e09d 100644 --- a/src/test/compile-fail/resolve-conflict-item-vs-import.rs +++ b/src/test/compile-fail/resolve-conflict-item-vs-import.rs @@ -13,6 +13,6 @@ use std::mem::transmute; fn transmute() {} //~^ ERROR a value named `transmute` has already been imported in this module -//~| was already imported +//~| `transmute` already imported fn main() { } From 8d9ba291f54e45cb4d12a066850680241efa25f5 Mon Sep 17 00:00:00 2001 From: Jeffrey Seyfried Date: Mon, 12 Dec 2016 10:09:22 +0000 Subject: [PATCH 14/16] Minor bugfix for macro invocation path resolution. --- src/librustc_resolve/macros.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/librustc_resolve/macros.rs b/src/librustc_resolve/macros.rs index 6399a266fcf..dfeb66e1d8c 100644 --- a/src/librustc_resolve/macros.rs +++ b/src/librustc_resolve/macros.rs @@ -205,7 +205,10 @@ impl<'a> base::Resolver for Resolver<'a> { } let ext = match self.resolve_path(&path, path_scope, Some(MacroNS), None) { - PathResult::NonModule(path_res) => Ok(self.get_macro(path_res.base_def)), + PathResult::NonModule(path_res) => match path_res.base_def { + Def::Err => Err(Determinacy::Determined), + def @ _ => Ok(self.get_macro(def)), + }, PathResult::Module(..) => unreachable!(), PathResult::Indeterminate if !force => return Err(Determinacy::Undetermined), _ => Err(Determinacy::Determined), From 39e6ae2dccdea270ef70aa7eecdd87252a5c36f2 Mon Sep 17 00:00:00 2001 From: Jeffrey Seyfried Date: Wed, 14 Dec 2016 09:59:41 +0000 Subject: [PATCH 15/16] Clean up `get_traits_containing_item`. --- src/librustc_resolve/lib.rs | 92 +++++++++++++++++-------------------- 1 file changed, 42 insertions(+), 50 deletions(-) diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index 3535aab9a1b..fb28b45d20e 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -2863,72 +2863,64 @@ impl<'a> Resolver<'a> { fn get_traits_containing_item(&mut self, name: Name) -> Vec { debug!("(getting traits containing item) looking for '{}'", name); - fn add_trait_info(found_traits: &mut Vec, - trait_def_id: DefId, - import_id: Option, - name: Name) { - debug!("(adding trait info) found trait {:?} for method '{}'", - trait_def_id, - name); - found_traits.push(TraitCandidate { - def_id: trait_def_id, - import_id: import_id, - }); - } - let mut found_traits = Vec::new(); // Look for the current trait. if let Some((trait_def_id, _)) = self.current_trait_ref { if self.trait_item_map.contains_key(&(name, trait_def_id)) { - add_trait_info(&mut found_traits, trait_def_id, None, name); + found_traits.push(TraitCandidate { def_id: trait_def_id, import_id: None }); } } let mut search_module = self.current_module; loop { - // Look for trait children. - let mut search_in_module = |this: &mut Self, module: Module<'a>| { - let mut traits = module.traits.borrow_mut(); - if traits.is_none() { - let mut collected_traits = Vec::new(); - module.for_each_child(|name, ns, binding| { - if ns != TypeNS { return } - if let Def::Trait(_) = binding.def() { - collected_traits.push((name, binding)); - } - }); - *traits = Some(collected_traits.into_boxed_slice()); - } + self.get_traits_in_module_containing_item(name, search_module, &mut found_traits); + match search_module.kind { + ModuleKind::Block(..) => search_module = search_module.parent.unwrap(), + _ => break, + } + } - for &(trait_name, binding) in traits.as_ref().unwrap().iter() { - let trait_def_id = binding.def().def_id(); - if this.trait_item_map.contains_key(&(name, trait_def_id)) { - let mut import_id = None; - if let NameBindingKind::Import { directive, .. } = binding.kind { - let id = directive.id; - this.maybe_unused_trait_imports.insert(id); - this.add_to_glob_map(id, trait_name); - import_id = Some(id); - } - add_trait_info(&mut found_traits, trait_def_id, import_id, name); - } - } - }; - search_in_module(self, search_module); - - if let ModuleKind::Block(..) = search_module.kind { - search_module = search_module.parent.unwrap(); - } else { - if !search_module.no_implicit_prelude { - self.prelude.map(|prelude| search_in_module(self, prelude)); - } - break; + if let Some(prelude) = self.prelude { + if !search_module.no_implicit_prelude { + self.get_traits_in_module_containing_item(name, prelude, &mut found_traits); } } found_traits } + fn get_traits_in_module_containing_item(&mut self, + name: Name, + module: Module, + found_traits: &mut Vec) { + let mut traits = module.traits.borrow_mut(); + if traits.is_none() { + let mut collected_traits = Vec::new(); + module.for_each_child(|name, ns, binding| { + if ns != TypeNS { return } + if let Def::Trait(_) = binding.def() { + collected_traits.push((name, binding)); + } + }); + *traits = Some(collected_traits.into_boxed_slice()); + } + + for &(trait_name, binding) in traits.as_ref().unwrap().iter() { + let trait_def_id = binding.def().def_id(); + if self.trait_item_map.contains_key(&(name, trait_def_id)) { + let import_id = match binding.kind { + NameBindingKind::Import { directive, .. } => { + self.maybe_unused_trait_imports.insert(directive.id); + self.add_to_glob_map(directive.id, trait_name); + Some(directive.id) + } + _ => None, + }; + found_traits.push(TraitCandidate { def_id: trait_def_id, import_id: import_id }); + } + } + } + /// When name resolution fails, this method can be used to look up candidate /// entities with the expected name. It allows filtering them using the /// supplied predicate (which should be used to only accept the types of From f705c69bf641b271828f37adb525cafc618237d8 Mon Sep 17 00:00:00 2001 From: Jeffrey Seyfried Date: Thu, 15 Dec 2016 11:13:24 +0000 Subject: [PATCH 16/16] Simplify `TyCtxt::create_and_enter`. --- src/librustc/ty/context.rs | 10 ++++------ src/librustc/ty/mod.rs | 9 ++++++++- src/librustc_driver/driver.rs | 16 +++------------- src/librustc_driver/pretty.rs | 5 ++--- src/librustc_driver/test.rs | 4 +--- 5 files changed, 18 insertions(+), 26 deletions(-) diff --git a/src/librustc/ty/context.rs b/src/librustc/ty/context.rs index 4854a14f733..20405398eff 100644 --- a/src/librustc/ty/context.rs +++ b/src/librustc/ty/context.rs @@ -762,11 +762,9 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> { /// reference to the context, to allow formatting values that need it. pub fn create_and_enter(s: &'tcx Session, arenas: &'tcx CtxtArenas<'tcx>, - trait_map: TraitMap, + resolutions: ty::Resolutions, named_region_map: resolve_lifetime::NamedRegionMap, map: ast_map::Map<'tcx>, - freevars: FreevarMap, - maybe_unused_trait_imports: NodeSet, region_maps: RegionMaps, lang_items: middle::lang_items::LanguageItems, stability: stability::Index<'tcx>, @@ -790,7 +788,7 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> { item_variance_map: RefCell::new(DepTrackingMap::new(dep_graph.clone())), variance_computed: Cell::new(false), sess: s, - trait_map: trait_map, + trait_map: resolutions.trait_map, tables: RefCell::new(Tables::empty()), impl_trait_refs: RefCell::new(DepTrackingMap::new(dep_graph.clone())), trait_defs: RefCell::new(DepTrackingMap::new(dep_graph.clone())), @@ -802,8 +800,8 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> { fulfilled_predicates: RefCell::new(fulfilled_predicates), map: map, mir_map: RefCell::new(DepTrackingMap::new(dep_graph.clone())), - freevars: RefCell::new(freevars), - maybe_unused_trait_imports: maybe_unused_trait_imports, + freevars: RefCell::new(resolutions.freevars), + maybe_unused_trait_imports: resolutions.maybe_unused_trait_imports, item_types: RefCell::new(DepTrackingMap::new(dep_graph.clone())), rcache: RefCell::new(FxHashMap()), tc_cache: RefCell::new(FxHashMap()), diff --git a/src/librustc/ty/mod.rs b/src/librustc/ty/mod.rs index df12c252907..8f478435efd 100644 --- a/src/librustc/ty/mod.rs +++ b/src/librustc/ty/mod.rs @@ -17,7 +17,7 @@ pub use self::LvaluePreference::*; pub use self::fold::TypeFoldable; use dep_graph::{self, DepNode}; -use hir::map as ast_map; +use hir::{map as ast_map, FreevarMap, TraitMap}; use middle; use hir::def::{Def, CtorKind, ExportMap}; use hir::def_id::{CrateNum, DefId, CRATE_DEF_INDEX, LOCAL_CRATE}; @@ -112,6 +112,13 @@ pub struct CrateAnalysis<'tcx> { pub hir_ty_to_ty: NodeMap>, } +#[derive(Clone)] +pub struct Resolutions { + pub freevars: FreevarMap, + pub trait_map: TraitMap, + pub maybe_unused_trait_imports: NodeSet, +} + #[derive(Copy, Clone)] pub enum DtorKind { NoDtor, diff --git a/src/librustc_driver/driver.rs b/src/librustc_driver/driver.rs index ace00f03185..360933c6b66 100644 --- a/src/librustc_driver/driver.rs +++ b/src/librustc_driver/driver.rs @@ -8,8 +8,7 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -use rustc::hir; -use rustc::hir::{map as hir_map, FreevarMap, TraitMap}; +use rustc::hir::{self, map as hir_map}; use rustc::hir::lowering::lower_crate; use rustc_data_structures::stable_hasher::StableHasher; use rustc_mir as mir; @@ -20,7 +19,7 @@ use rustc::session::search_paths::PathKind; use rustc::lint; use rustc::middle::{self, dependency_format, stability, reachable}; use rustc::middle::privacy::AccessLevels; -use rustc::ty::{self, TyCtxt}; +use rustc::ty::{self, TyCtxt, Resolutions}; use rustc::util::common::time; use rustc::util::nodemap::{NodeSet, NodeMap}; use rustc_borrowck as borrowck; @@ -59,13 +58,6 @@ use syntax_ext; use derive_registrar; -#[derive(Clone)] -pub struct Resolutions { - pub freevars: FreevarMap, - pub trait_map: TraitMap, - pub maybe_unused_trait_imports: NodeSet, -} - pub fn compile_input(sess: &Session, cstore: &CStore, input: &Input, @@ -864,11 +856,9 @@ pub fn phase_3_run_analysis_passes<'tcx, F, R>(sess: &'tcx Session, TyCtxt::create_and_enter(sess, arenas, - resolutions.trait_map, + resolutions, named_region_map, hir_map, - resolutions.freevars, - resolutions.maybe_unused_trait_imports, region_map, lang_items, index, diff --git a/src/librustc_driver/pretty.rs b/src/librustc_driver/pretty.rs index b055b043723..74df1e52bde 100644 --- a/src/librustc_driver/pretty.rs +++ b/src/librustc_driver/pretty.rs @@ -15,10 +15,9 @@ pub use self::PpSourceMode::*; pub use self::PpMode::*; use self::NodesMatchingUII::*; -use abort_on_err; -use driver::{self, Resolutions}; +use {abort_on_err, driver}; -use rustc::ty::{self, TyCtxt}; +use rustc::ty::{self, TyCtxt, Resolutions}; use rustc::cfg; use rustc::cfg::graphviz::LabelledCFG; use rustc::dep_graph::DepGraph; diff --git a/src/librustc_driver/test.rs b/src/librustc_driver/test.rs index 2f8550e5acd..cbab39c3908 100644 --- a/src/librustc_driver/test.rs +++ b/src/librustc_driver/test.rs @@ -138,11 +138,9 @@ fn test_env(source_string: &str, let index = stability::Index::new(&ast_map); TyCtxt::create_and_enter(&sess, &arenas, - resolutions.trait_map, + resolutions, named_region_map.unwrap(), ast_map, - resolutions.freevars, - resolutions.maybe_unused_trait_imports, region_map, lang_items, index,