Skip to content

Commit

Permalink
Auto merge of #66838 - eddyb:wf-skip-plus-global-caching, r=<try>
Browse files Browse the repository at this point in the history 
[WIP] [DO NOT MERGE] combine #66020 and #66821.

That is, the two fixes for #65510, and only for perf testing purposes.
The fact that they both work to a comparable extent, while touching different parts of the trait system, made me curious if there would be any gains from having both.

r? @nikomatsakis
  • Loading branch information
@bors
bors committed Nov 28, 2019
2 parents 2539b5f + ba8f8f8 commit 685e48b
Show file tree
Hide file tree
Showing 4 changed files with 53 additions and 110 deletions.
27 changes: 16 additions & 11 deletions src/librustc/traits/select.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,10 @@ struct TraitObligationStack<'prev, 'tcx> {
#[derive(Clone, Default)]
pub struct SelectionCache<'tcx> {
hashmap: Lock<
FxHashMap<ty::TraitRef<'tcx>, WithDepNode<SelectionResult<'tcx, SelectionCandidate<'tcx>>>>,
FxHashMap<
ty::ParamEnvAnd<'tcx, ty::TraitRef<'tcx>>,
WithDepNode<SelectionResult<'tcx, SelectionCandidate<'tcx>>>,
>,
>,
}

Expand Down Expand Up @@ -490,7 +493,9 @@ impl<'tcx> From<OverflowError> for SelectionError<'tcx> {

#[derive(Clone, Default)]
pub struct EvaluationCache<'tcx> {
hashmap: Lock<FxHashMap<ty::PolyTraitRef<'tcx>, WithDepNode<EvaluationResult>>>,
hashmap: Lock<
FxHashMap<ty::ParamEnvAnd<'tcx, ty::PolyTraitRef<'tcx>>, WithDepNode<EvaluationResult>>,
>,
}

impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
Expand Down Expand Up @@ -1143,15 +1148,15 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
let tcx = self.tcx();
if self.can_use_global_caches(param_env) {
let cache = tcx.evaluation_cache.hashmap.borrow();
if let Some(cached) = cache.get(&trait_ref) {
if let Some(cached) = cache.get(&param_env.and(trait_ref)) {
return Some(cached.get(tcx));
}
}
self.infcx
.evaluation_cache
.hashmap
.borrow()
.get(&trait_ref)
.get(&param_env.and(trait_ref))
.map(|v| v.get(tcx))
}

Expand Down Expand Up @@ -1182,7 +1187,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
.evaluation_cache
.hashmap
.borrow_mut()
.insert(trait_ref, WithDepNode::new(dep_node, result));
.insert(param_env.and(trait_ref), WithDepNode::new(dep_node, result));
return;
}
}
Expand All @@ -1195,7 +1200,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
.evaluation_cache
.hashmap
.borrow_mut()
.insert(trait_ref, WithDepNode::new(dep_node, result));
.insert(param_env.and(trait_ref), WithDepNode::new(dep_node, result));
}

/// For various reasons, it's possible for a subobligation
Expand Down Expand Up @@ -1575,7 +1580,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
// rule seems to be pretty clearly safe and also still retains
// a very high hit rate (~95% when compiling rustc).
if !param_env.caller_bounds.is_empty() {
return false;
//return false;
}

// Avoid using the master cache during coherence and just rely
Expand All @@ -1602,15 +1607,15 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
let trait_ref = &cache_fresh_trait_pred.skip_binder().trait_ref;
if self.can_use_global_caches(param_env) {
let cache = tcx.selection_cache.hashmap.borrow();
if let Some(cached) = cache.get(&trait_ref) {
if let Some(cached) = cache.get(&param_env.and(*trait_ref)) {
return Some(cached.get(tcx));
}
}
self.infcx
.selection_cache
.hashmap
.borrow()
.get(trait_ref)
.get(&param_env.and(*trait_ref))
.map(|v| v.get(tcx))
}

