From f83a972224db1250513390589480be7e14c2a6c5 Mon Sep 17 00:00:00 2001 From: Eduard Burtescu Date: Tue, 23 Dec 2014 21:34:36 +0200 Subject: [PATCH] rustc_resolve: fix fallout of merging ast::ViewItem into ast::Item. --- src/librustc_resolve/build_reduced_graph.rs | 284 +++++++++----------- src/librustc_resolve/check_unused.rs | 36 +-- src/librustc_resolve/lib.rs | 14 +- 3 files changed, 156 insertions(+), 178 deletions(-) diff --git a/src/librustc_resolve/build_reduced_graph.rs b/src/librustc_resolve/build_reduced_graph.rs index 21eec383df4..65bd83d7937 100644 --- a/src/librustc_resolve/build_reduced_graph.rs +++ b/src/librustc_resolve/build_reduced_graph.rs @@ -39,9 +39,9 @@ use syntax::ast::{Block, Crate}; use syntax::ast::{DeclItem, DefId}; use syntax::ast::{ForeignItem, ForeignItemFn, ForeignItemStatic}; -use syntax::ast::{Item, ItemConst, ItemEnum, ItemFn}; +use syntax::ast::{Item, ItemConst, ItemEnum, ItemExternCrate, ItemFn}; use syntax::ast::{ItemForeignMod, ItemImpl, ItemMac, ItemMod, ItemStatic}; -use syntax::ast::{ItemStruct, ItemTrait, ItemTy}; +use syntax::ast::{ItemStruct, ItemTrait, ItemTy, ItemUse}; use syntax::ast::{MethodImplItem, Name, NamedField, NodeId}; use syntax::ast::{PathListIdent, PathListMod}; use syntax::ast::{Public, SelfStatic}; @@ -50,8 +50,7 @@ use syntax::ast::TupleVariantKind; use syntax::ast::TyObjectSum; use syntax::ast::{TypeImplItem, UnnamedField}; -use syntax::ast::{Variant, ViewItem, ViewItemExternCrate}; -use syntax::ast::{ViewItemUse, ViewPathGlob, ViewPathList, ViewPathSimple}; +use syntax::ast::{Variant, ViewPathGlob, ViewPathList, ViewPathSimple}; use syntax::ast::{Visibility}; use syntax::ast::TyPath; use syntax::ast; @@ -238,11 +237,6 @@ fn add_child(&self, } fn block_needs_anonymous_module(&mut self, block: &Block) -> bool { - // If the block has view items, we need an anonymous module. - if block.view_items.len() > 0 { - return true; - } - // Check each statement. for statement in block.stmts.iter() { match statement.node { @@ -262,7 +256,7 @@ fn block_needs_anonymous_module(&mut self, block: &Block) -> bool { } } - // If we found neither view items nor items, we don't need to create + // If we found no items, we don't need to create // an anonymous module. return false; @@ -280,6 +274,133 @@ fn build_reduced_graph_for_item(&mut self, item: &Item, parent: &Rc) -> let modifiers = if is_public { PUBLIC } else { DefModifiers::empty() } | IMPORTABLE; match item.node { + ItemUse(ref view_path) => { + // Extract and intern the module part of the path. For + // globs and lists, the path is found directly in the AST; + // for simple paths we have to munge the path a little. + let module_path = match view_path.node { + ViewPathSimple(_, ref full_path) => { + full_path.segments + .init() + .iter().map(|ident| ident.identifier.name) + .collect() + } + + ViewPathGlob(ref module_ident_path) | + ViewPathList(ref module_ident_path, _) => { + module_ident_path.segments + .iter().map(|ident| ident.identifier.name).collect() + } + }; + + // Build up the import directives. + let shadowable = item.attrs.iter().any(|attr| { + attr.name() == token::get_name(special_idents::prelude_import.name) + }); + let shadowable = if shadowable { + Shadowable::Always + } else { + Shadowable::Never + }; + + match view_path.node { + ViewPathSimple(binding, ref full_path) => { + let source_name = + full_path.segments.last().unwrap().identifier.name; + if token::get_name(source_name).get() == "mod" || + token::get_name(source_name).get() == "self" { + self.resolve_error(view_path.span, + "`self` imports are only allowed within a { } list"); + } + + let subclass = SingleImport(binding.name, + source_name); + self.build_import_directive(&**parent, + module_path, + subclass, + view_path.span, + item.id, + is_public, + shadowable); + } + ViewPathList(_, ref source_items) => { + // Make sure there's at most one `mod` import in the list. + let mod_spans = source_items.iter().filter_map(|item| match item.node { + PathListMod { .. } => Some(item.span), + _ => None + }).collect::>(); + if mod_spans.len() > 1 { + self.resolve_error(mod_spans[0], + "`self` import can only appear once in the list"); + for other_span in mod_spans.iter().skip(1) { + self.session.span_note(*other_span, + "another `self` import appears here"); + } + } + + for source_item in source_items.iter() { + let (module_path, name) = match source_item.node { + PathListIdent { name, .. } => + (module_path.clone(), name.name), + PathListMod { .. } => { + let name = match module_path.last() { + Some(name) => *name, + None => { + self.resolve_error(source_item.span, + "`self` import can only appear in an import list \ + with a non-empty prefix"); + continue; + } + }; + let module_path = module_path.init(); + (module_path.to_vec(), name) + } + }; + self.build_import_directive( + &**parent, + module_path, + SingleImport(name, name), + source_item.span, + source_item.node.id(), + is_public, + shadowable); + } + } + ViewPathGlob(_) => { + self.build_import_directive(&**parent, + module_path, + GlobImport, + view_path.span, + item.id, + is_public, + shadowable); + } + } + parent.clone() + } + + ItemExternCrate(_) => { + // n.b. we don't need to look at the path option here, because cstore already did + for &crate_id in self.session.cstore + .find_extern_mod_stmt_cnum(item.id).iter() { + let def_id = DefId { krate: crate_id, node: 0 }; + self.external_exports.insert(def_id); + let parent_link = ModuleParentLink(parent.downgrade(), name); + let external_module = Rc::new(Module::new(parent_link, + Some(def_id), + NormalModuleKind, + false, + true)); + debug!("(build reduced graph for item) found extern `{}`", + self.module_to_string(&*external_module)); + self.check_for_conflicts_between_external_crates(&**parent, name, sp); + parent.external_module_children.borrow_mut() + .insert(name, external_module.clone()); + self.build_reduced_graph_for_external_crate(&external_module); + } + parent.clone() + } + ItemMod(..) => { let name_bindings = self.add_child(name, parent, ForbidDuplicateModules, sp); @@ -650,145 +771,6 @@ fn build_reduced_graph_for_variant(&mut self, variant.span, PUBLIC | IMPORTABLE); } - /// Constructs the reduced graph for one 'view item'. View items consist - /// of imports and use directives. - fn build_reduced_graph_for_view_item(&mut self, view_item: &ViewItem, parent: &Rc) { - match view_item.node { - ViewItemUse(ref view_path) => { - // Extract and intern the module part of the path. For - // globs and lists, the path is found directly in the AST; - // for simple paths we have to munge the path a little. - let module_path = match view_path.node { - ViewPathSimple(_, ref full_path, _) => { - full_path.segments - .init() - .iter().map(|ident| ident.identifier.name) - .collect() - } - - ViewPathGlob(ref module_ident_path, _) | - ViewPathList(ref module_ident_path, _, _) => { - module_ident_path.segments - .iter().map(|ident| ident.identifier.name).collect() - } - }; - - // Build up the import directives. - let is_public = view_item.vis == ast::Public; - let shadowable = - view_item.attrs - .iter() - .any(|attr| { - attr.name() == token::get_name( - special_idents::prelude_import.name) - }); - let shadowable = if shadowable { - Shadowable::Always - } else { - Shadowable::Never - }; - - match view_path.node { - ViewPathSimple(binding, ref full_path, id) => { - let source_name = - full_path.segments.last().unwrap().identifier.name; - if token::get_name(source_name).get() == "mod" || - token::get_name(source_name).get() == "self" { - self.resolve_error(view_path.span, - "`self` imports are only allowed within a { } list"); - } - - let subclass = SingleImport(binding.name, - source_name); - self.build_import_directive(&**parent, - module_path, - subclass, - view_path.span, - id, - is_public, - shadowable); - } - ViewPathList(_, ref source_items, _) => { - // Make sure there's at most one `mod` import in the list. - let mod_spans = source_items.iter().filter_map(|item| match item.node { - PathListMod { .. } => Some(item.span), - _ => None - }).collect::>(); - if mod_spans.len() > 1 { - self.resolve_error(mod_spans[0], - "`self` import can only appear once in the list"); - for other_span in mod_spans.iter().skip(1) { - self.session.span_note(*other_span, - "another `self` import appears here"); - } - } - - for source_item in source_items.iter() { - let (module_path, name) = match source_item.node { - PathListIdent { name, .. } => - (module_path.clone(), name.name), - PathListMod { .. } => { - let name = match module_path.last() { - Some(name) => *name, - None => { - self.resolve_error(source_item.span, - "`self` import can only appear in an import list \ - with a non-empty prefix"); - continue; - } - }; - let module_path = module_path.init(); - (module_path.to_vec(), name) - } - }; - self.build_import_directive( - &**parent, - module_path, - SingleImport(name, name), - source_item.span, - source_item.node.id(), - is_public, - shadowable); - } - } - ViewPathGlob(_, id) => { - self.build_import_directive(&**parent, - module_path, - GlobImport, - view_path.span, - id, - is_public, - shadowable); - } - } - } - - ViewItemExternCrate(name, _, node_id) => { - // n.b. we don't need to look at the path option here, because cstore already did - for &crate_id in self.session.cstore - .find_extern_mod_stmt_cnum(node_id).iter() { - let def_id = DefId { krate: crate_id, node: 0 }; - self.external_exports.insert(def_id); - let parent_link = ModuleParentLink(parent.downgrade(), name.name); - let external_module = Rc::new(Module::new(parent_link, - Some(def_id), - NormalModuleKind, - false, - true)); - debug!("(build reduced graph for item) found extern `{}`", - self.module_to_string(&*external_module)); - self.check_for_conflicts_between_external_crates( - &**parent, - name.name, - view_item.span); - parent.external_module_children.borrow_mut() - .insert(name.name, external_module.clone()); - self.build_reduced_graph_for_external_crate(&external_module); - } - } - } - } - /// Constructs the reduced graph for one foreign item. fn build_reduced_graph_for_foreign_item(&mut self, foreign_item: &ForeignItem, @@ -1270,10 +1252,6 @@ fn visit_foreign_item(&mut self, foreign_item: &ForeignItem) { }) } - fn visit_view_item(&mut self, view_item: &ViewItem) { - self.builder.build_reduced_graph_for_view_item(view_item, &self.parent); - } - fn visit_block(&mut self, block: &Block) { let np = self.builder.build_reduced_graph_for_block(block, &self.parent); let old_parent = replace(&mut self.parent, np); diff --git a/src/librustc_resolve/check_unused.rs b/src/librustc_resolve/check_unused.rs index 18066a7b94b..97370112ab4 100644 --- a/src/librustc_resolve/check_unused.rs +++ b/src/librustc_resolve/check_unused.rs @@ -25,7 +25,6 @@ use rustc::lint; use rustc::middle::privacy::{DependsOn, LastImport, Used, Unused}; use syntax::ast; -use syntax::ast::{ViewItem, ViewItemExternCrate, ViewItemUse}; use syntax::ast::{ViewPathGlob, ViewPathList, ViewPathSimple}; use syntax::codemap::{Span, DUMMY_SP}; use syntax::visit::{self, Visitor}; @@ -109,53 +108,54 @@ fn finalize_import(&mut self, id: ast::NodeId, span: Span) { } impl<'a, 'b, 'v, 'tcx> Visitor<'v> for UnusedImportCheckVisitor<'a, 'b, 'tcx> { - fn visit_view_item(&mut self, vi: &ViewItem) { + fn visit_item(&mut self, item: &ast::Item) { // Ignore is_public import statements because there's no way to be sure // whether they're used or not. Also ignore imports with a dummy span // because this means that they were generated in some fashion by the // compiler and we don't need to consider them. - if vi.vis == ast::Public || vi.span == DUMMY_SP { - visit::walk_view_item(self, vi); + if item.vis == ast::Public || item.span == DUMMY_SP { + visit::walk_item(self, item); return; } - match vi.node { - ViewItemExternCrate(_, _, id) => { - if let Some(crate_num) = self.session.cstore.find_extern_mod_stmt_cnum(id) { + match item.node { + ast::ItemExternCrate(_) => { + if let Some(crate_num) = self.session.cstore.find_extern_mod_stmt_cnum(item.id) { if !self.used_crates.contains(&crate_num) { self.session.add_lint(lint::builtin::UNUSED_EXTERN_CRATES, - id, - vi.span, + item.id, + item.span, "unused extern crate".to_string()); } } }, - ViewItemUse(ref p) => { + ast::ItemUse(ref p) => { match p.node { - ViewPathSimple(_, _, id) => { - self.finalize_import(id, p.span) + ViewPathSimple(_, _) => { + self.finalize_import(item.id, p.span) } - ViewPathList(_, ref list, _) => { + ViewPathList(_, ref list) => { for i in list.iter() { self.finalize_import(i.node.id(), i.span); } } - ViewPathGlob(_, id) => { - if !self.used_imports.contains(&(id, TypeNS)) && - !self.used_imports.contains(&(id, ValueNS)) { + ViewPathGlob(_) => { + if !self.used_imports.contains(&(item.id, TypeNS)) && + !self.used_imports.contains(&(item.id, ValueNS)) { self.session .add_lint(lint::builtin::UNUSED_IMPORTS, - id, + item.id, p.span, "unused import".to_string()); } } } } + _ => {} } - visit::walk_view_item(self, vi); + visit::walk_item(self, item); } } diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index d4273e34a5b..e4453e8fc59 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -65,10 +65,10 @@ use syntax::ast::{ExprClosure, ExprForLoop, ExprLoop, ExprWhile, ExprMethodCall}; use syntax::ast::{ExprPath, ExprQPath, ExprStruct, FnDecl}; use syntax::ast::{ForeignItemFn, ForeignItemStatic, Generics}; -use syntax::ast::{Ident, ImplItem, Item, ItemConst, ItemEnum, ItemFn}; -use syntax::ast::{ItemForeignMod, ItemImpl, ItemMac, ItemMod, ItemStatic}; -use syntax::ast::{ItemStruct, ItemTrait, ItemTy, Local, LOCAL_CRATE}; -use syntax::ast::{MethodImplItem, Mod, Name, NodeId}; +use syntax::ast::{Ident, ImplItem, Item, ItemConst, ItemEnum, ItemExternCrate}; +use syntax::ast::{ItemFn, ItemForeignMod, ItemImpl, ItemMac, ItemMod, ItemStatic}; +use syntax::ast::{ItemStruct, ItemTrait, ItemTy, ItemUse}; +use syntax::ast::{Local, MethodImplItem, Mod, Name, NodeId}; use syntax::ast::{Pat, PatEnum, PatIdent, PatLit}; use syntax::ast::{PatRange, PatStruct, Path}; use syntax::ast::{PolyTraitRef, PrimTy, SelfExplicit}; @@ -1139,7 +1139,7 @@ fn record_import_use(&mut self, import_id: NodeId, name: Name) { } fn get_trait_name(&self, did: DefId) -> Name { - if did.krate == LOCAL_CRATE { + if did.krate == ast::LOCAL_CRATE { self.ast_map.expect_item(did.node).ident.name } else { csearch::get_trait_name(&self.session.cstore, did) @@ -2966,9 +2966,9 @@ fn resolve_item(&mut self, item: &Item) { }); } - ItemMac(..) => { + ItemExternCrate(_) | ItemUse(_) | ItemMac(..) => { // do nothing, these are just around to be encoded - } + } } } -- 2.44.0