nordic-dev.net/rust
nordic-dev.net/rust
2
0
Fork 0
mirror of https://github.com/rust-lang/rust.git synced 2025-06-04 19:29:07 +00:00
Code Issues Packages Projects Releases Wiki Activity

Revert "correct subtle bug in the type variable code"

Browse Source 
This reverts commit ccd92c2a4e.

This commit is the source of a major perf regression, and was not
intended to be included in #47861. At some point I must have
accidentally re-added the commit.
This commit is contained in:
Sean Griffin 2018-03-02 06:52:12 -07:00
parent 9cb18a92ad
commit f5f53e9677
1 changed files with 107 additions and 60 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

167
src/librustc/infer/type_variable.rs
View File

@ -16,15 +16,11 @@ use std::cmp;
use std::marker::PhantomData; use std::marker::PhantomData;
use std::u32; use std::u32;
use rustc_data_structures::fx::FxHashMap; use rustc_data_structures::fx::FxHashMap;
use rustc_data_structures::snapshot_vec as sv;
use rustc_data_structures::unify as ut; use rustc_data_structures::unify as ut;
pub struct TypeVariableTable<'tcx> { pub struct TypeVariableTable<'tcx> {
/// Extra data for each type variable, such as the origin. This is values: sv::SnapshotVec<Delegate>,
/// not stored in the unification table since, when we inquire
/// after the origin of a variable X, we want the origin of **that
/// variable X**, not the origin of some other variable Y with
/// which X has been unified.
var_data: Vec<TypeVariableData>,
/// Two variables are unified in `eq_relations` when we have a /// Two variables are unified in `eq_relations` when we have a
/// constraint `?X == ?Y`. This table also stores, for each key, /// constraint `?X == ?Y`. This table also stores, for each key,
@ -118,20 +114,21 @@ impl<'tcx> TypeVariableValue<'tcx> {
} }
pub struct Snapshot<'tcx> { pub struct Snapshot<'tcx> {
/// number of variables at the time of the snapshot snapshot: sv::Snapshot,
num_vars: usize,
/// snapshot from the `eq_relations` table
eq_snapshot: ut::Snapshot<ut::InPlace<TyVidEqKey<'tcx>>>, eq_snapshot: ut::Snapshot<ut::InPlace<TyVidEqKey<'tcx>>>,
/// snapshot from the `sub_relations` table
sub_snapshot: ut::Snapshot<ut::InPlace<ty::TyVid>>, sub_snapshot: ut::Snapshot<ut::InPlace<ty::TyVid>>,
} }
struct Instantiate {
vid: ty::TyVid,
}
struct Delegate;
impl<'tcx> TypeVariableTable<'tcx> { impl<'tcx> TypeVariableTable<'tcx> {
pub fn new() -> TypeVariableTable<'tcx> { pub fn new() -> TypeVariableTable<'tcx> {
TypeVariableTable { TypeVariableTable {
var_data: Vec::new(), values: sv::SnapshotVec::new(),
eq_relations: ut::UnificationTable::new(), eq_relations: ut::UnificationTable::new(),
sub_relations: ut::UnificationTable::new(), sub_relations: ut::UnificationTable::new(),
} }
@ -142,7 +139,7 @@ impl<'tcx> TypeVariableTable<'tcx> {
/// Note that this function does not return care whether /// Note that this function does not return care whether
/// `vid` has been unified with something else or not. /// `vid` has been unified with something else or not.
pub fn var_diverges<'a>(&'a self, vid: ty::TyVid) -> bool { pub fn var_diverges<'a>(&'a self, vid: ty::TyVid) -> bool {
self.var_data[vid.index as usize].diverging self.values.get(vid.index as usize).diverging
} }
/// Returns the origin that was given when `vid` was created. /// Returns the origin that was given when `vid` was created.
@ -150,7 +147,7 @@ impl<'tcx> TypeVariableTable<'tcx> {
/// Note that this function does not return care whether /// Note that this function does not return care whether
/// `vid` has been unified with something else or not. /// `vid` has been unified with something else or not.
pub fn var_origin(&self, vid: ty::TyVid) -> &TypeVariableOrigin { pub fn var_origin(&self, vid: ty::TyVid) -> &TypeVariableOrigin {
&self.var_data[vid.index as usize].origin &self.values.get(vid.index as usize).origin
} }
/// Records that `a == b`, depending on `dir`. /// Records that `a == b`, depending on `dir`.
@ -182,6 +179,11 @@ impl<'tcx> TypeVariableTable<'tcx> {
"instantiating type variable `{:?}` twice: new-value = {:?}, old-value={:?}", "instantiating type variable `{:?}` twice: new-value = {:?}, old-value={:?}",
vid, ty, self.eq_relations.probe_value(vid)); vid, ty, self.eq_relations.probe_value(vid));
self.eq_relations.union_value(vid, TypeVariableValue::Known { value: ty }); self.eq_relations.union_value(vid, TypeVariableValue::Known { value: ty });
// Hack: we only need this so that `types_escaping_snapshot`
// can see what has been unified; see the Delegate impl for
// more details.
self.values.record(Instantiate { vid: vid });
} }
/// Creates a new type variable. /// Creates a new type variable.
@ -204,8 +206,11 @@ impl<'tcx> TypeVariableTable<'tcx> {
let sub_key = self.sub_relations.new_key(()); let sub_key = self.sub_relations.new_key(());
assert_eq!(eq_key.vid, sub_key); assert_eq!(eq_key.vid, sub_key);
assert_eq!(self.var_data.len(), sub_key.index as usize); let index = self.values.push(TypeVariableData {
self.var_data.push(TypeVariableData { origin, diverging }); origin,
diverging,
});
assert_eq!(eq_key.vid.index, index as u32);
debug!("new_var(index={:?}, diverging={:?}, origin={:?}", eq_key.vid, diverging, origin); debug!("new_var(index={:?}, diverging={:?}, origin={:?}", eq_key.vid, diverging, origin);
@ -214,7 +219,7 @@ impl<'tcx> TypeVariableTable<'tcx> {
/// Returns the number of type variables created thus far. /// Returns the number of type variables created thus far.
pub fn num_vars(&self) -> usize { pub fn num_vars(&self) -> usize {
self.var_data.len() self.values.len()
} }
/// Returns the "root" variable of `vid` in the `eq_relations` /// Returns the "root" variable of `vid` in the `eq_relations`
@ -270,7 +275,7 @@ impl<'tcx> TypeVariableTable<'tcx> {
/// be processed in a stack-like fashion. /// be processed in a stack-like fashion.
pub fn snapshot(&mut self) -> Snapshot<'tcx> { pub fn snapshot(&mut self) -> Snapshot<'tcx> {
Snapshot { Snapshot {
num_vars: self.var_data.len(), snapshot: self.values.start_snapshot(),
eq_snapshot: self.eq_relations.snapshot(), eq_snapshot: self.eq_relations.snapshot(),
sub_snapshot: self.sub_relations.snapshot(), sub_snapshot: self.sub_relations.snapshot(),
} }
@ -280,12 +285,21 @@ impl<'tcx> TypeVariableTable<'tcx> {
/// snapshots created since that point must already have been /// snapshots created since that point must already have been
/// committed or rolled back. /// committed or rolled back.
pub fn rollback_to(&mut self, s: Snapshot<'tcx>) { pub fn rollback_to(&mut self, s: Snapshot<'tcx>) {
let Snapshot { num_vars, eq_snapshot, sub_snapshot } = s; debug!("rollback_to{:?}", {
debug!("type_variables::rollback_to(num_vars = {})", num_vars); for action in self.values.actions_since_snapshot(&s.snapshot) {
assert!(self.var_data.len() >= num_vars); match *action {
sv::UndoLog::NewElem(index) => {
debug!("inference variable _#{}t popped", index)
}
_ => { }
}
}
});
let Snapshot { snapshot, eq_snapshot, sub_snapshot } = s;
self.values.rollback_to(snapshot);
self.eq_relations.rollback_to(eq_snapshot); self.eq_relations.rollback_to(eq_snapshot);
self.sub_relations.rollback_to(sub_snapshot); self.sub_relations.rollback_to(sub_snapshot);
self.var_data.truncate(num_vars);
} }
/// Commits all changes since the snapshot was created, making /// Commits all changes since the snapshot was created, making
@ -293,8 +307,8 @@ impl<'tcx> TypeVariableTable<'tcx> {
/// another snapshot). Any snapshots created since that point /// another snapshot). Any snapshots created since that point
/// must already have been committed or rolled back. /// must already have been committed or rolled back.
pub fn commit(&mut self, s: Snapshot<'tcx>) { pub fn commit(&mut self, s: Snapshot<'tcx>) {
let Snapshot { num_vars, eq_snapshot, sub_snapshot } = s; let Snapshot { snapshot, eq_snapshot, sub_snapshot } = s;
debug!("type_variables::commit(num_vars = {})", num_vars); self.values.commit(snapshot);
self.eq_relations.commit(eq_snapshot); self.eq_relations.commit(eq_snapshot);
self.sub_relations.commit(sub_snapshot); self.sub_relations.commit(sub_snapshot);
} }
@ -303,12 +317,19 @@ impl<'tcx> TypeVariableTable<'tcx> {
/// ty-variables created during the snapshot, and the values /// ty-variables created during the snapshot, and the values
/// `{V2}` are the root variables that they were unified with, /// `{V2}` are the root variables that they were unified with,
/// along with their origin. /// along with their origin.
pub fn types_created_since_snapshot(&mut self, snapshot: &Snapshot<'tcx>) -> TypeVariableMap { pub fn types_created_since_snapshot(&mut self, s: &Snapshot<'tcx>) -> TypeVariableMap {
self.var_data let actions_since_snapshot = self.values.actions_since_snapshot(&s.snapshot);
actions_since_snapshot
.iter() .iter()
.enumerate() .filter_map(|action| match action {
.skip(snapshot.num_vars) // skip those that existed when snapshot was taken &sv::UndoLog::NewElem(index) => Some(ty::TyVid { index: index as u32 }),
.map(|(index, data)| (ty::TyVid { index: index as u32 }, data.origin)) _ => None,
})
.map(|vid| {
let origin = self.values.get(vid.index as usize).origin.clone();
(vid, origin)
})
.collect() .collect()
} }
@ -318,42 +339,47 @@ impl<'tcx> TypeVariableTable<'tcx> {
/// a type variable `V0`, then we started the snapshot, then we /// a type variable `V0`, then we started the snapshot, then we
/// created a type variable `V1`, unifed `V0` with `T0`, and /// created a type variable `V1`, unifed `V0` with `T0`, and
/// unified `V1` with `T1`, this function would return `{T0}`. /// unified `V1` with `T1`, this function would return `{T0}`.
pub fn types_escaping_snapshot(&mut self, snapshot: &Snapshot<'tcx>) -> Vec<Ty<'tcx>> { pub fn types_escaping_snapshot(&mut self, s: &Snapshot<'tcx>) -> Vec<Ty<'tcx>> {
// We want to select only those instantiations that have let mut new_elem_threshold = u32::MAX;
// occurred since the snapshot *and* which affect some let mut escaping_types = Vec::new();
// variable that existed prior to the snapshot. This code just let actions_since_snapshot = self.values.actions_since_snapshot(&s.snapshot);
// affects all instantiatons that ever occurred which affect debug!("actions_since_snapshot.len() = {}", actions_since_snapshot.len());
// variables prior to the snapshot. for action in actions_since_snapshot {
// match *action {
// It's hard to do better than this, though, without changing sv::UndoLog::NewElem(index) => {
// the unification table to prefer "lower" vids -- the problem // if any new variables were created during the
// is that we may have a variable X (from before the snapshot) // snapshot, remember the lower index (which will
// and Y (from after the snapshot) which get unified, with Y // always be the first one we see). Note that this
// chosen as the new root. Now we are "instantiating" Y with a // action must precede those variables being
// value, but it escapes into X, but we wouldn't readily see // specified.
// that. (In fact, earlier revisions of this code had this new_elem_threshold = cmp::min(new_elem_threshold, index as u32);
// bug; it was introduced when we added the `eq_relations` debug!("NewElem({}) new_elem_threshold={}", index, new_elem_threshold);
// table, but it's hard to create rust code that triggers it.) }
//
// We could tell the table to prefer lower vids, and then we would sv::UndoLog::Other(Instantiate { vid, .. }) => {
// see the case above, but we would get less-well-balanced trees. if vid.index < new_elem_threshold {
// // quick check to see if this variable was
// Since I hope to kill the leak-check in this branch, and // created since the snapshot started or not.
// that's the code which uses this logic anyway, I'm going to let escaping_type = match self.eq_relations.probe_value(vid) {
// use the less efficient algorithm for now. TypeVariableValue::Unknown { .. } => bug!(),
let mut escaping_types = Vec::with_capacity(snapshot.num_vars); TypeVariableValue::Known { value } => value,
escaping_types.extend( };
(0..snapshot.num_vars) // for all variables that pre-exist the snapshot, collect.. escaping_types.push(escaping_type);
.map(|i| ty::TyVid { index: i as u32 }) }
.filter_map(|vid| self.probe(vid).known())); // ..types they are instantiated with. debug!("SpecifyVar({:?}) new_elem_threshold={}", vid, new_elem_threshold);
debug!("types_escaping_snapshot = {:?}", escaping_types); }
_ => { }
}
}
escaping_types escaping_types
} }
/// Returns indices of all variables that are not yet /// Returns indices of all variables that are not yet
/// instantiated. /// instantiated.
pub fn unsolved_variables(&mut self) -> Vec<ty::TyVid> { pub fn unsolved_variables(&mut self) -> Vec<ty::TyVid> {
(0..self.var_data.len()) (0..self.values.len())
.filter_map(|i| { .filter_map(|i| {
let vid = ty::TyVid { index: i as u32 }; let vid = ty::TyVid { index: i as u32 };
match self.probe(vid) { match self.probe(vid) {
@ -364,6 +390,27 @@ impl<'tcx> TypeVariableTable<'tcx> {
.collect() .collect()
} }
} }
impl sv::SnapshotVecDelegate for Delegate {
type Value = TypeVariableData;
type Undo = Instantiate;
fn reverse(_values: &mut Vec<TypeVariableData>, _action: Instantiate) {
// We don't actually have to *do* anything to reverse an
// instanation; the value for a variable is stored in the
// `eq_relations` and hence its rollback code will handle
// it. In fact, we could *almost* just remove the
// `SnapshotVec` entirely, except that we would have to
// reproduce *some* of its logic, since we want to know which
// type variables have been instantiated since the snapshot
// was started, so we can implement `types_escaping_snapshot`.
//
// (If we extended the `UnificationTable` to let us see which
// values have been unified and so forth, that might also
// suffice.)
}
}
/////////////////////////////////////////////////////////////////////////// ///////////////////////////////////////////////////////////////////////////
/// These structs (a newtyped TyVid) are used as the unification key /// These structs (a newtyped TyVid) are used as the unification key