From e8ea38e42a7e361e37b1cd3622f3baf1c8055986 Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Thu, 15 Sep 2016 00:51:46 +0300 Subject: [PATCH] Further cleanup in resolve `try_define` is not used in build_reduced_graph anymore Collection of field names for error reporting is optimized Some comments added --- src/librustc_metadata/decoder.rs | 2 + src/librustc_resolve/build_reduced_graph.rs | 114 ++++++++------------ src/librustc_resolve/lib.rs | 14 ++- 3 files changed, 54 insertions(+), 76 deletions(-) diff --git a/src/librustc_metadata/decoder.rs b/src/librustc_metadata/decoder.rs index 59a33fcbbcd..8aa9d3cf9ca 100644 --- a/src/librustc_metadata/decoder.rs +++ b/src/librustc_metadata/decoder.rs @@ -699,6 +699,8 @@ impl<'a, 'tcx> CrateMetadata { } } Def::Variant(def_id) => { + // Braced variants, unlike structs, generate unusable names in + // value namespace, they are reserved for possible future use. let vkind = self.get_variant_kind(child_index).unwrap(); let ctor_def = Def::VariantCtor(def_id, vkind.ctor_kind()); callback(def::Export { def: ctor_def, name: name }); diff --git a/src/librustc_resolve/build_reduced_graph.rs b/src/librustc_resolve/build_reduced_graph.rs index b6afacf05e2..f689ed6a41c 100644 --- a/src/librustc_resolve/build_reduced_graph.rs +++ b/src/librustc_resolve/build_reduced_graph.rs @@ -31,7 +31,6 @@ use std::rc::Rc; use syntax::ast::Name; use syntax::attr; -use syntax::parse::token; use syntax::ast::{self, Block, ForeignItem, ForeignItemKind, Item, ItemKind}; use syntax::ast::{Mutability, StmtKind, TraitItem, TraitItemKind}; @@ -77,6 +76,12 @@ impl<'b> Resolver<'b> { }) } + fn insert_field_names(&mut self, def_id: DefId, field_names: Vec) { + if !field_names.is_empty() { + self.field_names.insert(def_id, field_names); + } + } + /// Constructs the reduced graph for one item. fn build_reduced_graph_for_item(&mut self, item: &Item, expansion: Mark) { let parent = self.current_module; @@ -301,28 +306,26 @@ impl<'b> Resolver<'b> { self.define(parent, name, ValueNS, (ctor_def, sp, vis)); } - // Record the def ID and fields of this struct. - let field_names = struct_def.fields().iter().enumerate().map(|(index, field)| { + // Record field names for error reporting. + let field_names = struct_def.fields().iter().filter_map(|field| { self.resolve_visibility(&field.vis); field.ident.map(|ident| ident.name) - .unwrap_or_else(|| token::intern(&index.to_string())) }).collect(); let item_def_id = self.definitions.local_def_id(item.id); - self.structs.insert(item_def_id, field_names); + self.insert_field_names(item_def_id, field_names); } ItemKind::Union(ref vdata, _) => { let def = Def::Union(self.definitions.local_def_id(item.id)); self.define(parent, name, TypeNS, (def, sp, vis)); - // Record the def ID and fields of this union. - let field_names = vdata.fields().iter().enumerate().map(|(index, field)| { + // Record field names for error reporting. + let field_names = vdata.fields().iter().filter_map(|field| { self.resolve_visibility(&field.vis); field.ident.map(|ident| ident.name) - .unwrap_or_else(|| token::intern(&index.to_string())) }).collect(); let item_def_id = self.definitions.local_def_id(item.id); - self.structs.insert(item_def_id, field_names); + self.insert_field_names(item_def_id, field_names); } ItemKind::DefaultImpl(..) | ItemKind::Impl(..) => {} @@ -347,18 +350,17 @@ impl<'b> Resolver<'b> { parent: Module<'b>, vis: ty::Visibility) { let name = variant.node.name.name; - let ctor_kind = CtorKind::from_vdata(&variant.node.data); - if variant.node.data.is_struct() { - // Not adding fields for variants as they are not accessed with a self receiver - let variant_def_id = self.definitions.local_def_id(variant.node.data.id()); - self.structs.insert(variant_def_id, Vec::new()); - } + let def_id = self.definitions.local_def_id(variant.node.data.id()); - // All variants are defined in both type and value namespaces as future-proofing. - let def = Def::Variant(self.definitions.local_def_id(variant.node.data.id())); - let ctor_def = Def::VariantCtor(self.definitions.local_def_id(variant.node.data.id()), - ctor_kind); + // Define a name in the type namespace. + let def = Def::Variant(def_id); self.define(parent, name, TypeNS, (def, variant.span, vis)); + + // 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_vdata(&variant.node.data); + let ctor_def = Def::VariantCtor(def_id, ctor_kind); self.define(parent, name, ValueNS, (ctor_def, variant.span, vis)); } @@ -407,79 +409,55 @@ impl<'b> Resolver<'b> { }; match def { - Def::Mod(_) | Def::Enum(..) => { - debug!("(building reduced graph for external crate) building module {} {:?}", - name, vis); + Def::Mod(..) | Def::Enum(..) => { let module = self.new_module(parent, ModuleKind::Def(def, name), false); - let _ = self.try_define(parent, name, TypeNS, (module, DUMMY_SP, vis)); + self.define(parent, name, TypeNS, (module, DUMMY_SP, vis)); } Def::Variant(..) => { - debug!("(building reduced graph for external crate) building variant {}", name); - // All variants are defined in both type and value namespaces as future-proofing. - let vkind = self.session.cstore.variant_kind(def_id).unwrap(); - let _ = self.try_define(parent, name, TypeNS, (def, DUMMY_SP, vis)); - if vkind == ty::VariantKind::Struct { - // Not adding fields for variants as they are not accessed with a self receiver - self.structs.insert(def_id, Vec::new()); - } + self.define(parent, name, TypeNS, (def, DUMMY_SP, vis)); } Def::VariantCtor(..) => { - let _ = self.try_define(parent, name, ValueNS, (def, DUMMY_SP, vis)); + self.define(parent, name, ValueNS, (def, DUMMY_SP, vis)); } Def::Fn(..) | Def::Static(..) | Def::Const(..) | Def::AssociatedConst(..) | Def::Method(..) => { - debug!("(building reduced graph for external crate) building value (fn/static) {}", - name); - let _ = self.try_define(parent, name, ValueNS, (def, DUMMY_SP, vis)); + self.define(parent, name, ValueNS, (def, DUMMY_SP, vis)); } - Def::Trait(_) => { - debug!("(building reduced graph for external crate) building type {}", name); - - // If this is a trait, add all the trait item names to the trait - // info. + Def::Trait(..) => { + let module = self.new_module(parent, ModuleKind::Def(def, name), false); + self.define(parent, name, TypeNS, (module, DUMMY_SP, vis)); + // If this is a trait, add all the trait item names to the trait info. let trait_item_def_ids = self.session.cstore.impl_or_trait_items(def_id); - for &trait_item_def in &trait_item_def_ids { - let trait_item_name = - self.session.cstore.def_key(trait_item_def) - .disambiguated_data.data.get_opt_name() - .expect("opt_item_name returned None for trait"); - - debug!("(building reduced graph for external crate) ... adding trait item \ - '{}'", - trait_item_name); - + for trait_item_def_id in trait_item_def_ids { + let trait_item_name = self.session.cstore.def_key(trait_item_def_id) + .disambiguated_data.data.get_opt_name() + .expect("opt_item_name returned None for trait"); self.trait_item_map.insert((trait_item_name, def_id), false); } - - let module = self.new_module(parent, ModuleKind::Def(def, name), false); - let _ = self.try_define(parent, name, TypeNS, (module, DUMMY_SP, vis)); } Def::TyAlias(..) | Def::AssociatedTy(..) => { - debug!("(building reduced graph for external crate) building type {}", name); - let _ = self.try_define(parent, name, TypeNS, (def, DUMMY_SP, vis)); + self.define(parent, name, TypeNS, (def, DUMMY_SP, vis)); } Def::Struct(..) => { - debug!("(building reduced graph for external crate) building type and value for {}", - name); - let _ = self.try_define(parent, name, TypeNS, (def, DUMMY_SP, vis)); + self.define(parent, name, TypeNS, (def, DUMMY_SP, vis)); - // Record the def ID and fields of this struct. - let fields = self.session.cstore.struct_field_names(def_id); - self.structs.insert(def_id, fields); + // 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(..) => { - let _ = self.try_define(parent, name, ValueNS, (def, DUMMY_SP, vis)); + self.define(parent, name, ValueNS, (def, DUMMY_SP, vis)); } - Def::Union(_) => { - let _ = self.try_define(parent, name, TypeNS, (def, DUMMY_SP, vis)); + Def::Union(..) => { + self.define(parent, name, TypeNS, (def, DUMMY_SP, vis)); - // Record the def ID and fields of this union. - let fields = self.session.cstore.struct_field_names(def_id); - self.structs.insert(def_id, fields); + // 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::Local(..) | Def::PrimTy(..) | @@ -488,7 +466,7 @@ impl<'b> Resolver<'b> { Def::Label(..) | Def::SelfTy(..) | Def::Err => { - bug!("didn't expect `{:?}`", def); + bug!("unexpected definition: {:?}", def); } } } diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index 534b3e39879..3dbe4d91536 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -1006,7 +1006,9 @@ pub struct Resolver<'a> { trait_item_map: FnvHashMap<(Name, DefId), bool /* is static method? */>, - structs: FnvHashMap>, + // Names of fields of an item `DefId` accessible with dot syntax. + // Used for hints during error reporting. + field_names: FnvHashMap>, // All imports known to succeed or fail. determined_imports: Vec<&'a ImportDirective<'a>>, @@ -1218,7 +1220,7 @@ impl<'a> Resolver<'a> { prelude: None, trait_item_map: FnvHashMap(), - structs: FnvHashMap(), + field_names: FnvHashMap(), determined_imports: Vec::new(), indeterminate_imports: Vec::new(), @@ -2779,8 +2781,8 @@ impl<'a> Resolver<'a> { match resolution.base_def { Def::Enum(did) | Def::TyAlias(did) | Def::Union(did) | Def::Struct(did) | Def::Variant(did) if resolution.depth == 0 => { - if let Some(fields) = self.structs.get(&did) { - if fields.iter().any(|&field_name| name == field_name) { + if let Some(field_names) = self.field_names.get(&did) { + if field_names.iter().any(|&field_name| name == field_name) { return Field; } } @@ -2852,7 +2854,6 @@ impl<'a> Resolver<'a> { _ => false, }; if is_struct_variant { - let _ = self.structs.contains_key(&path_res.base_def.def_id()); let path_name = path_names_to_string(path, 0); let mut err = resolve_struct_error(self, @@ -2885,9 +2886,6 @@ impl<'a> Resolver<'a> { } } else { // Be helpful if the name refers to a struct - // (The pattern matching def_tys where the id is in self.structs - // matches on regular structs while excluding tuple- and enum-like - // structs, which wouldn't result in this error.) let path_name = path_names_to_string(path, 0); let type_res = self.with_no_errors(|this| { this.resolve_path(expr.id, path, 0, TypeNS)