Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor variance and remove last [pub] map #41734

Merged
merged 10 commits into from
May 6, 2017
6 changes: 6 additions & 0 deletions src/librustc/dep_graph/dep_node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ pub enum DepNode<D: Clone + Debug> {
TransCrateItem(D),
TransInlinedItem(D),
TransWriteMetadata,
CrateVariances,

// Nodes representing bits of computed IR in the tcx. Each shared
// table in the tcx (or elsewhere) maps to one of these
Expand All @@ -89,6 +90,8 @@ pub enum DepNode<D: Clone + Debug> {
// predicates for an item wind up in `ItemSignature`).
AssociatedItems(D),
ItemSignature(D),
ItemVarianceConstraints(D),
ItemVariances(D),
IsForeignItem(D),
TypeParamPredicates((D, D)),
SizedConstraint(D),
Expand Down Expand Up @@ -199,6 +202,7 @@ impl<D: Clone + Debug> DepNode<D> {
MirKrate => Some(MirKrate),
TypeckBodiesKrate => Some(TypeckBodiesKrate),
Coherence => Some(Coherence),
CrateVariances => Some(CrateVariances),
Resolve => Some(Resolve),
Variance => Some(Variance),
PrivacyAccessLevels(k) => Some(PrivacyAccessLevels(k)),
Expand Down Expand Up @@ -230,6 +234,8 @@ impl<D: Clone + Debug> DepNode<D> {
TransInlinedItem(ref d) => op(d).map(TransInlinedItem),
AssociatedItems(ref d) => op(d).map(AssociatedItems),
ItemSignature(ref d) => op(d).map(ItemSignature),
ItemVariances(ref d) => op(d).map(ItemVariances),
ItemVarianceConstraints(ref d) => op(d).map(ItemVarianceConstraints),
IsForeignItem(ref d) => op(d).map(IsForeignItem),
TypeParamPredicates((ref item, ref param)) => {
Some(TypeParamPredicates((try_opt!(op(item)), try_opt!(op(param)))))
Expand Down
4 changes: 0 additions & 4 deletions src/librustc/ty/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -468,9 +468,6 @@ pub struct GlobalCtxt<'tcx> {

pub lang_items: middle::lang_items::LanguageItems,

/// True if the variance has been computed yet; false otherwise.
pub variance_computed: Cell<bool>,

/// Set of used unsafe nodes (functions or blocks). Unsafe nodes not
/// present in this set can be warned about.
pub used_unsafe: RefCell<NodeSet>,
Expand Down Expand Up @@ -753,7 +750,6 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
dep_graph: dep_graph.clone(),
types: common_types,
named_region_map: named_region_map,
variance_computed: Cell::new(false),
trait_map: resolutions.trait_map,
export_map: resolutions.export_map,
fulfilled_predicates: RefCell::new(fulfilled_predicates),
Expand Down
16 changes: 15 additions & 1 deletion src/librustc/ty/maps.rs
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,12 @@ impl<'tcx> QueryDescription for queries::crate_inherent_impls_overlap_check<'tcx
}
}

impl<'tcx> QueryDescription for queries::crate_variances<'tcx> {
fn describe(_tcx: TyCtxt, _: CrateNum) -> String {
format!("computing the variances for items in this crate")
}
}

impl<'tcx> QueryDescription for queries::mir_shims<'tcx> {
fn describe(tcx: TyCtxt, def: ty::InstanceDef<'tcx>) -> String {
format!("generating MIR shim for `{}`",
Expand Down Expand Up @@ -673,9 +679,13 @@ define_maps! { <'tcx>
/// True if this is a foreign item (i.e., linked via `extern { ... }`).
[] is_foreign_item: IsForeignItem(DefId) -> bool,

/// Get a map with the variance of every item; use `item_variance`
/// instead.
[] crate_variances: crate_variances(CrateNum) -> Rc<ty::CrateVariancesMap>,

/// Maps from def-id of a type or region parameter to its
/// (inferred) variance.
[pub] variances_of: ItemSignature(DefId) -> Rc<Vec<ty::Variance>>,
[pub] variances_of: ItemVariances(DefId) -> Rc<Vec<ty::Variance>>,

/// Maps from an impl/trait def-id to a list of the def-ids of its items
[] associated_item_def_ids: AssociatedItemDefIds(DefId) -> Rc<Vec<DefId>>,
Expand Down Expand Up @@ -810,3 +820,7 @@ fn const_eval_dep_node((def_id, _): (DefId, &Substs)) -> DepNode<DefId> {
fn mir_keys(_: CrateNum) -> DepNode<DefId> {
DepNode::MirKeys
}

fn crate_variances(_: CrateNum) -> DepNode<DefId> {
DepNode::CrateVariances
}
22 changes: 22 additions & 0 deletions src/librustc/ty/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ use rustc_const_math::ConstInt;
use rustc_data_structures::accumulate_vec::IntoIter as AccIntoIter;
use rustc_data_structures::stable_hasher::{StableHasher, StableHasherResult,
HashStable};
use rustc_data_structures::transitive_relation::TransitiveRelation;

use hir;
use hir::itemlikevisit::ItemLikeVisitor;
Expand Down Expand Up @@ -309,6 +310,27 @@ pub enum Variance {
Bivariant, // T<A> <: T<B> -- e.g., unused type parameter
}

/// The crate variances map is computed during typeck and contains the
/// variance of every item in the local crate. You should not use it
/// directly, because to do so will make your pass dependent on the
/// HIR of every item in the local crate. Instead, use
/// `tcx.item_variances()` to get the variance for a *particular*
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should probably say tcx.variances_of().

/// item.
pub struct CrateVariancesMap {
/// This relation tracks the dependencies between the variance of
/// various items. In particular, if `a < b`, then the variance of
/// `a` depends on the sources of `b`.
pub dependencies: TransitiveRelation<DefId>,

/// For each item with generics, maps to a vector of the variance
/// of its generics. If an item has no generics, it will have no
/// entry.
pub variances: FxHashMap<DefId, Rc<Vec<ty::Variance>>>,

/// An empty vector, useful for cloning.
pub empty_variance: Rc<Vec<ty::Variance>>,
}

#[derive(Clone, Copy, Debug, RustcDecodable, RustcEncodable)]
pub struct MethodCallee<'tcx> {
/// Impl method ID, for inherent methods, or trait method ID, otherwise.
Expand Down
10 changes: 2 additions & 8 deletions src/librustc/ty/relate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -124,14 +124,8 @@ fn relate_item_substs<'a, 'gcx, 'tcx, R>(relation: &mut R,
a_subst,
b_subst);

let variances;
let opt_variances = if relation.tcx().variance_computed.get() {
variances = relation.tcx().variances_of(item_def_id);
Some(&*variances)
} else {
None
};
relate_substs(relation, opt_variances, a_subst, b_subst)
let opt_variances = relation.tcx().variances_of(item_def_id);
relate_substs(relation, Some(&opt_variances), a_subst, b_subst)
}

pub fn relate_substs<'a, 'gcx, 'tcx, R>(relation: &mut R,
Expand Down
83 changes: 56 additions & 27 deletions src/librustc_data_structures/transitive_relation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,21 +9,23 @@
// except according to those terms.

use bitvec::BitMatrix;
use stable_hasher::{HashStable, StableHasher, StableHasherResult};
use fx::FxHashMap;
use rustc_serialize::{Encodable, Encoder, Decodable, Decoder};
use stable_hasher::{HashStable, StableHasher, StableHasherResult};
use std::cell::RefCell;
use std::fmt::Debug;
use std::hash::Hash;
use std::mem;



#[derive(Clone)]
pub struct TransitiveRelation<T: Debug + PartialEq> {
// List of elements. This is used to map from a T to a usize. We
// expect domain to be small so just use a linear list versus a
// hashmap or something.
pub struct TransitiveRelation<T: Clone + Debug + Eq + Hash + Clone> {
// List of elements. This is used to map from a T to a usize.
elements: Vec<T>,

// Maps each element to an index.
map: FxHashMap<T, Index>,

// List of base edges in the graph. Require to compute transitive
// closure.
edges: Vec<Edge>,
Expand All @@ -40,19 +42,20 @@ pub struct TransitiveRelation<T: Debug + PartialEq> {
closure: RefCell<Option<BitMatrix>>,
}

#[derive(Clone, PartialEq, PartialOrd, RustcEncodable, RustcDecodable)]
#[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash, RustcEncodable, RustcDecodable)]
struct Index(usize);

#[derive(Clone, PartialEq, RustcEncodable, RustcDecodable)]
#[derive(Clone, PartialEq, Eq, RustcEncodable, RustcDecodable)]
struct Edge {
source: Index,
target: Index,
}

impl<T: Debug + PartialEq> TransitiveRelation<T> {
impl<T: Clone + Debug + Eq + Hash + Clone> TransitiveRelation<T> {
pub fn new() -> TransitiveRelation<T> {
TransitiveRelation {
elements: vec![],
map: FxHashMap(),
edges: vec![],
closure: RefCell::new(None),
}
Expand All @@ -63,29 +66,35 @@ impl<T: Debug + PartialEq> TransitiveRelation<T> {
}

fn index(&self, a: &T) -> Option<Index> {
self.elements.iter().position(|e| *e == *a).map(Index)
self.map.get(a).cloned()
}

fn add_index(&mut self, a: T) -> Index {
match self.index(&a) {
Some(i) => i,
None => {
self.elements.push(a);

// if we changed the dimensions, clear the cache
*self.closure.borrow_mut() = None;

Index(self.elements.len() - 1)
}
}
let &mut TransitiveRelation {
ref mut elements,
ref closure,
ref mut map,
..
} = self;

map.entry(a.clone())
.or_insert_with(|| {
elements.push(a);

// if we changed the dimensions, clear the cache
*closure.borrow_mut() = None;

Index(elements.len() - 1)
})
.clone()
}

/// Applies the (partial) function to each edge and returns a new
/// relation. If `f` returns `None` for any end-point, returns
/// `None`.
pub fn maybe_map<F, U>(&self, mut f: F) -> Option<TransitiveRelation<U>>
where F: FnMut(&T) -> Option<U>,
U: Debug + PartialEq,
U: Clone + Debug + Eq + Hash + Clone,
{
let mut result = TransitiveRelation::new();
for edge in &self.edges {
Expand Down Expand Up @@ -125,6 +134,20 @@ impl<T: Debug + PartialEq> TransitiveRelation<T> {
}
}

/// Returns a vector of all things less than `a`.
///
/// Really this probably ought to be `impl Iterator<Item=&T>`, but
/// I'm too lazy to make that work, and -- given the caching
/// strategy -- it'd be a touch tricky anyhow.
pub fn less_than(&self, a: &T) -> Vec<&T> {
match self.index(a) {
Some(a) => self.with_closure(|closure| {
closure.iter(a.0).map(|i| &self.elements[i]).collect()
}),
None => vec![],
}
}

/// Picks what I am referring to as the "postdominating"
/// upper-bound for `a` and `b`. This is usually the least upper
/// bound, but in cases where there is no single least upper
Expand Down Expand Up @@ -335,7 +358,7 @@ fn pare_down(candidates: &mut Vec<usize>, closure: &BitMatrix) {
}

impl<T> Encodable for TransitiveRelation<T>
where T: Encodable + Debug + PartialEq
where T: Clone + Encodable + Debug + Eq + Hash + Clone
{
fn encode<E: Encoder>(&self, s: &mut E) -> Result<(), E::Error> {
s.emit_struct("TransitiveRelation", 2, |s| {
Expand All @@ -347,19 +370,23 @@ impl<T> Encodable for TransitiveRelation<T>
}

impl<T> Decodable for TransitiveRelation<T>
where T: Decodable + Debug + PartialEq
where T: Clone + Decodable + Debug + Eq + Hash + Clone
{
fn decode<D: Decoder>(d: &mut D) -> Result<Self, D::Error> {
d.read_struct("TransitiveRelation", 2, |d| {
let elements = d.read_struct_field("elements", 0, |d| Decodable::decode(d))?;
let elements: Vec<T> = d.read_struct_field("elements", 0, |d| Decodable::decode(d))?;
let edges = d.read_struct_field("edges", 1, |d| Decodable::decode(d))?;
Ok(TransitiveRelation { elements, edges, closure: RefCell::new(None) })
let map = elements.iter()
.enumerate()
.map(|(index, elem)| (elem.clone(), Index(index)))
.collect();
Ok(TransitiveRelation { elements, edges, map, closure: RefCell::new(None) })
})
}
}

impl<CTX, T> HashStable<CTX> for TransitiveRelation<T>
where T: HashStable<CTX> + PartialEq + Debug
where T: HashStable<CTX> + Eq + Debug + Clone + Hash
{
fn hash_stable<W: StableHasherResult>(&self,
hcx: &mut CTX,
Expand All @@ -369,6 +396,8 @@ impl<CTX, T> HashStable<CTX> for TransitiveRelation<T>
let TransitiveRelation {
ref elements,
ref edges,
// "map" is just a copy of elements vec
map: _,
// "closure" is just a copy of the data above
closure: _
} = *self;
Expand Down
4 changes: 1 addition & 3 deletions src/librustc_typeck/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -293,6 +293,7 @@ pub fn provide(providers: &mut Providers) {
collect::provide(providers);
coherence::provide(providers);
check::provide(providers);
variance::provide(providers);
}

pub fn check_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>)
Expand All @@ -307,9 +308,6 @@ pub fn check_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>)

})?;

time(time_passes, "variance inference", ||
variance::infer_variance(tcx));

tcx.sess.track_errors(|| {
time(time_passes, "impl wf inference", ||
impl_wf_check::impl_wf_check(tcx));
Expand Down
Loading