Refactor variance and remove last [pub] map

src/librustc/dep_graph/dep_node.rs

@@ -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
@@ -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),
@@ -178,6 +181,7 @@ impl<D: Clone + Debug> DepNode<D> {
             TransCrateItem,
             AssociatedItems,
             ItemSignature,
+            ItemVariances,
             IsForeignItem,
             AssociatedItemDefIds,
             InherentImpls,
@@ -199,6 +203,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)),
@@ -230,6 +235,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)))))

src/librustc/dep_graph/mod.rs

@@ -18,7 +18,6 @@ mod raii;
 mod safe;
 mod shadow;
 mod thread;
-mod visit;
 
 pub use self::dep_tracking_map::{DepTrackingMap, DepTrackingMapConfig};
 pub use self::dep_node::DepNode;
@@ -28,5 +27,4 @@ pub use self::graph::WorkProduct;
 pub use self::query::DepGraphQuery;
 pub use self::safe::AssertDepGraphSafe;
 pub use self::safe::DepGraphSafe;
-pub use self::visit::visit_all_item_likes_in_krate;
 pub use self::raii::DepTask;

src/librustc/hir/intravisit.rs

@@ -88,7 +88,7 @@ pub enum NestedVisitorMap<'this, 'tcx: 'this> {
     /// that are inside of an item-like.
     ///
     /// **This is the most common choice.** A very commmon pattern is
-    /// to use `tcx.visit_all_item_likes_in_krate()` as an outer loop,
+    /// to use `visit_all_item_likes()` as an outer loop,
     /// and to have the visitor that visits the contents of each item
     /// using this setting.
     OnlyBodies(&'this Map<'tcx>),

src/librustc/hir/itemlikevisit.rs

@@ -19,9 +19,8 @@ use super::intravisit::Visitor;
 ///
 /// 1. **Shallow visit**: Get a simple callback for every item (or item-like thing) in the HIR.
 ///    - Example: find all items with a `#[foo]` attribute on them.
-///    - How: Implement `ItemLikeVisitor` and call `tcx.visit_all_item_likes_in_krate()`.
+///    - How: Implement `ItemLikeVisitor` and call `tcx.hir.krate().visit_all_item_likes()`.
 ///    - Pro: Efficient; just walks the lists of item-like things, not the nodes themselves.
-///    - Pro: Integrates well into dependency tracking.
 ///    - Con: Don't get information about nesting
 ///    - Con: Don't have methods for specific bits of HIR, like "on
 ///      every expr, do this".
@@ -30,7 +29,7 @@ use super::intravisit::Visitor;
 ///    within one another.
 ///    - Example: Examine each expression to look for its type and do some check or other.
 ///    - How: Implement `intravisit::Visitor` and use
-///      `tcx.visit_all_item_likes_in_krate(visitor.as_deep_visitor())`. Within
+///      `tcx.hir.krate().visit_all_item_likes(visitor.as_deep_visitor())`. Within
 ///      your `intravisit::Visitor` impl, implement methods like
 ///      `visit_expr()`; don't forget to invoke
 ///      `intravisit::walk_visit_expr()` to keep walking the subparts.

src/librustc/ty/context.rs

@@ -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>,
@@ -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),

src/librustc/ty/maps.rs

@@ -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 `{}`",
@@ -535,18 +541,6 @@ macro_rules! define_map_struct {
         }
     };
 
-    // Detect things with the `pub` modifier
-    (tcx: $tcx:tt,
-     input: (([pub $($other_modifiers:tt)*] $attrs:tt $name:tt) $($input:tt)*),
-     output: $output:tt) => {
-        define_map_struct! {
-            tcx: $tcx,
-            ready: ([pub] $attrs $name),
-            input: ($($input)*),
-            output: $output
-        }
-    };
-
     // No modifiers left? This is a private item.
     (tcx: $tcx:tt,
      input: (([] $attrs:tt $name:tt) $($input:tt)*),
@@ -673,9 +667,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>>,
+    [] 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>>,
@@ -810,3 +808,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
+}

src/librustc/ty/mod.rs

@@ -15,7 +15,7 @@ pub use self::IntVarValue::*;
 pub use self::LvaluePreference::*;
 pub use self::fold::TypeFoldable;
 