Expand Down Expand Up @@ -1671,7 +1676,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
tcx.selection_cache
.hashmap
.borrow_mut()
.insert(trait_ref, WithDepNode::new(dep_node, candidate));
.insert(param_env.and(trait_ref), WithDepNode::new(dep_node, candidate));
return;
}
}
Expand All @@ -1685,7 +1690,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
.selection_cache
.hashmap
.borrow_mut()
.insert(trait_ref, WithDepNode::new(dep_node, candidate));
.insert(param_env.and(trait_ref), WithDepNode::new(dep_node, candidate));
}

fn assemble_candidates<'o>(
Expand Down
58 changes: 33 additions & 25 deletions src/librustc/ty/wf.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ pub fn trait_obligations<'a, 'tcx>(
item: Option<&'tcx hir::Item>,
) -> Vec<traits::PredicateObligation<'tcx>> {
let mut wf = WfPredicates { infcx, param_env, body_id, span, out: vec![], item };
wf.compute_trait_ref(trait_ref, Elaborate::All);
wf.compute_trait_ref(trait_ref, Elaborate::All, false);
wf.normalize()
}

Expand All @@ -69,7 +69,7 @@ pub fn predicate_obligations<'a, 'tcx>(
// (*) ok to skip binders, because wf code is prepared for it
match *predicate {
ty::Predicate::Trait(ref t) => {
wf.compute_trait_ref(&t.skip_binder().trait_ref, Elaborate::None); // (*)
wf.compute_trait_ref(&t.skip_binder().trait_ref, Elaborate::None, false); // (*)
}
ty::Predicate::RegionOutlives(..) => {
}
Expand Down Expand Up @@ -163,14 +163,18 @@ impl<'a, 'tcx> WfPredicates<'a, 'tcx> {
}

/// Pushes the obligations required for `trait_ref` to be WF into `self.out`.
fn compute_trait_ref(&mut self, trait_ref: &ty::TraitRef<'tcx>, elaborate: Elaborate) {
fn compute_trait_ref(
&mut self,
trait_ref: &ty::TraitRef<'tcx>,
elaborate: Elaborate,
is_proj: bool,
) {
let tcx = self.infcx.tcx;
let obligations = self.nominal_obligations(trait_ref.def_id, trait_ref.substs);

let cause = self.cause(traits::MiscObligation);
let param_env = self.param_env;

let item = &self.item;
let item = self.item;
let extend_cause_with_original_assoc_item_obligation = |
cause: &mut traits::ObligationCause<'_>,
pred: &ty::Predicate<'_>,
Expand Down Expand Up @@ -313,26 +317,30 @@ impl<'a, 'tcx> WfPredicates<'a, 'tcx> {
}
};

if let Elaborate::All = elaborate {
let trait_assoc_items = tcx.associated_items(trait_ref.def_id);

let predicates = obligations.iter()
.map(|obligation| obligation.predicate.clone())
.collect();
let implied_obligations = traits::elaborate_predicates(tcx, predicates);
let implied_obligations = implied_obligations.map(|pred| {
let mut cause = cause.clone();
extend_cause_with_original_assoc_item_obligation(
&mut cause,
&pred,
trait_assoc_items.clone(),
);
traits::Obligation::new(cause, param_env, pred)
});
self.out.extend(implied_obligations);
}
if !is_proj {
let obligations = self.nominal_obligations(trait_ref.def_id, trait_ref.substs);

if let Elaborate::All = elaborate {
let trait_assoc_items = tcx.associated_items(trait_ref.def_id);

let predicates = obligations.iter()
.map(|obligation| obligation.predicate.clone())
.collect();
let implied_obligations = traits::elaborate_predicates(tcx, predicates);
let implied_obligations = implied_obligations.map(|pred| {
let mut cause = cause.clone();
extend_cause_with_original_assoc_item_obligation(
&mut cause,
&pred,
trait_assoc_items.clone(),
);
traits::Obligation::new(cause, param_env, pred)
});
self.out.extend(implied_obligations);
}

self.out.extend(obligations);
self.out.extend(obligations);
}

self.out.extend(trait_ref.substs.types()
.filter(|ty| !ty.has_escaping_bound_vars())
Expand All @@ -350,7 +358,7 @@ impl<'a, 'tcx> WfPredicates<'a, 'tcx> {
// WF and (b) the trait-ref holds. (It may also be
// normalizable and be WF that way.)
let trait_ref = data.trait_ref(self.infcx.tcx);
self.compute_trait_ref(&trait_ref, Elaborate::None);
self.compute_trait_ref(&trait_ref, Elaborate::None, true);

if !data.has_escaping_bound_vars() {
let predicate = trait_ref.to_predicate();
Expand Down
11 changes: 2 additions & 9 deletions src/test/ui/associated-types/associated-types-overridden-binding.stderr
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,6 @@ LL | trait Foo: Iterator<Item = i32> {}
LL | trait Bar: Foo<Item = u32> {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error[E0282]: type annotations needed
--> $DIR/associated-types-overridden-binding.rs:7:1
|
LL | trait U32Iterator = I32Iterator<Item = u32>;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ cannot infer type

error: aborting due to 2 previous errors
error: aborting due to previous error

Some errors have detailed explanations: E0282, E0284.
For more information about an error, try `rustc --explain E0282`.
For more information about this error, try `rustc --explain E0284`.
67 changes: 2 additions & 65 deletions src/test/ui/issues/issue-20831-debruijn.stderr
View file
Original file line number Diff line number Diff line change
@@ -1,65 +1,3 @@
error[E0308]: mismatched types
--> $DIR/issue-20831-debruijn.rs:28:5
|
LL | / fn subscribe(&mut self, t : Box<dyn Subscriber<Input=<Self as Publisher>::Output> + 'a>) {
LL | | // Not obvious, but there is an implicit lifetime here -------^
LL | |
LL | |
... |
LL | | self.sub = t;
LL | | }
| |_____^ lifetime mismatch
|
= note: expected type `'a`
found type `'_`
note: the anonymous lifetime #2 defined on the method body at 28:5...
--> $DIR/issue-20831-debruijn.rs:28:5
|
LL | / fn subscribe(&mut self, t : Box<dyn Subscriber<Input=<Self as Publisher>::Output> + 'a>) {
LL | | // Not obvious, but there is an implicit lifetime here -------^
LL | |
LL | |
... |
LL | | self.sub = t;
LL | | }
| |_____^
note: ...does not necessarily outlive the lifetime `'a` as defined on the impl at 26:6
--> $DIR/issue-20831-debruijn.rs:26:6
|
LL | impl<'a> Publisher<'a> for MyStruct<'a> {
| ^^

error[E0308]: mismatched types
--> $DIR/issue-20831-debruijn.rs:28:5
|
LL | / fn subscribe(&mut self, t : Box<dyn Subscriber<Input=<Self as Publisher>::Output> + 'a>) {
LL | | // Not obvious, but there is an implicit lifetime here -------^
LL | |
LL | |
... |
LL | | self.sub = t;
LL | | }
| |_____^ lifetime mismatch
|
= note: expected type `'a`
found type `'_`
note: the lifetime `'a` as defined on the impl at 26:6...
--> $DIR/issue-20831-debruijn.rs:26:6
|
LL | impl<'a> Publisher<'a> for MyStruct<'a> {
| ^^
note: ...does not necessarily outlive the anonymous lifetime #2 defined on the method body at 28:5
--> $DIR/issue-20831-debruijn.rs:28:5
|
LL | / fn subscribe(&mut self, t : Box<dyn Subscriber<Input=<Self as Publisher>::Output> + 'a>) {
LL | | // Not obvious, but there is an implicit lifetime here -------^
LL | |
LL | |
... |
LL | | self.sub = t;
LL | | }
| |_____^

error[E0495]: cannot infer an appropriate lifetime for lifetime parameter `'a` due to conflicting requirements
--> $DIR/issue-20831-debruijn.rs:28:5
|
Expand Down Expand Up @@ -92,7 +30,6 @@ LL | impl<'a> Publisher<'a> for MyStruct<'a> {
expected Publisher<'_>
found Publisher<'_>

error: aborting due to 3 previous errors
error: aborting due to previous error

Some errors have detailed explanations: E0308, E0495.
For more information about an error, try `rustc --explain E0308`.
For more information about this error, try `rustc --explain E0495`.

0 comments on commit 685e48b

Please sign in to comment.