From 94edd8fa4836e42820529ab751a4bbd56b6f4b14 Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Mon, 23 Oct 2017 17:47:07 +0200 Subject: [PATCH] Remove dependency tracking for variance computation This custom tracking is now replaced by the red/green algorithm. Fix https://github.com/rust-lang/rust/issues/45471 --- src/librustc/ich/impls_ty.rs | 2 -- src/librustc/ty/mod.rs | 6 ------ src/librustc_typeck/variance/README.md | 20 +++++++------------- src/librustc_typeck/variance/constraints.rs | 16 +--------------- src/librustc_typeck/variance/mod.rs | 13 +------------ src/librustc_typeck/variance/solve.rs | 4 ++-- 6 files changed, 11 insertions(+), 50 deletions(-) diff --git a/src/librustc/ich/impls_ty.rs b/src/librustc/ich/impls_ty.rs index 50f7e4ba176..8a8dfbabbe1 100644 --- a/src/librustc/ich/impls_ty.rs +++ b/src/librustc/ich/impls_ty.rs @@ -755,13 +755,11 @@ impl<'gcx> HashStable> for ty::CrateVariancesMap { hcx: &mut StableHashingContext<'gcx>, hasher: &mut StableHasher) { let ty::CrateVariancesMap { - ref dependencies, ref variances, // This is just an irrelevant helper value. empty_variance: _, } = *self; - dependencies.hash_stable(hcx, hasher); variances.hash_stable(hcx, hasher); } } diff --git a/src/librustc/ty/mod.rs b/src/librustc/ty/mod.rs index 129c81c5cd6..02e550711cd 100644 --- a/src/librustc/ty/mod.rs +++ b/src/librustc/ty/mod.rs @@ -55,7 +55,6 @@ 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; @@ -313,11 +312,6 @@ pub enum Variance { /// `tcx.variances_of()` to get the variance for a *particular* /// 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, - /// 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. diff --git a/src/librustc_typeck/variance/README.md b/src/librustc_typeck/variance/README.md index 59291617889..64d3389b34a 100644 --- a/src/librustc_typeck/variance/README.md +++ b/src/librustc_typeck/variance/README.md @@ -104,22 +104,16 @@ into two queries: - `crate_variances` computes the variance for all items in the current crate. - `variances_of` accesses the variance for an individual reading; it works by requesting `crate_variances` and extracting the relevant data. - + If you limit yourself to reading `variances_of`, your code will only depend then on the inference inferred for that particular item. -Eventually, the goal is to rely on the red-green dependency management -algorithm. At the moment, however, we rely instead on a hack, where -`variances_of` ignores the dependencies of accessing -`crate_variances` and instead computes the *correct* dependencies -itself. To this end, when we build up the constraints in the system, -we also built up a transitive `dependencies` relation as part of the -crate map. A `(X, Y)` pair is added to the map each time we have a -constraint that the variance of some inferred for the item `X` depends -on the variance of some element of `Y`. This is to some extent a -mirroring of the inference graph in the dependency graph. This means -we can just completely ignore the fixed-point iteration, since it is -just shuffling values along this graph. +Ultimately, this setup relies on the red-green algorithm. +In particular, every variance query ultimately depends on -- effectively -- +all type definitions in the entire crate (through `crate_variances`), +but since most changes will not result in a change +to the actual results from variance inference, +the `variances_of` query will wind up being considered green after it is re-evaluated. ### Addendum: Variance on traits diff --git a/src/librustc_typeck/variance/constraints.rs b/src/librustc_typeck/variance/constraints.rs index c1653cfb43b..857b35158f2 100644 --- a/src/librustc_typeck/variance/constraints.rs +++ b/src/librustc_typeck/variance/constraints.rs @@ -22,7 +22,6 @@ use syntax::ast; use rustc::hir; use rustc::hir::itemlikevisit::ItemLikeVisitor; -use rustc_data_structures::transitive_relation::TransitiveRelation; use rustc_data_structures::stable_hasher::StableHashingContextProvider; use super::terms::*; @@ -38,11 +37,6 @@ pub struct ConstraintContext<'a, 'tcx: 'a> { bivariant: VarianceTermPtr<'a>, pub constraints: Vec>, - - /// 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, } /// Declares that the variable `decl_id` appears in a location with @@ -63,7 +57,6 @@ pub struct Constraint<'a> { /// then while we are visiting `Bar`, the `CurrentItem` would have /// the def-id and the start of `Foo`'s inferreds. pub struct CurrentItem { - def_id: DefId, inferred_start: InferredIndex, } @@ -81,7 +74,6 @@ pub fn add_constraints_from_crate<'a, 'tcx>(terms_cx: TermsContext<'a, 'tcx>) invariant, bivariant, constraints: Vec::new(), - dependencies: TransitiveRelation::new(), }; tcx.hir.krate().visit_all_item_likes(&mut constraint_cx); @@ -201,7 +193,7 @@ impl<'a, 'tcx> ConstraintContext<'a, 'tcx> { let id = tcx.hir.as_local_node_id(def_id).unwrap(); let inferred_start = self.terms_cx.inferred_starts[&id]; - let current_item = &CurrentItem { def_id, inferred_start }; + let current_item = &CurrentItem { inferred_start }; match tcx.type_of(def_id).sty { ty::TyAdt(def, _) => { // Not entirely obvious: constraints on structs/enums do not @@ -410,12 +402,6 @@ impl<'a, 'tcx> ConstraintContext<'a, 'tcx> { return; } - // Add a corresponding relation into the dependencies to - // indicate that the variance for `current` relies on `def_id`. - if self.tcx().dep_graph.is_fully_enabled() { - self.dependencies.add(current.def_id, def_id); - } - let (local, remote) = if let Some(id) = self.tcx().hir.as_local_node_id(def_id) { (Some(self.terms_cx.inferred_starts[&id]), None) } else { diff --git a/src/librustc_typeck/variance/mod.rs b/src/librustc_typeck/variance/mod.rs index 7a9f35545e2..418d2b94670 100644 --- a/src/librustc_typeck/variance/mod.rs +++ b/src/librustc_typeck/variance/mod.rs @@ -94,20 +94,9 @@ fn variances_of<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, item_def_id: DefId) // Everything else must be inferred. - // Lacking red/green, we read the variances for all items here - // but ignore the dependencies, then re-synthesize the ones we need. - let crate_map = tcx.dep_graph.with_ignore(|| tcx.crate_variances(LOCAL_CRATE)); + let crate_map = tcx.crate_variances(LOCAL_CRATE); let dep_node = item_def_id.to_dep_node(tcx, DepKind::ItemVarianceConstraints); tcx.dep_graph.read(dep_node); - for &dep_def_id in crate_map.dependencies.less_than(&item_def_id) { - if dep_def_id.is_local() { - let dep_node = dep_def_id.to_dep_node(tcx, DepKind::ItemVarianceConstraints); - tcx.dep_graph.read(dep_node); - } else { - let dep_node = dep_def_id.to_dep_node(tcx, DepKind::ItemVariances); - tcx.dep_graph.read(dep_node); - } - } crate_map.variances.get(&item_def_id) .unwrap_or(&crate_map.empty_variance) diff --git a/src/librustc_typeck/variance/solve.rs b/src/librustc_typeck/variance/solve.rs index 495eb95419a..434e8ce148f 100644 --- a/src/librustc_typeck/variance/solve.rs +++ b/src/librustc_typeck/variance/solve.rs @@ -34,7 +34,7 @@ struct SolveContext<'a, 'tcx: 'a> { } pub fn solve_constraints(constraints_cx: ConstraintContext) -> ty::CrateVariancesMap { - let ConstraintContext { terms_cx, dependencies, constraints, .. } = constraints_cx; + let ConstraintContext { terms_cx, constraints, .. } = constraints_cx; let mut solutions = vec![ty::Bivariant; terms_cx.inferred_terms.len()]; for &(id, ref variances) in &terms_cx.lang_items { @@ -53,7 +53,7 @@ pub fn solve_constraints(constraints_cx: ConstraintContext) -> ty::CrateVariance let variances = solutions_cx.create_map(); let empty_variance = Rc::new(Vec::new()); - ty::CrateVariancesMap { dependencies, variances, empty_variance } + ty::CrateVariancesMap { variances, empty_variance } } impl<'a, 'tcx> SolveContext<'a, 'tcx> {