-use dep_graph::{self, DepNode};
+use dep_graph::DepNode;
 use hir::{map as hir_map, FreevarMap, TraitMap};
 use hir::def::{Def, CtorKind, ExportMap};
 use hir::def_id::{CrateNum, DefId, DefIndex, CRATE_DEF_INDEX, LOCAL_CRATE};
@@ -55,9 +55,9 @@ 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;
 
 pub use self::sty::{Binder, DebruijnIndex};
 pub use self::sty::{FnSig, PolyFnSig};
@@ -309,6 +309,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.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.
+    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.
@@ -2543,14 +2564,6 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
         self.mk_region(ty::ReScope(self.node_extent(id)))
     }
 
-    pub fn visit_all_item_likes_in_krate<V,F>(self,
-                                              dep_node_fn: F,
-                                              visitor: &mut V)
-        where F: FnMut(DefId) -> DepNode<DefId>, V: ItemLikeVisitor<'gcx>
-    {
-        dep_graph::visit_all_item_likes_in_krate(self.global_tcx(), dep_node_fn, visitor);
-    }
-
     /// Looks up the span of `impl_did` if the impl is local; otherwise returns `Err`
     /// with the name of the crate containing the impl.
     pub fn span_of_impl(self, impl_did: DefId) -> Result<Span, Symbol> {

src/librustc/ty/relate.rs

@@ -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,

src/librustc_data_structures/transitive_relation.rs

@@ -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>,
@@ -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),
         }
@@ -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 {
@@ -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
@@ -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| {
@@ -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,
@@ -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;

src/librustc_incremental/assert_dep_graph.rs

@@ -51,7 +51,7 @@ use rustc::ty::TyCtxt;
 use rustc_data_structures::fx::FxHashSet;
 use rustc_data_structures::graph::{Direction, INCOMING, OUTGOING, NodeIndex};
 use rustc::hir;
-use rustc::hir::itemlikevisit::ItemLikeVisitor;
+use rustc::hir::intravisit::{self, NestedVisitorMap, Visitor};
 use rustc::ich::{ATTR_IF_THIS_CHANGED, ATTR_THEN_THIS_WOULD_NEED};
 use graphviz::IntoCow;
 use std::env;
@@ -80,7 +80,7 @@ pub fn assert_dep_graph<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>) {
                                           if_this_changed: vec![],
                                           then_this_would_need: vec![] };
         visitor.process_attrs(ast::CRATE_NODE_ID, &tcx.hir.krate().attrs);
-        tcx.hir.krate().visit_all_item_likes(&mut visitor);
+        tcx.hir.krate().visit_all_item_likes(&mut visitor.as_deep_visitor());
         (visitor.if_this_changed, visitor.then_this_would_need)
     };
 
@@ -166,17 +166,29 @@ impl<'a, 'tcx> IfThisChanged<'a, 'tcx> {
     }
 }
 
-impl<'a, 'tcx> ItemLikeVisitor<'tcx> for IfThisChanged<'a, 'tcx> {
+impl<'a, 'tcx> Visitor<'tcx> for IfThisChanged<'a, 'tcx> {
+    fn nested_visit_map<'this>(&'this mut self) -> NestedVisitorMap<'this, 'tcx> {
+        NestedVisitorMap::OnlyBodies(&self.tcx.hir)
+    }
+
     fn visit_item(&mut self, item: &'tcx hir::Item) {
         self.process_attrs(item.id, &item.attrs);
+        intravisit::walk_item(self, item);
     }
 
     fn visit_trait_item(&mut self, trait_item: &'tcx hir::TraitItem) {
         self.process_attrs(trait_item.id, &trait_item.attrs);
+        intravisit::walk_trait_item(self, trait_item);
     }
 
     fn visit_impl_item(&mut self, impl_item: &'tcx hir::ImplItem) {
         self.process_attrs(impl_item.id, &impl_item.attrs);
+        intravisit::walk_impl_item(self, impl_item);
+    }
+
+    fn visit_struct_field(&mut self, s: &'tcx hir::StructField) {
+        self.process_attrs(s.id, &s.attrs);
+        intravisit::walk_struct_field(self, s);
     }
 }
 

