From b175aef0c40d0b60316fabc6b4023c60c5bc832e Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Sun, 23 Apr 2017 05:14:10 -0400 Subject: [PATCH 01/10] make transitive relation use a hash map --- .../transitive_relation.rs | 69 +++++++++++-------- 1 file changed, 42 insertions(+), 27 deletions(-) diff --git a/src/librustc_data_structures/transitive_relation.rs b/src/librustc_data_structures/transitive_relation.rs index b0fca5c0ff377..0d166cc0b9e81 100644 --- a/src/librustc_data_structures/transitive_relation.rs +++ b/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 { - // 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 { + // List of elements. This is used to map from a T to a usize. elements: Vec, + // Maps each element to an index. + map: FxHashMap, + // List of base edges in the graph. Require to compute transitive // closure. edges: Vec, @@ -40,19 +42,20 @@ pub struct TransitiveRelation { closure: RefCell>, } -#[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 TransitiveRelation { +impl TransitiveRelation { pub fn new() -> TransitiveRelation { TransitiveRelation { elements: vec![], + map: FxHashMap(), edges: vec![], closure: RefCell::new(None), } @@ -63,21 +66,27 @@ impl TransitiveRelation { } fn index(&self, a: &T) -> Option { - 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 @@ -85,7 +94,7 @@ impl TransitiveRelation { /// `None`. pub fn maybe_map(&self, mut f: F) -> Option> where F: FnMut(&T) -> Option, - U: Debug + PartialEq, + U: Clone + Debug + Eq + Hash + Clone, { let mut result = TransitiveRelation::new(); for edge in &self.edges { @@ -335,7 +344,7 @@ fn pare_down(candidates: &mut Vec, closure: &BitMatrix) { } impl Encodable for TransitiveRelation - where T: Encodable + Debug + PartialEq + where T: Clone + Encodable + Debug + Eq + Hash + Clone { fn encode(&self, s: &mut E) -> Result<(), E::Error> { s.emit_struct("TransitiveRelation", 2, |s| { @@ -347,19 +356,23 @@ impl Encodable for TransitiveRelation } impl Decodable for TransitiveRelation - where T: Decodable + Debug + PartialEq + where T: Clone + Decodable + Debug + Eq + Hash + Clone { fn decode(d: &mut D) -> Result { d.read_struct("TransitiveRelation", 2, |d| { - let elements = d.read_struct_field("elements", 0, |d| Decodable::decode(d))?; + let elements: Vec = 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 HashStable for TransitiveRelation - where T: HashStable + PartialEq + Debug + where T: HashStable + Eq + Debug + Clone + Hash { fn hash_stable(&self, hcx: &mut CTX, @@ -369,6 +382,8 @@ impl HashStable for TransitiveRelation 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; From 55412a201aa687e16cb76681d8dc7d595253800d Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Sun, 23 Apr 2017 05:44:02 -0400 Subject: [PATCH 02/10] track `CurrentItem`, not just `Generics` --- src/librustc_typeck/variance/constraints.rs | 91 ++++++++++++--------- 1 file changed, 54 insertions(+), 37 deletions(-) diff --git a/src/librustc_typeck/variance/constraints.rs b/src/librustc_typeck/variance/constraints.rs index 529b2700679d5..a617551eeb191 100644 --- a/src/librustc_typeck/variance/constraints.rs +++ b/src/librustc_typeck/variance/constraints.rs @@ -48,6 +48,21 @@ pub struct Constraint<'a> { pub variance: &'a VarianceTerm<'a>, } +/// To build constriants, we visit one item (type, trait) at a time +/// and look at its contents. So e.g. if we have +/// +/// struct Foo { +/// b: Bar +/// } +/// +/// then while we are visiting `Bar`, the `CurrentItem` would have +/// the def-id and generics of `Foo`. +#[allow(dead_code)] // TODO -- `def_id` field not used yet +pub struct CurrentItem<'a> { + def_id: DefId, + generics: &'a ty::Generics, +} + pub fn add_constraints_from_crate<'a, 'tcx>(terms_cx: TermsContext<'a, 'tcx>) -> ConstraintContext<'a, 'tcx> { let tcx = terms_cx.tcx; @@ -73,7 +88,7 @@ pub fn add_constraints_from_crate<'a, 'tcx>(terms_cx: TermsContext<'a, 'tcx>) impl<'a, 'tcx, 'v> ItemLikeVisitor<'v> for ConstraintContext<'a, 'tcx> { fn visit_item(&mut self, item: &hir::Item) { let tcx = self.terms_cx.tcx; - let did = tcx.hir.local_def_id(item.id); + let def_id = tcx.hir.local_def_id(item.id); debug!("visit_item item={}", tcx.hir.node_to_string(item.id)); @@ -82,6 +97,7 @@ impl<'a, 'tcx, 'v> ItemLikeVisitor<'v> for ConstraintContext<'a, 'tcx> { hir::ItemStruct(..) | hir::ItemUnion(..) => { let generics = tcx.generics_of(did); + let current_item = &CurrentItem { def_id, generics }; // Not entirely obvious: constraints on structs/enums do not // affect the variance of their type parameters. See discussion @@ -90,18 +106,19 @@ impl<'a, 'tcx, 'v> ItemLikeVisitor<'v> for ConstraintContext<'a, 'tcx> { // self.add_constraints_from_generics(generics); for field in tcx.adt_def(did).all_fields() { - self.add_constraints_from_ty(generics, - tcx.type_of(field.did), + self.add_constraints_from_ty(current_item, + tcx.item_type(field.did), self.covariant); } } hir::ItemTrait(..) => { let generics = tcx.generics_of(did); + let current_item = &CurrentItem { def_id, generics }; let trait_ref = ty::TraitRef { - def_id: did, - substs: Substs::identity_for_item(tcx, did) + def_id: def_id, + substs: Substs::identity_for_item(tcx, def_id) }; - self.add_constraints_from_trait_ref(generics, + self.add_constraints_from_trait_ref(current_item, trait_ref, self.invariant); } @@ -279,7 +296,7 @@ impl<'a, 'tcx> ConstraintContext<'a, 'tcx> { } fn add_constraints_from_trait_ref(&mut self, - generics: &ty::Generics, + current: &CurrentItem, trait_ref: ty::TraitRef<'tcx>, variance: VarianceTermPtr<'a>) { debug!("add_constraints_from_trait_ref: trait_ref={:?} variance={:?}", @@ -293,7 +310,7 @@ impl<'a, 'tcx> ConstraintContext<'a, 'tcx> { // README.md for a discussion on dep-graph management. self.tcx().dep_graph.read(VarianceDepNode(trait_ref.def_id)); - self.add_constraints_from_substs(generics, + self.add_constraints_from_substs(current, trait_ref.def_id, &trait_generics.types, &trait_generics.regions, @@ -305,7 +322,7 @@ impl<'a, 'tcx> ConstraintContext<'a, 'tcx> { /// in a context with the generics defined in `generics` and /// ambient variance `variance` fn add_constraints_from_ty(&mut self, - generics: &ty::Generics, + current: &CurrentItem, ty: Ty<'tcx>, variance: VarianceTermPtr<'a>) { debug!("add_constraints_from_ty(ty={:?}, variance={:?})", @@ -325,22 +342,22 @@ impl<'a, 'tcx> ConstraintContext<'a, 'tcx> { ty::TyRef(region, ref mt) => { let contra = self.contravariant(variance); - self.add_constraints_from_region(generics, region, contra); - self.add_constraints_from_mt(generics, mt, variance); + self.add_constraints_from_region(current, region, contra); + self.add_constraints_from_mt(current, mt, variance); } ty::TyArray(typ, _) | ty::TySlice(typ) => { - self.add_constraints_from_ty(generics, typ, variance); + self.add_constraints_from_ty(current, typ, variance); } ty::TyRawPtr(ref mt) => { - self.add_constraints_from_mt(generics, mt, variance); + self.add_constraints_from_mt(current, mt, variance); } ty::TyTuple(subtys, _) => { for &subty in subtys { - self.add_constraints_from_ty(generics, subty, variance); + self.add_constraints_from_ty(current, subty, variance); } } @@ -352,7 +369,7 @@ impl<'a, 'tcx> ConstraintContext<'a, 'tcx> { // README.md for a discussion on dep-graph management. self.tcx().dep_graph.read(VarianceDepNode(def.did)); - self.add_constraints_from_substs(generics, + self.add_constraints_from_substs(current, def.did, &adt_generics.types, &adt_generics.regions, @@ -369,7 +386,7 @@ impl<'a, 'tcx> ConstraintContext<'a, 'tcx> { // README.md for a discussion on dep-graph management. self.tcx().dep_graph.read(VarianceDepNode(trait_ref.def_id)); - self.add_constraints_from_substs(generics, + self.add_constraints_from_substs(current, trait_ref.def_id, &trait_generics.types, &trait_generics.regions, @@ -380,25 +397,25 @@ impl<'a, 'tcx> ConstraintContext<'a, 'tcx> { ty::TyDynamic(ref data, r) => { // The type `Foo` is contravariant w/r/t `'a`: let contra = self.contravariant(variance); - self.add_constraints_from_region(generics, r, contra); + self.add_constraints_from_region(current, r, contra); if let Some(p) = data.principal() { let poly_trait_ref = p.with_self_ty(self.tcx(), self.tcx().types.err); - self.add_constraints_from_trait_ref(generics, poly_trait_ref.0, variance); + self.add_constraints_from_trait_ref(current, poly_trait_ref.0, variance); } for projection in data.projection_bounds() { - self.add_constraints_from_ty(generics, projection.0.ty, self.invariant); + self.add_constraints_from_ty(current, projection.0.ty, self.invariant); } } ty::TyParam(ref data) => { - assert_eq!(generics.parent, None); + assert_eq!(current.generics.parent, None); let mut i = data.idx as usize; - if !generics.has_self || i > 0 { - i -= generics.regions.len(); + if !current.generics.has_self || i > 0 { + i -= current.generics.regions.len(); } - let def_id = generics.types[i].def_id; + let def_id = current.generics.types[i].def_id; let node_id = self.tcx().hir.as_local_node_id(def_id).unwrap(); match self.terms_cx.inferred_map.get(&node_id) { Some(&index) => { @@ -414,7 +431,7 @@ impl<'a, 'tcx> ConstraintContext<'a, 'tcx> { ty::TyFnDef(.., sig) | ty::TyFnPtr(sig) => { - self.add_constraints_from_sig(generics, sig, variance); + self.add_constraints_from_sig(current, sig, variance); } ty::TyError => { @@ -433,7 +450,7 @@ impl<'a, 'tcx> ConstraintContext<'a, 'tcx> { /// Adds constraints appropriate for a nominal type (enum, struct, /// object, etc) appearing in a context with ambient variance `variance` fn add_constraints_from_substs(&mut self, - generics: &ty::Generics, + current: &CurrentItem, def_id: DefId, type_param_defs: &[ty::TypeParameterDef], region_param_defs: &[ty::RegionParameterDef], @@ -451,41 +468,41 @@ impl<'a, 'tcx> ConstraintContext<'a, 'tcx> { debug!("add_constraints_from_substs: variance_decl={:?} variance_i={:?}", variance_decl, variance_i); - self.add_constraints_from_ty(generics, substs_ty, variance_i); + self.add_constraints_from_ty(current, substs_ty, variance_i); } for p in region_param_defs { let variance_decl = self.declared_variance(p.def_id, def_id, p.index as usize); let variance_i = self.xform(variance, variance_decl); let substs_r = substs.region_for_def(p); - self.add_constraints_from_region(generics, substs_r, variance_i); + self.add_constraints_from_region(current, substs_r, variance_i); } } /// Adds constraints appropriate for a function with signature /// `sig` appearing in a context with ambient variance `variance` fn add_constraints_from_sig(&mut self, - generics: &ty::Generics, + current: &CurrentItem, sig: ty::PolyFnSig<'tcx>, variance: VarianceTermPtr<'a>) { let contra = self.contravariant(variance); for &input in sig.0.inputs() { - self.add_constraints_from_ty(generics, input, contra); + self.add_constraints_from_ty(current, input, contra); } - self.add_constraints_from_ty(generics, sig.0.output(), variance); + self.add_constraints_from_ty(current, sig.0.output(), variance); } /// Adds constraints appropriate for a region appearing in a /// context with ambient variance `variance` fn add_constraints_from_region(&mut self, - generics: &ty::Generics, + current: &CurrentItem, region: ty::Region<'tcx>, variance: VarianceTermPtr<'a>) { match *region { ty::ReEarlyBound(ref data) => { - assert_eq!(generics.parent, None); - let i = data.index as usize - generics.has_self as usize; - let def_id = generics.regions[i].def_id; + assert_eq!(current.generics.parent, None); + let i = data.index as usize - current.generics.has_self as usize; + let def_id = current.generics.regions[i].def_id; let node_id = self.tcx().hir.as_local_node_id(def_id).unwrap(); if self.is_to_be_inferred(node_id) { let index = self.inferred_index(node_id); @@ -518,17 +535,17 @@ impl<'a, 'tcx> ConstraintContext<'a, 'tcx> { /// Adds constraints appropriate for a mutability-type pair /// appearing in a context with ambient variance `variance` fn add_constraints_from_mt(&mut self, - generics: &ty::Generics, + current: &CurrentItem, mt: &ty::TypeAndMut<'tcx>, variance: VarianceTermPtr<'a>) { match mt.mutbl { hir::MutMutable => { let invar = self.invariant(variance); - self.add_constraints_from_ty(generics, mt.ty, invar); + self.add_constraints_from_ty(current, mt.ty, invar); } hir::MutImmutable => { - self.add_constraints_from_ty(generics, mt.ty, variance); + self.add_constraints_from_ty(current, mt.ty, variance); } } } From 4824a199ca3c6c1c12c90f0146db540045220c9a Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Mon, 24 Apr 2017 11:15:12 -0400 Subject: [PATCH 03/10] factor variances into a proper query There are now two queries: crate and item. The crate one computes the variance of all items in the crate; it is sort of an implementation detail, and not meant to be used. The item one reads from the crate one, synthesizing correct deps in lieu of the red-green algorithm. At the same time, remove the `variance_computed` flag, which was a horrible hack used to force invariance early on (e.g. when type-checking constants). This is only needed because of trait applications, and traits are always invariant anyway. Therefore, we now change to take advantage of the query system: - When asked to compute variances for a trait, just return a vector saying 'all invariant'. - Remove the corresponding "inferreds" from traits, and tweak the constraint generation code to understand that traits are always inferred. --- src/librustc/dep_graph/dep_node.rs | 6 + src/librustc/ty/context.rs | 4 - src/librustc/ty/maps.rs | 16 +- src/librustc/ty/mod.rs | 22 +++ src/librustc/ty/relate.rs | 10 +- .../transitive_relation.rs | 14 ++ src/librustc_typeck/lib.rs | 4 +- src/librustc_typeck/variance/README.md | 68 +++---- src/librustc_typeck/variance/constraints.rs | 167 +++++++++++------- src/librustc_typeck/variance/mod.rs | 63 ++++++- src/librustc_typeck/variance/solve.rs | 24 +-- src/librustc_typeck/variance/terms.rs | 39 +--- .../compile-fail/dep-graph-variance-alias.rs | 32 ++++ 13 files changed, 295 insertions(+), 174 deletions(-) create mode 100644 src/test/compile-fail/dep-graph-variance-alias.rs diff --git a/src/librustc/dep_graph/dep_node.rs b/src/librustc/dep_graph/dep_node.rs index 66505d9a06b59..b4920f909cbda 100644 --- a/src/librustc/dep_graph/dep_node.rs +++ b/src/librustc/dep_graph/dep_node.rs @@ -81,6 +81,7 @@ pub enum DepNode { 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 { // predicates for an item wind up in `ItemSignature`). AssociatedItems(D), ItemSignature(D), + ItemVarianceConstraints(D), + ItemVariances(D), IsForeignItem(D), TypeParamPredicates((D, D)), SizedConstraint(D), @@ -199,6 +202,7 @@ impl DepNode { 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 +234,8 @@ impl DepNode { 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))))) diff --git a/src/librustc/ty/context.rs b/src/librustc/ty/context.rs index 0dd23eb7e700c..13295e4030d56 100644 --- a/src/librustc/ty/context.rs +++ b/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, - /// Set of used unsafe nodes (functions or blocks). Unsafe nodes not /// present in this set can be warned about. pub used_unsafe: RefCell, @@ -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), diff --git a/src/librustc/ty/maps.rs b/src/librustc/ty/maps.rs index 385abbd039e08..3a2a614734582 100644 --- a/src/librustc/ty/maps.rs +++ b/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 `{}`", @@ -673,9 +679,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, + /// Maps from def-id of a type or region parameter to its /// (inferred) variance. - [pub] variances_of: ItemSignature(DefId) -> Rc>, + [pub] variances_of: ItemVariances(DefId) -> Rc>, /// Maps from an impl/trait def-id to a list of the def-ids of its items [] associated_item_def_ids: AssociatedItemDefIds(DefId) -> Rc>, @@ -810,3 +820,7 @@ fn const_eval_dep_node((def_id, _): (DefId, &Substs)) -> DepNode { fn mir_keys(_: CrateNum) -> DepNode { DepNode::MirKeys } + +fn crate_variances(_: CrateNum) -> DepNode { + DepNode::CrateVariances +} diff --git a/src/librustc/ty/mod.rs b/src/librustc/ty/mod.rs index 8191b392de528..481098450eda4 100644 --- a/src/librustc/ty/mod.rs +++ b/src/librustc/ty/mod.rs @@ -55,6 +55,7 @@ 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; @@ -309,6 +310,27 @@ pub enum Variance { Bivariant, // T <: T -- 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.item_variances()` 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. + pub variances: FxHashMap>>, + + /// An empty vector, useful for cloning. + pub empty_variance: Rc>, +} + #[derive(Clone, Copy, Debug, RustcDecodable, RustcEncodable)] pub struct MethodCallee<'tcx> { /// Impl method ID, for inherent methods, or trait method ID, otherwise. diff --git a/src/librustc/ty/relate.rs b/src/librustc/ty/relate.rs index ac434c01c6a88..dfa11b9c71a04 100644 --- a/src/librustc/ty/relate.rs +++ b/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, diff --git a/src/librustc_data_structures/transitive_relation.rs b/src/librustc_data_structures/transitive_relation.rs index 0d166cc0b9e81..46463944043bd 100644 --- a/src/librustc_data_structures/transitive_relation.rs +++ b/src/librustc_data_structures/transitive_relation.rs @@ -134,6 +134,20 @@ impl TransitiveRelation { } } + /// Returns a vector of all things less than `a`. + /// + /// Really this probably ought to be `impl Iterator`, 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 diff --git a/src/librustc_typeck/lib.rs b/src/librustc_typeck/lib.rs index 94b4bfade9498..efb7cacc5820c 100644 --- a/src/librustc_typeck/lib.rs +++ b/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)); diff --git a/src/librustc_typeck/variance/README.md b/src/librustc_typeck/variance/README.md index ac785e4058bde..9ec20c1a45c34 100644 --- a/src/librustc_typeck/variance/README.md +++ b/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. +- `item_variances` accesses the variance for an individual reading; it + works by requesting `crate_variances` and extracting the relevant data. + +If you limit yourself to reading `item_variances`, 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 +`item_variances` 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 diff --git a/src/librustc_typeck/variance/constraints.rs b/src/librustc_typeck/variance/constraints.rs index a617551eeb191..e986a381cd963 100644 --- a/src/librustc_typeck/variance/constraints.rs +++ b/src/librustc_typeck/variance/constraints.rs @@ -15,6 +15,7 @@ use hir::def_id::DefId; use middle::resolve_lifetime as rl; +use rustc::dep_graph::{AssertDepGraphSafe, DepNode}; use rustc::ty::subst::Substs; use rustc::ty::{self, Ty, TyCtxt}; use rustc::hir::map as hir_map; @@ -22,12 +23,12 @@ use syntax::ast; use rustc::hir; use rustc::hir::itemlikevisit::ItemLikeVisitor; +use rustc_data_structures::transitive_relation::TransitiveRelation; + use super::terms::*; use super::terms::VarianceTerm::*; use super::xform::*; -use dep_graph::DepNode::ItemSignature as VarianceDepNode; - pub struct ConstraintContext<'a, 'tcx: 'a> { pub terms_cx: TermsContext<'a, 'tcx>, @@ -38,6 +39,11 @@ 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 @@ -57,7 +63,6 @@ pub struct Constraint<'a> { /// /// then while we are visiting `Bar`, the `CurrentItem` would have /// the def-id and generics of `Foo`. -#[allow(dead_code)] // TODO -- `def_id` field not used yet pub struct CurrentItem<'a> { def_id: DefId, generics: &'a ty::Generics, @@ -77,10 +82,10 @@ pub fn add_constraints_from_crate<'a, 'tcx>(terms_cx: TermsContext<'a, 'tcx>) invariant: invariant, bivariant: bivariant, constraints: Vec::new(), + dependencies: TransitiveRelation::new(), }; - // See README.md for a discussion on dep-graph management. - tcx.visit_all_item_likes_in_krate(VarianceDepNode, &mut constraint_cx); + tcx.hir.krate().visit_all_item_likes(&mut constraint_cx); constraint_cx } @@ -90,13 +95,64 @@ impl<'a, 'tcx, 'v> ItemLikeVisitor<'v> for ConstraintContext<'a, 'tcx> { let tcx = self.terms_cx.tcx; let def_id = tcx.hir.local_def_id(item.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. + match item.node { + hir::ItemEnum(..) | + hir::ItemStruct(..) | + hir::ItemUnion(..) => { + tcx.dep_graph.with_task(DepNode::ItemVarianceConstraints(def_id), + AssertDepGraphSafe(self), + def_id, + visit_item_task); + } + _ => { + // Nothing to do here, skip the task. + } + } + + fn visit_item_task<'a, 'tcx>(ccx: AssertDepGraphSafe<&mut ConstraintContext<'a, 'tcx>>, + def_id: DefId) + { + ccx.0.build_constraints_for_item(def_id); + } + } + + fn visit_trait_item(&mut self, _trait_item: &hir::TraitItem) { + } + + fn visit_impl_item(&mut self, _impl_item: &hir::ImplItem) { + } +} + +/// Is `param_id` a lifetime according to `map`? +fn is_lifetime(map: &hir_map::Map, param_id: ast::NodeId) -> bool { + match map.find(param_id) { + Some(hir_map::NodeLifetime(..)) => true, + _ => false, + } +} + +impl<'a, 'tcx> ConstraintContext<'a, 'tcx> { + fn tcx(&self) -> TyCtxt<'a, 'tcx, 'tcx> { + self.terms_cx.tcx + } + + fn build_constraints_for_item(&mut self, def_id: DefId) { + let tcx = self.tcx(); + let id = self.tcx().hir.as_local_node_id(def_id).unwrap(); + let item = tcx.hir.expect_item(id); debug!("visit_item item={}", tcx.hir.node_to_string(item.id)); match item.node { hir::ItemEnum(..) | hir::ItemStruct(..) | hir::ItemUnion(..) => { - let generics = tcx.generics_of(did); + let generics = tcx.generics_of(def_id); let current_item = &CurrentItem { def_id, generics }; // Not entirely obvious: constraints on structs/enums do not @@ -105,24 +161,14 @@ impl<'a, 'tcx, 'v> ItemLikeVisitor<'v> for ConstraintContext<'a, 'tcx> { // // self.add_constraints_from_generics(generics); - for field in tcx.adt_def(did).all_fields() { + for field in tcx.adt_def(def_id).all_fields() { self.add_constraints_from_ty(current_item, - tcx.item_type(field.did), + tcx.type_of(field.did), self.covariant); } } - hir::ItemTrait(..) => { - let generics = tcx.generics_of(did); - let current_item = &CurrentItem { def_id, generics }; - let trait_ref = ty::TraitRef { - def_id: def_id, - substs: Substs::identity_for_item(tcx, def_id) - }; - self.add_constraints_from_trait_ref(current_item, - trait_ref, - self.invariant); - } + hir::ItemTrait(..) | hir::ItemExternCrate(_) | hir::ItemUse(..) | hir::ItemStatic(..) | @@ -133,38 +179,25 @@ impl<'a, 'tcx, 'v> ItemLikeVisitor<'v> for ConstraintContext<'a, 'tcx> { hir::ItemGlobalAsm(..) | hir::ItemTy(..) | hir::ItemImpl(..) | - hir::ItemDefaultImpl(..) => {} + hir::ItemDefaultImpl(..) => { + span_bug!(item.span, "`build_constraints_for_item` invoked for non-type-def"); + } } } - fn visit_trait_item(&mut self, _trait_item: &hir::TraitItem) { - } - - fn visit_impl_item(&mut self, _impl_item: &hir::ImplItem) { - } -} - -/// Is `param_id` a lifetime according to `map`? -fn is_lifetime(map: &hir_map::Map, param_id: ast::NodeId) -> bool { - match map.find(param_id) { - Some(hir_map::NodeLifetime(..)) => true, - _ => false, - } -} - -impl<'a, 'tcx> ConstraintContext<'a, 'tcx> { - fn tcx(&self) -> TyCtxt<'a, 'tcx, 'tcx> { - self.terms_cx.tcx + /// Load the generics for another item, adding a corresponding + /// relation into the dependencies to indicate that the variance + /// for `current` relies on `def_id`. + fn read_generics(&mut self, current: &CurrentItem, def_id: DefId) -> &'tcx ty::Generics { + let generics = self.tcx().generics_of(def_id); + if self.tcx().dep_graph.is_fully_enabled() { + self.dependencies.add(current.def_id, def_id); + } + generics } - fn inferred_index(&self, param_id: ast::NodeId) -> InferredIndex { - match self.terms_cx.inferred_map.get(¶m_id) { - Some(&index) => index, - None => { - bug!("no inferred index entry for {}", - self.tcx().hir.node_to_string(param_id)); - } - } + fn opt_inferred_index(&self, param_id: ast::NodeId) -> Option<&InferredIndex> { + self.terms_cx.inferred_map.get(¶m_id) } fn find_binding_for_lifetime(&self, param_id: ast::NodeId) -> ast::NodeId { @@ -245,8 +278,27 @@ impl<'a, 'tcx> ConstraintContext<'a, 'tcx> { // Parameter on an item defined within current crate: // variance not yet inferred, so return a symbolic // variance. - let InferredIndex(index) = self.inferred_index(param_node_id); - self.terms_cx.inferred_infos[index].term + if let Some(&InferredIndex(index)) = self.opt_inferred_index(param_node_id) { + self.terms_cx.inferred_infos[index].term + } else { + // If there is no inferred entry for a type parameter, + // it must be declared on a (locally defiend) trait -- they don't + // get inferreds because they are always invariant. + if cfg!(debug_assertions) { + let item_node_id = self.tcx().hir.as_local_node_id(item_def_id).unwrap(); + let item = self.tcx().hir.expect_item(item_node_id); + let success = match item.node { + hir::ItemTrait(..) => true, + _ => false, + }; + if !success { + bug!("parameter {:?} has no inferred, but declared on non-trait: {:?}", + item_def_id, + item); + } + } + self.invariant + } } else { // Parameter on an item defined within another crate: // variance already inferred, just look it up. @@ -305,11 +357,6 @@ impl<'a, 'tcx> ConstraintContext<'a, 'tcx> { let trait_generics = self.tcx().generics_of(trait_ref.def_id); - // This edge is actually implied by the call to - // `trait_def`, but I'm trying to be future-proof. See - // README.md for a discussion on dep-graph management. - self.tcx().dep_graph.read(VarianceDepNode(trait_ref.def_id)); - self.add_constraints_from_substs(current, trait_ref.def_id, &trait_generics.types, @@ -362,12 +409,7 @@ impl<'a, 'tcx> ConstraintContext<'a, 'tcx> { } ty::TyAdt(def, substs) => { - let adt_generics = self.tcx().generics_of(def.did); - - // This edge is actually implied by the call to - // `trait_def`, but I'm trying to be future-proof. See - // README.md for a discussion on dep-graph management. - self.tcx().dep_graph.read(VarianceDepNode(def.did)); + let adt_generics = self.read_generics(current, def.did); self.add_constraints_from_substs(current, def.did, @@ -381,11 +423,6 @@ impl<'a, 'tcx> ConstraintContext<'a, 'tcx> { let trait_ref = &data.trait_ref; let trait_generics = self.tcx().generics_of(trait_ref.def_id); - // This edge is actually implied by the call to - // `trait_def`, but I'm trying to be future-proof. See - // README.md for a discussion on dep-graph management. - self.tcx().dep_graph.read(VarianceDepNode(trait_ref.def_id)); - self.add_constraints_from_substs(current, trait_ref.def_id, &trait_generics.types, @@ -505,7 +542,7 @@ impl<'a, 'tcx> ConstraintContext<'a, 'tcx> { let def_id = current.generics.regions[i].def_id; let node_id = self.tcx().hir.as_local_node_id(def_id).unwrap(); if self.is_to_be_inferred(node_id) { - let index = self.inferred_index(node_id); + let &index = self.opt_inferred_index(node_id).unwrap(); self.add_constraint(index, variance); } } diff --git a/src/librustc_typeck/variance/mod.rs b/src/librustc_typeck/variance/mod.rs index cd0ab1cbb9543..99e23589dd730 100644 --- a/src/librustc_typeck/variance/mod.rs +++ b/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. @@ -27,10 +32,60 @@ mod solve; /// 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 { + 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> { + 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") + } + } } diff --git a/src/librustc_typeck/variance/solve.rs b/src/librustc_typeck/variance/solve.rs index 27116cbbb7aef..5ba5005ccc210 100644 --- a/src/librustc_typeck/variance/solve.rs +++ b/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, } -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>> { // 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; @@ -137,9 +138,10 @@ impl<'a, 'tcx> SolveContext<'a, 'tcx> { 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 { diff --git a/src/librustc_typeck/variance/terms.rs b/src/librustc_typeck/variance/terms.rs index 61ff154e458d3..3f4b15e35a42d 100644 --- a/src/librustc_typeck/variance/terms.rs +++ b/src/librustc_typeck/variance/terms.rs @@ -139,7 +139,6 @@ fn lang_items(tcx: TyCtxt) -> Vec<(ast::NodeId, Vec)> { 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 +148,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. + // `CrateVariancesMap`, 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); - } - - 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 +210,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(..) | diff --git a/src/test/compile-fail/dep-graph-variance-alias.rs b/src/test/compile-fail/dep-graph-variance-alias.rs new file mode 100644 index 0000000000000..9b621a13fc484 --- /dev/null +++ b/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 or the MIT license +// , 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 { + f: T +} + +#[rustc_if_this_changed] +type TypeAlias = Foo; + +#[rustc_then_this_would_need(ItemVariances)] //~ ERROR OK +struct Use { + x: TypeAlias +} From 4355169f8adf31f9722da28a988fa77589f44722 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Tue, 2 May 2017 11:22:03 -0400 Subject: [PATCH 04/10] kill the old `visit_all_item_likes` infrastructure --- src/librustc/dep_graph/mod.rs | 2 - src/librustc/dep_graph/visit.rs | 77 --------------------------- src/librustc/hir/intravisit.rs | 2 +- src/librustc/hir/itemlikevisit.rs | 5 +- src/librustc/ty/mod.rs | 11 +--- src/librustc_typeck/variance/terms.rs | 4 +- 6 files changed, 5 insertions(+), 96 deletions(-) delete mode 100644 src/librustc/dep_graph/visit.rs diff --git a/src/librustc/dep_graph/mod.rs b/src/librustc/dep_graph/mod.rs index 6cb86a30400a7..809bed939f54c 100644 --- a/src/librustc/dep_graph/mod.rs +++ b/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; diff --git a/src/librustc/dep_graph/visit.rs b/src/librustc/dep_graph/visit.rs deleted file mode 100644 index bf3748659fe07..0000000000000 --- a/src/librustc/dep_graph/visit.rs +++ /dev/null @@ -1,77 +0,0 @@ -// Copyright 2014 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 or the MIT license -// , at your -// option. This file may not be copied, modified, or distributed -// except according to those terms. - -use hir; -use hir::def_id::DefId; -use hir::itemlikevisit::ItemLikeVisitor; -use ty::TyCtxt; - -use super::dep_node::DepNode; - -/// Visit all the items in the krate in some order. When visiting a -/// particular item, first create a dep-node by calling `dep_node_fn` -/// and push that onto the dep-graph stack of tasks, and also create a -/// read edge from the corresponding AST node. This is used in -/// compiler passes to automatically record the item that they are -/// working on. -pub fn visit_all_item_likes_in_krate<'a, 'tcx, V, F>(tcx: TyCtxt<'a, 'tcx, 'tcx>, - mut dep_node_fn: F, - visitor: &mut V) - where F: FnMut(DefId) -> DepNode, V: ItemLikeVisitor<'tcx> -{ - struct TrackingVisitor<'visit, 'tcx: 'visit, F: 'visit, V: 'visit> { - tcx: TyCtxt<'visit, 'tcx, 'tcx>, - dep_node_fn: &'visit mut F, - visitor: &'visit mut V, - } - - impl<'visit, 'tcx, F, V> ItemLikeVisitor<'tcx> for TrackingVisitor<'visit, 'tcx, F, V> - where F: FnMut(DefId) -> DepNode, V: ItemLikeVisitor<'tcx> - { - fn visit_item(&mut self, i: &'tcx hir::Item) { - let item_def_id = self.tcx.hir.local_def_id(i.id); - let task_id = (self.dep_node_fn)(item_def_id); - let _task = self.tcx.dep_graph.in_task(task_id.clone()); - debug!("Started task {:?}", task_id); - self.tcx.dep_graph.read(DepNode::Hir(item_def_id)); - self.visitor.visit_item(i); - debug!("Ended task {:?}", task_id); - } - - fn visit_trait_item(&mut self, i: &'tcx hir::TraitItem) { - let trait_item_def_id = self.tcx.hir.local_def_id(i.id); - let task_id = (self.dep_node_fn)(trait_item_def_id); - let _task = self.tcx.dep_graph.in_task(task_id.clone()); - debug!("Started task {:?}", task_id); - self.tcx.dep_graph.read(DepNode::Hir(trait_item_def_id)); - self.visitor.visit_trait_item(i); - debug!("Ended task {:?}", task_id); - } - - fn visit_impl_item(&mut self, i: &'tcx hir::ImplItem) { - let impl_item_def_id = self.tcx.hir.local_def_id(i.id); - let task_id = (self.dep_node_fn)(impl_item_def_id); - let _task = self.tcx.dep_graph.in_task(task_id.clone()); - debug!("Started task {:?}", task_id); - self.tcx.dep_graph.read(DepNode::Hir(impl_item_def_id)); - self.visitor.visit_impl_item(i); - debug!("Ended task {:?}", task_id); - } - } - - let krate = tcx.dep_graph.with_ignore(|| tcx.hir.krate()); - let mut tracking_visitor = TrackingVisitor { - tcx: tcx, - dep_node_fn: &mut dep_node_fn, - visitor: visitor, - }; - krate.visit_all_item_likes(&mut tracking_visitor) -} - diff --git a/src/librustc/hir/intravisit.rs b/src/librustc/hir/intravisit.rs index 3e610dd3c0d87..def6b2b3421f6 100644 --- a/src/librustc/hir/intravisit.rs +++ b/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>), diff --git a/src/librustc/hir/itemlikevisit.rs b/src/librustc/hir/itemlikevisit.rs index 0d79017066b01..ce1a34faf5ee8 100644 --- a/src/librustc/hir/itemlikevisit.rs +++ b/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. diff --git a/src/librustc/ty/mod.rs b/src/librustc/ty/mod.rs index 481098450eda4..2355ed7e147b3 100644 --- a/src/librustc/ty/mod.rs +++ b/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}; @@ -58,7 +58,6 @@ use rustc_data_structures::stable_hasher::{StableHasher, StableHasherResult, 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}; @@ -2565,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(self, - dep_node_fn: F, - visitor: &mut V) - where F: FnMut(DefId) -> DepNode, 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 { diff --git a/src/librustc_typeck/variance/terms.rs b/src/librustc_typeck/variance/terms.rs index 3f4b15e35a42d..ad787c57e76f2 100644 --- a/src/librustc_typeck/variance/terms.rs +++ b/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 } From 898f978c97caf0e04c342b3a9d8b5cb680c82ce4 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Tue, 25 Apr 2017 05:45:59 -0400 Subject: [PATCH 05/10] add back variance testing mechanism make it work for traits etc uniformly --- src/librustc_typeck/lib.rs | 5 +++ src/librustc_typeck/variance/mod.rs | 4 ++ src/librustc_typeck/variance/solve.rs | 10 ----- src/librustc_typeck/variance/test.rs | 41 +++++++++++++++++++ .../compile-fail/variance-regions-direct.rs | 1 - .../compile-fail/variance-regions-indirect.rs | 4 -- .../compile-fail/variance-trait-bounds.rs | 5 +-- 7 files changed, 51 insertions(+), 19 deletions(-) create mode 100644 src/librustc_typeck/variance/test.rs diff --git a/src/librustc_typeck/lib.rs b/src/librustc_typeck/lib.rs index efb7cacc5820c..8cee1c8ba29dc 100644 --- a/src/librustc_typeck/lib.rs +++ b/src/librustc_typeck/lib.rs @@ -318,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))?; diff --git a/src/librustc_typeck/variance/mod.rs b/src/librustc_typeck/variance/mod.rs index 99e23589dd730..1afe2725ac87d 100644 --- a/src/librustc_typeck/variance/mod.rs +++ b/src/librustc_typeck/variance/mod.rs @@ -29,6 +29,9 @@ 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; @@ -89,3 +92,4 @@ fn variances_of<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, item_def_id: DefId) } } } + diff --git a/src/librustc_typeck/variance/solve.rs b/src/librustc_typeck/variance/solve.rs index 5ba5005ccc210..af8ad491ec00e 100644 --- a/src/librustc_typeck/variance/solve.rs +++ b/src/librustc_typeck/variance/solve.rs @@ -128,16 +128,6 @@ 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); - } - map.insert(item_def_id, Rc::new(item_variances)); } diff --git a/src/librustc_typeck/variance/test.rs b/src/librustc_typeck/variance/test.rs new file mode 100644 index 0000000000000..53f2bd37fbc61 --- /dev/null +++ b/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 or the MIT license +// , 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 item_variances = self.tcx.variances_of(item_def_id); + span_err!(self.tcx.sess, + item.span, + E0208, + "{:?}", + item_variances); + } + } + + fn visit_trait_item(&mut self, _: &'tcx hir::TraitItem) { } + fn visit_impl_item(&mut self, _: &'tcx hir::ImplItem) { } +} diff --git a/src/test/compile-fail/variance-regions-direct.rs b/src/test/compile-fail/variance-regions-direct.rs index bf46edcfab8b1..cd047cc580f87 100644 --- a/src/test/compile-fail/variance-regions-direct.rs +++ b/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 } diff --git a/src/test/compile-fail/variance-regions-indirect.rs b/src/test/compile-fail/variance-regions-indirect.rs index e28828f62e52d..60d4d73fe88b8 100644 --- a/src/test/compile-fail/variance-regions-indirect.rs +++ b/src/test/compile-fail/variance-regions-indirect.rs @@ -16,7 +16,6 @@ #[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), @@ -24,19 +23,16 @@ enum Base<'a, 'b, 'c:'b, 'd> { //~ ERROR [+, -, o, *] #[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> } diff --git a/src/test/compile-fail/variance-trait-bounds.rs b/src/test/compile-fail/variance-trait-bounds.rs index 4c737a7594d26..58fb785c48ca7 100644 --- a/src/test/compile-fail/variance-trait-bounds.rs +++ b/src/test/compile-fail/variance-trait-bounds.rs @@ -30,8 +30,7 @@ struct TestStruct> { //~ ERROR [+, +] } #[rustc_variance] -enum TestEnum> {//~ ERROR [*, +] - //~^ ERROR parameter `U` is never used +enum TestEnum> { //~ ERROR [*, +] Foo(T) } @@ -51,13 +50,11 @@ trait TestTrait3 { //~ ERROR [o, o] #[rustc_variance] struct TestContraStruct> { //~ ERROR [*, +] - //~^ ERROR parameter `U` is never used t: T } #[rustc_variance] struct TestBox+Setter> { //~ ERROR [*, +] - //~^ ERROR parameter `U` is never used t: T } From 3819ccda9c841f859c3654ee25f132b8f0ebef9f Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Tue, 25 Apr 2017 05:46:29 -0400 Subject: [PATCH 06/10] allow tests to refer to `ItemVariances` --- src/librustc/dep_graph/dep_node.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/librustc/dep_graph/dep_node.rs b/src/librustc/dep_graph/dep_node.rs index b4920f909cbda..6e8b26b0a70e4 100644 --- a/src/librustc/dep_graph/dep_node.rs +++ b/src/librustc/dep_graph/dep_node.rs @@ -181,6 +181,7 @@ impl DepNode { TransCrateItem, AssociatedItems, ItemSignature, + ItemVariances, IsForeignItem, AssociatedItemDefIds, InherentImpls, From e8f6fbb25555bd7e4b0e6cbdc4f7dfbe96ebe16c Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Tue, 25 Apr 2017 09:08:21 -0400 Subject: [PATCH 07/10] allow dep-graph assertions on fields --- src/librustc_incremental/assert_dep_graph.rs | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/src/librustc_incremental/assert_dep_graph.rs b/src/librustc_incremental/assert_dep_graph.rs index 897ca0f295761..7905128bb6eca 100644 --- a/src/librustc_incremental/assert_dep_graph.rs +++ b/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); } } From 50df5f85d05fc1a7967905f230205c559c09cba2 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Tue, 2 May 2017 17:47:38 -0400 Subject: [PATCH 08/10] correct the new graphs resulting from various tests (Now that variances are not part of signature.) --- src/test/compile-fail/dep-graph-struct-signature.rs | 6 ++++-- src/test/compile-fail/dep-graph-type-alias.rs | 12 +++++++++--- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/src/test/compile-fail/dep-graph-struct-signature.rs b/src/test/compile-fail/dep-graph-struct-signature.rs index 7ed8b95f88b00..3f568194e23d8 100644 --- a/src/test/compile-fail/dep-graph-struct-signature.rs +++ b/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) { } } diff --git a/src/test/compile-fail/dep-graph-type-alias.rs b/src/test/compile-fail/dep-graph-type-alias.rs index 4cc15e8b522ac..56636a00a313a 100644 --- a/src/test/compile-fail/dep-graph-type-alias.rs +++ b/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) } From 79de56a14b320327e5077232809ed5f2067aeb40 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Wed, 3 May 2017 16:42:46 -0400 Subject: [PATCH 09/10] remove `pub` modifier (and last use thereof) --- src/librustc/ty/maps.rs | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) diff --git a/src/librustc/ty/maps.rs b/src/librustc/ty/maps.rs index 3a2a614734582..1452a46ac0187 100644 --- a/src/librustc/ty/maps.rs +++ b/src/librustc/ty/maps.rs @@ -541,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)*), @@ -685,7 +673,7 @@ define_maps! { <'tcx> /// Maps from def-id of a type or region parameter to its /// (inferred) variance. - [pub] variances_of: ItemVariances(DefId) -> Rc>, + [] variances_of: ItemVariances(DefId) -> Rc>, /// Maps from an impl/trait def-id to a list of the def-ids of its items [] associated_item_def_ids: AssociatedItemDefIds(DefId) -> Rc>, From 3da5daf42587c9cece98a7b0985215cc40c31d58 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Fri, 5 May 2017 14:34:42 -0400 Subject: [PATCH 10/10] change various uses of `item_variances` to `variances_of` --- src/librustc/ty/mod.rs | 2 +- src/librustc_metadata/encoder.rs | 6 +++--- src/librustc_typeck/variance/README.md | 6 +++--- src/librustc_typeck/variance/test.rs | 4 ++-- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/librustc/ty/mod.rs b/src/librustc/ty/mod.rs index 2355ed7e147b3..9ffab1acf3cde 100644 --- a/src/librustc/ty/mod.rs +++ b/src/librustc/ty/mod.rs @@ -313,7 +313,7 @@ pub enum Variance { /// 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.item_variances()` to get the variance for a *particular* +/// `tcx.variances_of()` to get the variance for a *particular* /// item. pub struct CrateVariancesMap { /// This relation tracks the dependencies between the variance of diff --git a/src/librustc_metadata/encoder.rs b/src/librustc_metadata/encoder.rs index 125026b799c98..796cb8c4d651d 100644 --- a/src/librustc_metadata/encoder.rs +++ b/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 { - debug!("EntryBuilder::encode_item_variances({:?})", def_id); + fn encode_variances_of(&mut self, def_id: DefId) -> LazySeq { + 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 { diff --git a/src/librustc_typeck/variance/README.md b/src/librustc_typeck/variance/README.md index 9ec20c1a45c34..592916178897c 100644 --- a/src/librustc_typeck/variance/README.md +++ b/src/librustc_typeck/variance/README.md @@ -102,15 +102,15 @@ 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. -- `item_variances` accesses the variance for an individual reading; it +- `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 `item_variances`, your code will only +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 -`item_variances` ignores the dependencies of accessing +`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 diff --git a/src/librustc_typeck/variance/test.rs b/src/librustc_typeck/variance/test.rs index 53f2bd37fbc61..1acadb7e77236 100644 --- a/src/librustc_typeck/variance/test.rs +++ b/src/librustc_typeck/variance/test.rs @@ -27,12 +27,12 @@ impl<'a, 'tcx> ItemLikeVisitor<'tcx> for VarianceTest<'a, 'tcx> { // 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 item_variances = self.tcx.variances_of(item_def_id); + let variances_of = self.tcx.variances_of(item_def_id); span_err!(self.tcx.sess, item.span, E0208, "{:?}", - item_variances); + variances_of); } }