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.

Sign up for GitHub

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

Jump to bottom

Make use of the implemented red/green algorithm for variance #47696

Merged
merged 1 commit into from
Jan 26, 2018
Merged

Make use of the implemented red/green algorithm for variance #47696

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
45 changes: 1 addition & 44 deletions src/librustc_typeck/variance/constraints.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,16 +14,12 @@
//! We walk the set of items and, for each member, generate new constraints.

use hir::def_id::DefId;
use rustc::dep_graph::{DepGraphSafe, DepKind, DepNodeColor};
use rustc::ich::StableHashingContext;
use rustc::ty::subst::Substs;
use rustc::ty::{self, Ty, TyCtxt};
use syntax::ast;
use rustc::hir;
use rustc::hir::itemlikevisit::ItemLikeVisitor;

use rustc_data_structures::stable_hasher::StableHashingContextProvider;

use super::terms::*;
use super::terms::VarianceTerm::*;

Expand Down Expand Up @@ -132,50 +128,11 @@ impl<'a, 'tcx, 'v> ItemLikeVisitor<'v> for ConstraintContext<'a, 'tcx> {
}
}

impl<'a, 'tcx> StableHashingContextProvider for ConstraintContext<'a, 'tcx> {
type ContextType = StableHashingContext<'tcx>;

fn create_stable_hashing_context(&self) -> Self::ContextType {
self.terms_cx.tcx.create_stable_hashing_context()
}
}

impl<'a, 'tcx> DepGraphSafe for ConstraintContext<'a, 'tcx> {}

impl<'a, 'tcx> ConstraintContext<'a, 'tcx> {
fn visit_node_helper(&mut self, id: ast::NodeId) {
let tcx = self.terms_cx.tcx;
let def_id = tcx.hir.local_def_id(id);

// Encapsulate constructing the constraints into a task we can
// reference later. This can go away once the red-green
// algorithm is in place.
//
// See README.md for a detailed discussion
// on dep-graph management.
let dep_node = def_id.to_dep_node(tcx, DepKind::ItemVarianceConstraints);

if let Some(DepNodeColor::Green(_)) = tcx.dep_graph.node_color(&dep_node) {
// If the corresponding node has already been marked as green, the
// appropriate portion of the DepGraph has already been loaded from
// the previous graph, so we don't do any dep-tracking. Since we
// don't cache any values though, we still have to re-run the
// computation.
tcx.dep_graph.with_ignore(|| {
self.build_constraints_for_item(def_id);
});
} else {
tcx.dep_graph.with_task(dep_node,
self,
def_id,
visit_item_task);
}

fn visit_item_task<'a, 'tcx>(ccx: &mut ConstraintContext<'a, 'tcx>,
def_id: DefId)
{
ccx.build_constraints_for_item(def_id);
}
self.build_constraints_for_item(def_id);
}

fn tcx(&self) -> TyCtxt<'a, 'tcx, 'tcx> {
Expand Down
4 changes: 0 additions & 4 deletions src/librustc_typeck/variance/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
//! parameters. See README.md for details.

use arena;
use rustc::dep_graph::DepKind;
use rustc::hir;
use rustc::hir::def_id::{CrateNum, DefId, LOCAL_CRATE};
use rustc::ty::{self, CrateVariancesMap, TyCtxt};
Expand Down Expand Up @@ -95,9 +94,6 @@ fn variances_of<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, item_def_id: DefId)
// Everything else must be inferred.

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);

crate_map.variances.get(&item_def_id)
.unwrap_or(&crate_map.empty_variance)
.clone()
Expand Down
2 changes: 1 addition & 1 deletion src/test/compile-fail/dep-graph-variance-alias.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ struct Foo<T> {
f: T
}

#[rustc_if_this_changed]
#[rustc_if_this_changed(Krate)]
Copy link
Contributor

Choose a reason for hiding this comment

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

Huh, this looks suspicious.

Copy link
Contributor

Choose a reason for hiding this comment

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

This change doesn't seem right to me. I think we do expect to have to recompute item-variances for Use if the type alias changes. I would sort of expect that it would be an input into the AdtDef or what have you for Use... but I guess it might not be working out that way? Did you dig into what's going on here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The CrateVariances dep node is eval_always, so it only depends on Krate and not on any Hir nodes. This in turn means that ItemVariances won't transitively depend on any Hir nodes, just Krate. So there won't be a path from Hir(TypeAlias) to ItemVariances. This is still fine though since CrateVariances always gets recomputed. It is also correct without that because Krate has an implicit dependency on everything in a crate.

Copy link
Contributor

Choose a reason for hiding this comment

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

OK, makes sense. Thanks!

type TypeAlias<T> = Foo<T>;

#[rustc_then_this_would_need(ItemVariances)] //~ ERROR OK
Expand Down