src/librustc_metadata/encoder.rs

@@ -240,8 +240,8 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
 }
 
 impl<'a, 'b: 'a, 'tcx: 'b> EntryBuilder<'a, 'b, 'tcx> {
-    fn encode_item_variances(&mut self, def_id: DefId) -> LazySeq<ty::Variance> {
-        debug!("EntryBuilder::encode_item_variances({:?})", def_id);
+    fn encode_variances_of(&mut self, def_id: DefId) -> LazySeq<ty::Variance> {
+        debug!("EntryBuilder::encode_variances_of({:?})", def_id);
         let tcx = self.tcx;
         self.lazy_seq_from_slice(&tcx.variances_of(def_id))
     }
@@ -824,7 +824,7 @@ impl<'a, 'b: 'a, 'tcx: 'b> EntryBuilder<'a, 'b, 'tcx> {
                 hir::ItemEnum(..) |
                 hir::ItemStruct(..) |
                 hir::ItemUnion(..) |
-                hir::ItemTrait(..) => self.encode_item_variances(def_id),
+                hir::ItemTrait(..) => self.encode_variances_of(def_id),
                 _ => LazySeq::empty(),
             },
             generics: match item.node {

src/librustc_typeck/lib.rs

@@ -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>)
@@ -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));
@@ -320,6 +318,11 @@ pub fn check_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>)
           coherence::check_coherence(tcx));
     })?;
 
+    tcx.sess.track_errors(|| {
+        time(time_passes, "variance testing", ||
+             variance::test::test_variance(tcx));
+    })?;
+
     time(time_passes, "wf checking", || check::check_wf_new(tcx))?;
 
     time(time_passes, "item-types checking", || check::check_item_types(tcx))?;

src/librustc_typeck/variance/README.md

@@ -97,51 +97,29 @@ types involved before considering variance.
 
 #### Dependency graph management
 
-Because variance works in two phases, if we are not careful, we wind
-up with a muddled mess of a dep-graph. Basically, when gathering up
-the constraints, things are fairly well-structured, but then we do a
-fixed-point iteration and write the results back where they
-belong. You can't give this fixed-point iteration a single task
-because it reads from (and writes to) the variance of all types in the
-crate. In principle, we *could* switch the "current task" in a very
-fine-grained way while propagating constraints in the fixed-point
-iteration and everything would be automatically tracked, but that
-would add some overhead and isn't really necessary anyway.
-
-Instead what we do is to add edges into the dependency graph as we
-construct the constraint set: so, if computing the constraints for
-node `X` requires loading the inference variables from node `Y`, then
-we can add an edge `Y -> X`, since the variance we ultimately infer
-for `Y` will affect the variance we ultimately infer for `X`.
-
-At this point, we've basically mirrored 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. In other words, if we added the fine-grained switching of tasks
-I described earlier, all it would show is that we repeatedly read the
-values described by the constraints, but those edges were already
-added when building the constraints in the first place.
-
-Here is how this is implemented (at least as of the time of this
-writing). The associated `DepNode` for the variance map is (at least
-presently) `Signature(DefId)`. This means that, in `constraints.rs`,
-when we visit an item to load up its constraints, we set
-`Signature(DefId)` as the current task (the "memoization" pattern
-described in the `dep-graph` README). Then whenever we find an
-embedded type or trait, we add a synthetic read of `Signature(DefId)`,
-which covers the variances we will compute for all of its
-parameters. This read is synthetic (i.e., we call
-`variance_map.read()`) because, in fact, the final variance is not yet
-computed -- the read *will* occur (repeatedly) during the fixed-point
-iteration phase.
-
-In fact, we don't really *need* this synthetic read. That's because we
-do wind up looking up the `TypeScheme` or `TraitDef` for all
-references types/traits, and those reads add an edge from
-`Signature(DefId)` (that is, they share the same dep node as
-variance). However, I've kept the synthetic reads in place anyway,
-just for future-proofing (in case we change the dep-nodes in the
-future), and because it makes the intention a bit clearer I think.
+Because variance is a whole-crate inference, its dependency graph
+can become quite muddled if we are not careful. To resolve this, we refactor
+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.
 
 ### Addendum: Variance on traits
 

