Skip to content

Commit

Permalink
Auto merge of rust-lang#45473 - SimonSapin:variance-red-green, r=niko…
Browse files Browse the repository at this point in the history
…matsakis

Remove dependency tracking for variance computation

This custom tracking is now replaced by the red/green algorithm.

Fix rust-lang#45471
  • Loading branch information
bors committed Oct 25, 2017
2 parents 6e61bba + 94edd8f commit b247805
Show file tree
Hide file tree
Showing 6 changed files with 11 additions and 50 deletions.
2 changes: 0 additions & 2 deletions src/librustc/ich/impls_ty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -755,13 +755,11 @@ impl<'gcx> HashStable<StableHashingContext<'gcx>> for ty::CrateVariancesMap {
hcx: &mut StableHashingContext<'gcx>,
hasher: &mut StableHasher<W>) {
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);
}
}
Expand Down
6 changes: 0 additions & 6 deletions src/librustc/ty/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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<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.
Expand Down
20 changes: 7 additions & 13 deletions src/librustc_typeck/variance/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
16 changes: 1 addition & 15 deletions src/librustc_typeck/variance/constraints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::*;
Expand All @@ -38,11 +37,6 @@ pub struct ConstraintContext<'a, 'tcx: 'a> {
bivariant: VarianceTermPtr<'a>,

pub constraints: Vec<Constraint<'a>>,

/// 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>,
}

/// Declares that the variable `decl_id` appears in a location with
Expand All @@ -63,7 +57,6 @@ pub struct Constraint<'a> {
/// then while we are visiting `Bar<T>`, the `CurrentItem` would have
/// the def-id and the start of `Foo`'s inferreds.
pub struct CurrentItem {
def_id: DefId,
inferred_start: InferredIndex,
}

Expand All @@ -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);
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down
13 changes: 1 addition & 12 deletions src/librustc_typeck/variance/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions src/librustc_typeck/variance/solve.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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> {
Expand Down

0 comments on commit b247805

Please sign in to comment.