Skip to content

Remove dependency tracking for variance computation #45473

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

Merged
merged 1 commit into from
Oct 25, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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