From d5944074ecf1fb49e9934d923e43ac8bc3237f98 Mon Sep 17 00:00:00 2001 From: Inokentiy Babushkin Date: Sun, 23 Jul 2017 22:23:26 +0200 Subject: [PATCH] Improved the mapping handling. Closes #17. This uses the newly added crate tracking to make errors more discoverable and code more obvious in it's current behaviour. --- src/semcheck/mapping.rs | 60 ++++++++++++++++++++++++++++++++-------- src/semcheck/mismatch.rs | 17 ++++++------ src/semcheck/traverse.rs | 31 ++++++--------------- 3 files changed, 66 insertions(+), 42 deletions(-) diff --git a/src/semcheck/mapping.rs b/src/semcheck/mapping.rs index fc26922d8cf82..deb51d695f200 100644 --- a/src/semcheck/mapping.rs +++ b/src/semcheck/mapping.rs @@ -7,7 +7,7 @@ use rustc::hir::def::{Def, Export}; use rustc::hir::def_id::{CrateNum, DefId}; use rustc::ty::TypeParameterDef; -use std::collections::{BTreeSet, HashMap, VecDeque}; +use std::collections::{BTreeSet, HashMap, HashSet, VecDeque}; use syntax::ast::Name; @@ -23,6 +23,8 @@ pub struct IdMapping { new_crate: CrateNum, /// Toplevel items' old `DefId` mapped to old and new `Def`. toplevel_mapping: HashMap, + /// The set of toplevel items that have been removed. + removed_items: HashSet, /// Trait items' old `DefId` mapped to old and new `Def`. trait_item_mapping: HashMap, /// Other items' old `DefId` mapped to new `DefId`. @@ -39,6 +41,7 @@ impl IdMapping { old_crate: old_crate, new_crate: new_crate, toplevel_mapping: HashMap::new(), + removed_items: HashSet::new(), trait_item_mapping: HashMap::new(), internal_mapping: HashMap::new(), child_mapping: HashMap::new(), @@ -48,19 +51,30 @@ impl IdMapping { /// Register two exports representing the same item across versions. pub fn add_export(&mut self, old: Def, new: Def) -> bool { - if self.toplevel_mapping.contains_key(&old.def_id()) { + let old_def_id = old.def_id(); + + if !self.in_old_crate(old_def_id) || self.toplevel_mapping.contains_key(&old_def_id) { return false; } self.toplevel_mapping - .insert(old.def_id(), (old, new)); + .insert(old_def_id, (old, new)); true } + /// Register that an old item has no corresponding new item. + pub fn add_removal(&mut self, old: DefId) { + self.removed_items.insert(old); + } + /// Add any trait item's old and new `DefId`s. pub fn add_trait_item(&mut self, old: Def, new: Def, trait_def_id: DefId) { - self.trait_item_mapping.insert(old.def_id(), (old, new, trait_def_id)); + let old_def_id = old.def_id(); + + assert!(self.in_old_crate(old_def_id)); + + self.trait_item_mapping.insert(old_def_id, (old, new, trait_def_id)); } /// Add any other item's old and new `DefId`s. @@ -70,12 +84,14 @@ impl IdMapping { old, self.internal_mapping[&old], new); + assert!(self.in_old_crate(old)); self.internal_mapping.insert(old, new); } /// Add any other item's old and new `DefId`s, together with a parent entry. pub fn add_subitem(&mut self, parent: DefId, old: DefId, new: DefId) { + // NB: we rely on the assers in `add_internal_item` here. self.add_internal_item(old, new); self.child_mapping .entry(parent) @@ -85,16 +101,18 @@ impl IdMapping { /// Record that a `DefId` represents a new type parameter. pub fn add_type_param(&mut self, new: TypeParameterDef) { + assert!(self.in_new_crate(new.def_id)); self.type_params.insert(new.def_id, new); } /// Get the type parameter represented by a given `DefId`. - pub fn get_type_param(&self, def_id: &DefId) -> TypeParameterDef { - self.type_params[def_id] + pub fn get_type_param(&self, did: &DefId) -> TypeParameterDef { + self.type_params[did] } /// Check whether a `DefId` represents a newly added defaulted type parameter. pub fn is_defaulted_type_param(&self, new: &DefId) -> bool { + // TODO? self.type_params .get(new) .map_or(false, |def| def.has_default) @@ -102,12 +120,30 @@ impl IdMapping { /// Get the new `DefId` associated with the given old one. pub fn get_new_id(&self, old: DefId) -> DefId { - if let Some(new) = self.toplevel_mapping.get(&old) { - new.1.def_id() - } else if let Some(new) = self.trait_item_mapping.get(&old) { - new.1.def_id() + assert!(!self.in_new_crate(old)); + + if self.in_old_crate(old) && !self.removed_items.contains(&old) { + if let Some(new) = self.toplevel_mapping.get(&old) { + new.1.def_id() + } else if let Some(new) = self.trait_item_mapping.get(&old) { + new.1.def_id() + } else { + self.internal_mapping[&old] + } + } else { + old + } + } + + /// Get the new `DefId` associated with the given old one, respecting possibly removed + /// traits that are a parent of the given `DefId`. + pub fn get_new_trait_item_id(&self, old: DefId, trait_id: DefId) -> DefId { + assert!(!self.in_new_crate(trait_id)); + + if !self.removed_items.contains(&trait_id) { + self.get_new_id(old) } else { - self.internal_mapping[&old] + old } } @@ -148,10 +184,12 @@ impl IdMapping { .map(|m| m.iter().map(move |old| (*old, self.internal_mapping[old]))) } + /// Check whether a `DefId` belongs to an item in the old crate. pub fn in_old_crate(&self, did: DefId) -> bool { self.old_crate == did.krate } + /// Check whether a `DefId` belongs to an item in the new crate. pub fn in_new_crate(&self, did: DefId) -> bool { self.new_crate == did.krate } diff --git a/src/semcheck/mismatch.rs b/src/semcheck/mismatch.rs index caf78435f378a..af3c4192cc476 100644 --- a/src/semcheck/mismatch.rs +++ b/src/semcheck/mismatch.rs @@ -111,14 +111,12 @@ impl<'a, 'gcx, 'tcx> TypeRelation<'a, 'gcx, 'tcx> for Mismatch<'a, 'gcx, 'tcx> { .collect(); for field in a_adt.all_fields().filter(|f| f.vis == Public) { - if self.id_mapping.contains_id(field.did) { - let a_field_ty = field.ty(self.tcx, a_substs); - let b_field_ty = - b_fields[&self.id_mapping.get_new_id(field.did)] - .ty(self.tcx, b_substs); - - let _ = self.relate(&a_field_ty, &b_field_ty)?; - } + let a_field_ty = field.ty(self.tcx, a_substs); + let b_field_ty = + b_fields[&self.id_mapping.get_new_id(field.did)] + .ty(self.tcx, b_substs); + + let _ = self.relate(&a_field_ty, &b_field_ty)?; } Some((a_def.did, b_def.did)) @@ -188,7 +186,8 @@ impl<'a, 'gcx, 'tcx> TypeRelation<'a, 'gcx, 'tcx> for Mismatch<'a, 'gcx, 'tcx> { }; if let Some((old_def_id, new_def_id)) = matching { - if !self.id_mapping.contains_id(old_def_id) && self.id_mapping.in_old_crate(old_def_id) { + if !self.id_mapping.contains_id(old_def_id) && + self.id_mapping.in_old_crate(old_def_id) { self.id_mapping.add_internal_item(old_def_id, new_def_id); self.item_queue.push_back((old_def_id, new_def_id)); } diff --git a/src/semcheck/traverse.rs b/src/semcheck/traverse.rs index fc08e411c256b..49e357b36bb0a 100644 --- a/src/semcheck/traverse.rs +++ b/src/semcheck/traverse.rs @@ -261,6 +261,7 @@ fn diff_structure<'a, 'tcx>(changes: &mut ChangeSet, changes.new_path_change(n_def_id, o.ident.name, tcx.def_span(n_def_id)); changes.add_path_removal(n_def_id, o.span); } else { + id_mapping.add_removal(o_def_id); changes.new_path_change(o_def_id, o.ident.name, tcx.def_span(o_def_id)); changes.add_path_removal(o_def_id, o.span); } @@ -732,7 +733,7 @@ fn fold_to_new<'a, 'tcx, T>(id_mapping: &IdMapping, old.fold_with(&mut BottomUpFolder { tcx: tcx, fldop: |ty| { match ty.sty { - TyAdt(&AdtDef { ref did, .. }, substs) if id_mapping.contains_id(*did) => { + TyAdt(&AdtDef { ref did, .. }, substs) if id_mapping.in_old_crate(*did) => { let new_def_id = id_mapping.get_new_id(*did); let new_adt = tcx.adt_def(new_def_id); tcx.mk_adt(new_adt, substs) @@ -747,11 +748,7 @@ fn fold_to_new<'a, 'tcx, T>(id_mapping: &IdMapping, let new_preds = tcx.mk_existential_predicates(preds.iter().map(|p| { match *p.skip_binder() { Trait(ExistentialTraitRef { def_id: did, substs }) => { - let new_def_id = if id_mapping.contains_id(did) { - id_mapping.get_new_id(did) - } else { - did - }; + let new_def_id = id_mapping.get_new_id(did); Trait(ExistentialTraitRef { def_id: new_def_id, @@ -759,11 +756,7 @@ fn fold_to_new<'a, 'tcx, T>(id_mapping: &IdMapping, }) }, Projection(ExistentialProjection { item_def_id, substs, ty }) => { - let new_def_id = if id_mapping.contains_id(item_def_id) { - id_mapping.get_new_id(item_def_id) - } else { - item_def_id - }; + let new_def_id = id_mapping.get_new_id(item_def_id); Projection(ExistentialProjection { item_def_id: new_def_id, @@ -772,11 +765,7 @@ fn fold_to_new<'a, 'tcx, T>(id_mapping: &IdMapping, }) }, AutoTrait(did) => { - if id_mapping.contains_id(did) { - AutoTrait(id_mapping.get_new_id(did)) - } else { - AutoTrait(did) - } + AutoTrait(id_mapping.get_new_id(did)) }, } })); @@ -784,11 +773,9 @@ fn fold_to_new<'a, 'tcx, T>(id_mapping: &IdMapping, tcx.mk_dynamic(Binder(new_preds), region) }, TyProjection(proj) => { - let new_def_id = if id_mapping.contains_id(proj.item_def_id) { - id_mapping.get_new_id(proj.item_def_id) - } else { - proj.item_def_id - }; + let trait_def_id = tcx.associated_item(proj.item_def_id).container.id(); + let new_def_id = + id_mapping.get_new_trait_item_id(proj.item_def_id, trait_def_id); tcx.mk_projection(new_def_id, proj.substs) }, @@ -798,7 +785,7 @@ fn fold_to_new<'a, 'tcx, T>(id_mapping: &IdMapping, TyParam(param) => { if param.idx != 0 { // `Self` is special let old_did = index_map[¶m.idx]; - if id_mapping.contains_id(old_did) { + if id_mapping.in_old_crate(old_did) { let new_def_id = id_mapping.get_new_id(old_did); tcx.mk_param_from_def(&id_mapping.get_type_param(&new_def_id)) } else {