Skip to content

Commit 26e067b

Browse files
authored
Rollup merge of #41734 - nikomatsakis:incr-comp-refactor-variance, r=pnkfelix
Refactor variance and remove last `[pub]` map This PR refactors variance to work in a more red-green friendly way. Because red-green doesn't exist yet, it has to be a bit hacky. The basic idea is this: - We compute a big map with the variance for all items in the crate; when you request variances for a particular item, we read it from the crate - We now hard-code that traits are invariant (which they are, for deep reasons, not gonna' change) - When building constraints, we compute the transitive closure of all things within the crate that depend on what using `TransitiveRelation` - this lets us gin up the correct dependencies when requesting variance of a single item Ah damn, just remembered, one TODO: - [x] Update the variance README -- ah, I guess the README updates I did are sufficient r? @michaelwoerister
2 parents 9b2aacf + 3da5daf commit 26e067b

25 files changed

+472
-369
lines changed

src/librustc/dep_graph/dep_node.rs

+7
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,7 @@ pub enum DepNode<D: Clone + Debug> {
8181
TransCrateItem(D),
8282
TransInlinedItem(D),
8383
TransWriteMetadata,
84+
CrateVariances,
8485

8586
// Nodes representing bits of computed IR in the tcx. Each shared
8687
// table in the tcx (or elsewhere) maps to one of these
@@ -89,6 +90,8 @@ pub enum DepNode<D: Clone + Debug> {
8990
// predicates for an item wind up in `ItemSignature`).
9091
AssociatedItems(D),
9192
ItemSignature(D),
93+
ItemVarianceConstraints(D),
94+
ItemVariances(D),
9295
IsForeignItem(D),
9396
TypeParamPredicates((D, D)),
9497
SizedConstraint(D),
@@ -180,6 +183,7 @@ impl<D: Clone + Debug> DepNode<D> {
180183
TransCrateItem,
181184
AssociatedItems,
182185
ItemSignature,
186+
ItemVariances,
183187
IsForeignItem,
184188
AssociatedItemDefIds,
185189
InherentImpls,
@@ -201,6 +205,7 @@ impl<D: Clone + Debug> DepNode<D> {
201205
MirKrate => Some(MirKrate),
202206
TypeckBodiesKrate => Some(TypeckBodiesKrate),
203207
Coherence => Some(Coherence),
208+
CrateVariances => Some(CrateVariances),
204209
Resolve => Some(Resolve),
205210
Variance => Some(Variance),
206211
PrivacyAccessLevels(k) => Some(PrivacyAccessLevels(k)),
@@ -232,6 +237,8 @@ impl<D: Clone + Debug> DepNode<D> {
232237
TransInlinedItem(ref d) => op(d).map(TransInlinedItem),
233238
AssociatedItems(ref d) => op(d).map(AssociatedItems),
234239
ItemSignature(ref d) => op(d).map(ItemSignature),
240+
ItemVariances(ref d) => op(d).map(ItemVariances),
241+
ItemVarianceConstraints(ref d) => op(d).map(ItemVarianceConstraints),
235242
IsForeignItem(ref d) => op(d).map(IsForeignItem),
236243
TypeParamPredicates((ref item, ref param)) => {
237244
Some(TypeParamPredicates((try_opt!(op(item)), try_opt!(op(param)))))

src/librustc/dep_graph/mod.rs

-2
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ mod raii;
1818
mod safe;
1919
mod shadow;
2020
mod thread;
21-
mod visit;
2221

2322
pub use self::dep_tracking_map::{DepTrackingMap, DepTrackingMapConfig};
2423
pub use self::dep_node::DepNode;
@@ -28,5 +27,4 @@ pub use self::graph::WorkProduct;
2827
pub use self::query::DepGraphQuery;
2928
pub use self::safe::AssertDepGraphSafe;
3029
pub use self::safe::DepGraphSafe;
31-
pub use self::visit::visit_all_item_likes_in_krate;
3230
pub use self::raii::DepTask;

src/librustc/dep_graph/visit.rs

-77
This file was deleted.

src/librustc/hir/intravisit.rs

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

src/librustc/hir/itemlikevisit.rs

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

src/librustc/ty/context.rs

-4
Original file line numberDiff line numberDiff line change
@@ -470,9 +470,6 @@ pub struct GlobalCtxt<'tcx> {
470470

471471
pub lang_items: middle::lang_items::LanguageItems,
472472

473-
/// True if the variance has been computed yet; false otherwise.
474-
pub variance_computed: Cell<bool>,
475-
476473
/// Set of used unsafe nodes (functions or blocks). Unsafe nodes not
477474
/// present in this set can be warned about.
478475
pub used_unsafe: RefCell<NodeSet>,
@@ -744,7 +741,6 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
744741
dep_graph: dep_graph.clone(),
745742
types: common_types,
746743
named_region_map: named_region_map,
747-
variance_computed: Cell::new(false),
748744
trait_map: resolutions.trait_map,
749745
export_map: resolutions.export_map,
750746
fulfilled_predicates: RefCell::new(fulfilled_predicates),

src/librustc/ty/maps.rs

+15-13
Original file line numberDiff line numberDiff line change
@@ -266,6 +266,12 @@ impl<'tcx> QueryDescription for queries::crate_inherent_impls_overlap_check<'tcx
266266
}
267267
}
268268

269+
impl<'tcx> QueryDescription for queries::crate_variances<'tcx> {
270+
fn describe(_tcx: TyCtxt, _: CrateNum) -> String {
271+
format!("computing the variances for items in this crate")
272+
}
273+
}
274+
269275
impl<'tcx> QueryDescription for queries::mir_shims<'tcx> {
270276
fn describe(tcx: TyCtxt, def: ty::InstanceDef<'tcx>) -> String {
271277
format!("generating MIR shim for `{}`",
@@ -549,18 +555,6 @@ macro_rules! define_map_struct {
549555
}
550556
};
551557

552-
// Detect things with the `pub` modifier
553-
(tcx: $tcx:tt,
554-
input: (([pub $($other_modifiers:tt)*] $attrs:tt $name:tt) $($input:tt)*),
555-
output: $output:tt) => {
556-
define_map_struct! {
557-
tcx: $tcx,
558-
ready: ([pub] $attrs $name),
559-
input: ($($input)*),
560-
output: $output
561-
}
562-
};
563-
564558
// No modifiers left? This is a private item.
565559
(tcx: $tcx:tt,
566560
input: (([] $attrs:tt $name:tt) $($input:tt)*),
@@ -687,9 +681,13 @@ define_maps! { <'tcx>
687681
/// True if this is a foreign item (i.e., linked via `extern { ... }`).
688682
[] is_foreign_item: IsForeignItem(DefId) -> bool,
689683

684+
/// Get a map with the variance of every item; use `item_variance`
685+
/// instead.
686+
[] crate_variances: crate_variances(CrateNum) -> Rc<ty::CrateVariancesMap>,
687+
690688
/// Maps from def-id of a type or region parameter to its
691689
/// (inferred) variance.
692-
[pub] variances_of: ItemSignature(DefId) -> Rc<Vec<ty::Variance>>,
690+
[] variances_of: ItemVariances(DefId) -> Rc<Vec<ty::Variance>>,
693691

694692
/// Maps from an impl/trait def-id to a list of the def-ids of its items
695693
[] associated_item_def_ids: AssociatedItemDefIds(DefId) -> Rc<Vec<DefId>>,
@@ -825,3 +823,7 @@ fn const_eval_dep_node((def_id, _): (DefId, &Substs)) -> DepNode<DefId> {
825823
fn mir_keys(_: CrateNum) -> DepNode<DefId> {
826824
DepNode::MirKeys
827825
}
826+
827+
fn crate_variances(_: CrateNum) -> DepNode<DefId> {
828+
DepNode::CrateVariances
829+
}

src/librustc/ty/mod.rs

+23-10
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ pub use self::IntVarValue::*;
1515
pub use self::LvaluePreference::*;
1616
pub use self::fold::TypeFoldable;
1717

18-
use dep_graph::{self, DepNode};
18+
use dep_graph::DepNode;
1919
use hir::{map as hir_map, FreevarMap, TraitMap};
2020
use hir::def::{Def, CtorKind, ExportMap};
2121
use hir::def_id::{CrateNum, DefId, DefIndex, CRATE_DEF_INDEX, LOCAL_CRATE};
@@ -55,9 +55,9 @@ use rustc_const_math::ConstInt;
5555
use rustc_data_structures::accumulate_vec::IntoIter as AccIntoIter;
5656
use rustc_data_structures::stable_hasher::{StableHasher, StableHasherResult,
5757
HashStable};
58+
use rustc_data_structures::transitive_relation::TransitiveRelation;
5859

5960
use hir;
60-
use hir::itemlikevisit::ItemLikeVisitor;
6161

6262
pub use self::sty::{Binder, DebruijnIndex};
6363
pub use self::sty::{FnSig, PolyFnSig};
@@ -309,6 +309,27 @@ pub enum Variance {
309309
Bivariant, // T<A> <: T<B> -- e.g., unused type parameter
310310
}
311311

312+
/// The crate variances map is computed during typeck and contains the
313+
/// variance of every item in the local crate. You should not use it
314+
/// directly, because to do so will make your pass dependent on the
315+
/// HIR of every item in the local crate. Instead, use
316+
/// `tcx.variances_of()` to get the variance for a *particular*
317+
/// item.
318+
pub struct CrateVariancesMap {
319+
/// This relation tracks the dependencies between the variance of
320+
/// various items. In particular, if `a < b`, then the variance of
321+
/// `a` depends on the sources of `b`.
322+
pub dependencies: TransitiveRelation<DefId>,
323+
324+
/// For each item with generics, maps to a vector of the variance
325+
/// of its generics. If an item has no generics, it will have no
326+
/// entry.
327+
pub variances: FxHashMap<DefId, Rc<Vec<ty::Variance>>>,
328+
329+
/// An empty vector, useful for cloning.
330+
pub empty_variance: Rc<Vec<ty::Variance>>,
331+
}
332+
312333
#[derive(Clone, Copy, Debug, RustcDecodable, RustcEncodable)]
313334
pub struct MethodCallee<'tcx> {
314335
/// Impl method ID, for inherent methods, or trait method ID, otherwise.
@@ -2543,14 +2564,6 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
25432564
self.mk_region(ty::ReScope(self.node_extent(id)))
25442565
}
25452566

2546-
pub fn visit_all_item_likes_in_krate<V,F>(self,
2547-
dep_node_fn: F,
2548-
visitor: &mut V)
2549-
where F: FnMut(DefId) -> DepNode<DefId>, V: ItemLikeVisitor<'gcx>
2550-
{
2551-
dep_graph::visit_all_item_likes_in_krate(self.global_tcx(), dep_node_fn, visitor);
2552-
}
2553-
25542567
/// Looks up the span of `impl_did` if the impl is local; otherwise returns `Err`
25552568
/// with the name of the crate containing the impl.
25562569
pub fn span_of_impl(self, impl_did: DefId) -> Result<Span, Symbol> {

src/librustc/ty/relate.rs

+2-8
Original file line numberDiff line numberDiff line change
@@ -124,14 +124,8 @@ fn relate_item_substs<'a, 'gcx, 'tcx, R>(relation: &mut R,
124124
a_subst,
125125
b_subst);
126126

127-
let variances;
128-
let opt_variances = if relation.tcx().variance_computed.get() {
129-
variances = relation.tcx().variances_of(item_def_id);
130-
Some(&*variances)
131-
} else {
132-
None
133-
};
134-
relate_substs(relation, opt_variances, a_subst, b_subst)
127+
let opt_variances = relation.tcx().variances_of(item_def_id);
128+
relate_substs(relation, Some(&opt_variances), a_subst, b_subst)
135129
}
136130

137131
pub fn relate_substs<'a, 'gcx, 'tcx, R>(relation: &mut R,

0 commit comments

Comments
 (0)