src/librustc_typeck/variance/mod.rs

@@ -12,7 +12,12 @@
 //! parameters. See README.md for details.
 
 use arena;
-use rustc::ty::TyCtxt;
+use rustc::dep_graph::DepNode;
+use rustc::hir;
+use rustc::hir::def_id::{CrateNum, DefId, LOCAL_CRATE};
+use rustc::ty::{self, CrateVariancesMap, TyCtxt};
+use rustc::ty::maps::Providers;
+use std::rc::Rc;
 
 /// Defines the `TermsContext` basically houses an arena where we can
 /// allocate terms.
@@ -24,13 +29,67 @@ mod constraints;
 /// Code to solve constraints and write out the results.
 mod solve;
 
+/// Code to write unit tests of variance.
+pub mod test;
+
 /// Code for transforming variances.
 mod xform;
 
-pub fn infer_variance<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>) {
+pub fn provide(providers: &mut Providers) {
+    *providers = Providers {
+        variances_of,
+        crate_variances,
+        ..*providers
+    };
+}
+
+fn crate_variances<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, crate_num: CrateNum)
+                             -> Rc<CrateVariancesMap> {
+    assert_eq!(crate_num, LOCAL_CRATE);
     let mut arena = arena::TypedArena::new();
     let terms_cx = terms::determine_parameters_to_be_inferred(tcx, &mut arena);
     let constraints_cx = constraints::add_constraints_from_crate(terms_cx);
-    solve::solve_constraints(constraints_cx);
-    tcx.variance_computed.set(true);
+    Rc::new(solve::solve_constraints(constraints_cx))
+}
+
+fn variances_of<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, item_def_id: DefId)
+                            -> Rc<Vec<ty::Variance>> {
+    let item_id = tcx.hir.as_local_node_id(item_def_id).expect("expected local def-id");
+    let item = tcx.hir.expect_item(item_id);
+    match item.node {
+        hir::ItemTrait(..) => {
+            // Traits are always invariant.
+            let generics = tcx.generics_of(item_def_id);
+            assert!(generics.parent.is_none());
+            Rc::new(vec![ty::Variance::Invariant; generics.count()])
+        }
+
+        hir::ItemEnum(..) |
+        hir::ItemStruct(..) |
+        hir::ItemUnion(..) => {
+            // 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));
+            tcx.dep_graph.read(DepNode::ItemVarianceConstraints(item_def_id));
+            for &dep_def_id in crate_map.dependencies.less_than(&item_def_id) {
+                if dep_def_id.is_local() {
+                    tcx.dep_graph.read(DepNode::ItemVarianceConstraints(dep_def_id));
+                } else {
+                    tcx.dep_graph.read(DepNode::ItemVariances(dep_def_id));
+                }
+            }
+
+            crate_map.variances.get(&item_def_id)
+                               .unwrap_or(&crate_map.empty_variance)
+                               .clone()
+        }
+
+        _ => {
+            // Variance not relevant.
+            span_bug!(item.span, "asked to compute variance for wrong kind of item")
+        }
+    }
 }
+

src/librustc_typeck/variance/solve.rs

@@ -15,7 +15,9 @@
 //! optimal solution to the constraints. The final variance for each
 //! inferred is then written into the `variance_map` in the tcx.
 
+use rustc::hir::def_id::DefId;
 use rustc::ty;
+use rustc_data_structures::fx::FxHashMap;
 use std::rc::Rc;
 
 use super::constraints::*;
@@ -31,8 +33,8 @@ struct SolveContext<'a, 'tcx: 'a> {
     solutions: Vec<ty::Variance>,
 }
 
-pub fn solve_constraints(constraints_cx: ConstraintContext) {
-    let ConstraintContext { terms_cx, constraints, .. } = constraints_cx;
+pub fn solve_constraints(constraints_cx: ConstraintContext) -> ty::CrateVariancesMap {
+    let ConstraintContext { terms_cx, dependencies, constraints, .. } = constraints_cx;
 
     let solutions = terms_cx.inferred_infos
         .iter()
@@ -45,7 +47,10 @@ pub fn solve_constraints(constraints_cx: ConstraintContext) {
         solutions: solutions,
     };
     solutions_cx.solve();
-    solutions_cx.write();
+    let variances = solutions_cx.create_map();
+    let empty_variance = Rc::new(Vec::new());
+
+    ty::CrateVariancesMap { dependencies, variances, empty_variance }
 }
 
 impl<'a, 'tcx> SolveContext<'a, 'tcx> {
@@ -83,7 +88,7 @@ impl<'a, 'tcx> SolveContext<'a, 'tcx> {
         }
     }
 
-    fn write(&self) {
+    fn create_map(&self) -> FxHashMap<DefId, Rc<Vec<ty::Variance>>> {
         // Collect all the variances for a particular item and stick
         // them into the variance map. We rely on the fact that we
         // generate all the inferreds for a particular item
@@ -95,11 +100,7 @@ impl<'a, 'tcx> SolveContext<'a, 'tcx> {
 
         let tcx = self.terms_cx.tcx;
 
-        // Ignore the writes here because the relevant edges were
-        // already accounted for in `constraints.rs`. See the section
-        // on dependency graph management in README.md for more
-        // information.
-        let _ignore = tcx.dep_graph.in_ignore();
+        let mut map = FxHashMap();
 
         let solutions = &self.solutions;
         let inferred_infos = &self.terms_cx.inferred_infos;
@@ -127,19 +128,10 @@ impl<'a, 'tcx> SolveContext<'a, 'tcx> {
 
             let item_def_id = tcx.hir.local_def_id(item_id);
 
-            // For unit testing: check for a special "rustc_variance"
-            // attribute and report an error with various results if found.
-            if tcx.has_attr(item_def_id, "rustc_variance") {
-                span_err!(tcx.sess,
-                          tcx.hir.span(item_id),
-                          E0208,
-                          "{:?}",
-                          item_variances);
-            }
-
-            tcx.maps.variances_of.borrow_mut()
-               .insert(item_def_id, Rc::new(item_variances));
+            map.insert(item_def_id, Rc::new(item_variances));
         }
+
+        map
     }
 
     fn evaluate(&self, term: VarianceTermPtr<'a>) -> ty::Variance {

src/librustc_typeck/variance/terms.rs

@@ -32,8 +32,6 @@ use self::VarianceTerm::*;
 
 pub type VarianceTermPtr<'a> = &'a VarianceTerm<'a>;
 
-use dep_graph::DepNode::ItemSignature as VarianceDepNode;
-
 #[derive(Copy, Clone, Debug)]
 pub struct InferredIndex(pub usize);
 
@@ -109,7 +107,7 @@ pub fn determine_parameters_to_be_inferred<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>
     };
 
     // See README.md for a discussion on dep-graph management.
-    tcx.visit_all_item_likes_in_krate(|def_id| VarianceDepNode(def_id), &mut terms_cx);
+    tcx.hir.krate().visit_all_item_likes(&mut terms_cx);
 
     terms_cx
 }
@@ -139,7 +137,6 @@ fn lang_items(tcx: TyCtxt) -> Vec<(ast::NodeId, Vec<ty::Variance>)> {
 impl<'a, 'tcx> TermsContext<'a, 'tcx> {
     fn add_inferreds_for_item(&mut self,
                               item_id: ast::NodeId,
-                              has_self: bool,
                               generics: &hir::Generics) {
         //! Add "inferreds" for the generic parameters declared on this
         //! item. This has a lot of annoying parameters because we are
@@ -149,38 +146,17 @@ impl<'a, 'tcx> TermsContext<'a, 'tcx> {
         //!
 
         // NB: In the code below for writing the results back into the
-        // tcx, we rely on the fact that all inferreds for a particular
-        // item are assigned continuous indices.
-
-        let inferreds_on_entry = self.num_inferred();
-
-        if has_self {
-            self.add_inferred(item_id, 0, item_id);
-        }
+        // `CrateVariancesMap`, we rely on the fact that all inferreds
+        // for a particular item are assigned continuous indices.
 
-        for (i, p) in generics.lifetimes.iter().enumerate() {
+        for (p, i) in generics.lifetimes.iter().zip(0..) {
             let id = p.lifetime.id;
-            let i = has_self as usize + i;
             self.add_inferred(item_id, i, id);
         }
 
-        for (i, p) in generics.ty_params.iter().enumerate() {
-            let i = has_self as usize + generics.lifetimes.len() + i;
+        for (p, i) in generics.ty_params.iter().zip(generics.lifetimes.len()..) {
             self.add_inferred(item_id, i, p.id);
         }
-
-        // If this item has no type or lifetime parameters,
-        // then there are no variances to infer, so just
-        // insert an empty entry into the variance map.
-        // Arguably we could just leave the map empty in this
-        // case but it seems cleaner to be able to distinguish
-        // "invalid item id" from "item id with no
-        // parameters".
-        if self.num_inferred() == inferreds_on_entry {
-            let item_def_id = self.tcx.hir.local_def_id(item_id);
-            self.tcx.maps.variances_of.borrow_mut()
-                .insert(item_def_id, self.empty_variances.clone());
-        }
     }
 
     fn add_inferred(&mut self, item_id: ast::NodeId, index: usize, param_id: ast::NodeId) {
@@ -232,15 +208,10 @@ impl<'a, 'tcx, 'v> ItemLikeVisitor<'v> for TermsContext<'a, 'tcx> {
             hir::ItemEnum(_, ref generics) |
             hir::ItemStruct(_, ref generics) |
             hir::ItemUnion(_, ref generics) => {
-                self.add_inferreds_for_item(item.id, false, generics);
-            }
-            hir::ItemTrait(_, ref generics, ..) => {
-                // Note: all inputs for traits are ultimately
-                // constrained to be invariant. See `visit_item` in
-                // the impl for `ConstraintContext` in `constraints.rs`.
-                self.add_inferreds_for_item(item.id, true, generics);
+                self.add_inferreds_for_item(item.id, generics);
             }
 
+            hir::ItemTrait(..) |
             hir::ItemExternCrate(_) |
             hir::ItemUse(..) |
             hir::ItemDefaultImpl(..) |

src/librustc_typeck/variance/test.rs

@@ -0,0 +1,41 @@
+// Copyright 2013 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+use rustc::hir;
+use rustc::hir::itemlikevisit::ItemLikeVisitor;
+use rustc::ty::TyCtxt;
+
+pub fn test_variance<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>) {
+    tcx.hir.krate().visit_all_item_likes(&mut VarianceTest { tcx });
+}
+
+struct VarianceTest<'a, 'tcx: 'a> {
+    tcx: TyCtxt<'a, 'tcx, 'tcx>
+}
+
+impl<'a, 'tcx> ItemLikeVisitor<'tcx> for VarianceTest<'a, 'tcx> {
+    fn visit_item(&mut self, item: &'tcx hir::Item) {
+        let item_def_id = self.tcx.hir.local_def_id(item.id);
+
+        // For unit testing: check for a special "rustc_variance"
+        // attribute and report an error with various results if found.
+        if self.tcx.has_attr(item_def_id, "rustc_variance") {
+            let variances_of = self.tcx.variances_of(item_def_id);
+            span_err!(self.tcx.sess,
+                      item.span,
+                      E0208,
+                      "{:?}",
+                      variances_of);
+        }
+    }
+
+    fn visit_trait_item(&mut self, _: &'tcx hir::TraitItem) { }
+    fn visit_impl_item(&mut self, _: &'tcx hir::ImplItem) { }
+}

src/test/compile-fail/dep-graph-struct-signature.rs

@@ -58,13 +58,15 @@ mod signatures {
         fn method(&self, x: u32) { }
     }
 
-    #[rustc_then_this_would_need(ItemSignature)] //~ ERROR OK
     struct WillChanges {
+        #[rustc_then_this_would_need(ItemSignature)] //~ ERROR OK
         x: WillChange,
+        #[rustc_then_this_would_need(ItemSignature)] //~ ERROR OK
         y: WillChange
     }
 
-    #[rustc_then_this_would_need(ItemSignature)] //~ ERROR OK
+    // The fields change, not the type itself.
+    #[rustc_then_this_would_need(ItemSignature)] //~ ERROR no path
     fn indirect(x: WillChanges) { }
 }
 

src/test/compile-fail/dep-graph-type-alias.rs

@@ -23,15 +23,21 @@ fn main() { }
 #[rustc_if_this_changed]
 type TypeAlias = u32;
 
-#[rustc_then_this_would_need(ItemSignature)] //~ ERROR OK
+// The type alias directly affects the type of the field,
+// not the enclosing struct:
+#[rustc_then_this_would_need(ItemSignature)] //~ ERROR no path
 struct Struct {
+    #[rustc_then_this_would_need(ItemSignature)] //~ ERROR OK
     x: TypeAlias,
     y: u32
 }
 
-#[rustc_then_this_would_need(ItemSignature)] //~ ERROR OK
+#[rustc_then_this_would_need(ItemSignature)] //~ ERROR no path
 enum Enum {
-    Variant1(TypeAlias),
+    Variant1 {
+        #[rustc_then_this_would_need(ItemSignature)] //~ ERROR OK
+        t: TypeAlias
+    },
     Variant2(i32)
 }
 

src/test/compile-fail/dep-graph-variance-alias.rs

@@ -0,0 +1,32 @@
+// Copyright 2016 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+// Test that changing what a `type` points to does not go unnoticed
+// by the variance analysis.
+
+// compile-flags: -Z query-dep-graph
+
+#![feature(rustc_attrs)]
+#![allow(dead_code)]
+#![allow(unused_variables)]
+
+fn main() { }
+
+struct Foo<T> {
+    f: T
+}
+
+#[rustc_if_this_changed]
+type TypeAlias<T> = Foo<T>;
+
+#[rustc_then_this_would_need(ItemVariances)] //~ ERROR OK
+struct Use<T> {
+    x: TypeAlias<T>
+}

src/test/compile-fail/variance-regions-direct.rs

@@ -60,7 +60,6 @@ struct Test6<'a, 'b:'a> { //~ ERROR [-, o]
 
 #[rustc_variance]
 struct Test7<'a> { //~ ERROR [*]
-    //~^ ERROR parameter `'a` is never used
     x: isize
 }
 

src/test/compile-fail/variance-regions-indirect.rs

@@ -16,27 +16,23 @@
 
 #[rustc_variance]
 enum Base<'a, 'b, 'c:'b, 'd> { //~ ERROR [+, -, o, *]
-    //~^ ERROR parameter `'d` is never used
     Test8A(extern "Rust" fn(&'a isize)),
     Test8B(&'b [isize]),
     Test8C(&'b mut &'c str),
 }
 
 #[rustc_variance]
 struct Derived1<'w, 'x:'y, 'y, 'z> { //~ ERROR [*, o, -, +]
-    //~^ ERROR parameter `'w` is never used
     f: Base<'z, 'y, 'x, 'w>
 }
 
 #[rustc_variance] // Combine - and + to yield o
 struct Derived2<'a, 'b:'a, 'c> { //~ ERROR [o, o, *]
-    //~^ ERROR parameter `'c` is never used
     f: Base<'a, 'a, 'b, 'c>
 }
 
 #[rustc_variance] // Combine + and o to yield o (just pay attention to 'a here)
 struct Derived3<'a:'b, 'b, 'c> { //~ ERROR [o, -, *]
-    //~^ ERROR parameter `'c` is never used
     f: Base<'a, 'b, 'a, 'c>
 }
 

src/test/compile-fail/variance-trait-bounds.rs

@@ -30,8 +30,7 @@ struct TestStruct<U,T:Setter<U>> { //~ ERROR [+, +]
 }
 
 #[rustc_variance]
-enum TestEnum<U,T:Setter<U>> {//~ ERROR [*, +]
-    //~^ ERROR parameter `U` is never used
+enum TestEnum<U,T:Setter<U>> { //~ ERROR [*, +]
     Foo(T)
 }
 
@@ -51,13 +50,11 @@ trait TestTrait3<U> { //~ ERROR [o, o]
 
 #[rustc_variance]
 struct TestContraStruct<U,T:Setter<U>> { //~ ERROR [*, +]
-    //~^ ERROR parameter `U` is never used
     t: T
 }
 
 #[rustc_variance]
 struct TestBox<U,T:Getter<U>+Setter<U>> { //~ ERROR [*, +]
-    //~^ ERROR parameter `U` is never used
     t: T
 }