Skip to content

Commit 1ff0441

Browse files
committed
Auto merge of #66821 - eddyb:global-trait-caching, r=<try>
rustc: allow non-empty ParamEnv's in global trait select/eval caches. *Based on #66963* This appears to alleviate the symptoms of #65510 locally (without fixing WF directly), and is potentially easier to validate as sound (since it's a more ad-hoc version of queries we already have). I'm opening this PR primarily to test the effects on perf. r? @nikomatsakis cc @rust-lang/wg-traits
2 parents 2da942f + a266ea0 commit 1ff0441

File tree

4 files changed

+36
-26
lines changed

4 files changed

+36
-26
lines changed

src/librustc/traits/project.rs

+4-6
Original file line numberDiff line numberDiff line change
@@ -1079,12 +1079,10 @@ fn assemble_candidates_from_impls<'cx, 'tcx>(
10791079
if !is_default {
10801080
true
10811081
} else if obligation.param_env.reveal == Reveal::All {
1082-
debug_assert!(!poly_trait_ref.needs_infer());
1083-
if !poly_trait_ref.needs_subst() {
1084-
true
1085-
} else {
1086-
false
1087-
}
1082+
// NOTE(eddyb) inference variables can resolve to parameters, so
1083+
// assume `poly_trait_ref` isn't monomorphic, if it contains any.
1084+
let poly_trait_ref = selcx.infcx().resolve_vars_if_possible(&poly_trait_ref);
1085+
!poly_trait_ref.needs_infer() && !poly_trait_ref.needs_subst()
10881086
} else {
10891087
false
10901088
}

src/librustc/traits/select.rs

+19-18
Original file line numberDiff line numberDiff line change
@@ -204,7 +204,10 @@ struct TraitObligationStack<'prev, 'tcx> {
204204
#[derive(Clone, Default)]
205205
pub struct SelectionCache<'tcx> {
206206
hashmap: Lock<
207-
FxHashMap<ty::TraitRef<'tcx>, WithDepNode<SelectionResult<'tcx, SelectionCandidate<'tcx>>>>,
207+
FxHashMap<
208+
ty::ParamEnvAnd<'tcx, ty::TraitRef<'tcx>>,
209+
WithDepNode<SelectionResult<'tcx, SelectionCandidate<'tcx>>>,
210+
>,
208211
>,
209212
}
210213

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

491494
#[derive(Clone, Default)]
492495
pub struct EvaluationCache<'tcx> {
493-
hashmap: Lock<FxHashMap<ty::PolyTraitRef<'tcx>, WithDepNode<EvaluationResult>>>,
496+
hashmap: Lock<
497+
FxHashMap<ty::ParamEnvAnd<'tcx, ty::PolyTraitRef<'tcx>>, WithDepNode<EvaluationResult>>,
498+
>,
494499
}
495500

496501
impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
@@ -1143,15 +1148,15 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
11431148
let tcx = self.tcx();
11441149
if self.can_use_global_caches(param_env) {
11451150
let cache = tcx.evaluation_cache.hashmap.borrow();
1146-
if let Some(cached) = cache.get(&trait_ref) {
1151+
if let Some(cached) = cache.get(&param_env.and(trait_ref)) {
11471152
return Some(cached.get(tcx));
11481153
}
11491154
}
11501155
self.infcx
11511156
.evaluation_cache
11521157
.hashmap
11531158
.borrow()
1154-
.get(&trait_ref)
1159+
.get(&param_env.and(trait_ref))
11551160
.map(|v| v.get(tcx))
11561161
}
11571162

@@ -1182,7 +1187,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
11821187
.evaluation_cache
11831188
.hashmap
11841189
.borrow_mut()
1185-
.insert(trait_ref, WithDepNode::new(dep_node, result));
1190+
.insert(param_env.and(trait_ref), WithDepNode::new(dep_node, result));
11861191
return;
11871192
}
11881193
}
@@ -1195,7 +1200,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
11951200
.evaluation_cache
11961201
.hashmap
11971202
.borrow_mut()
1198-
.insert(trait_ref, WithDepNode::new(dep_node, result));
1203+
.insert(param_env.and(trait_ref), WithDepNode::new(dep_node, result));
11991204
}
12001205

12011206
/// For various reasons, it's possible for a subobligation
@@ -1567,14 +1572,10 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
15671572
/// Do note that if the type itself is not in the
15681573
/// global tcx, the local caches will be used.
15691574
fn can_use_global_caches(&self, param_env: ty::ParamEnv<'tcx>) -> bool {
1570-
// If there are any where-clauses in scope, then we always use
1571-
// a cache local to this particular scope. Otherwise, we
1572-
// switch to a global cache. We used to try and draw
1573-
// finer-grained distinctions, but that led to a serious of
1574-
// annoying and weird bugs like #22019 and #18290. This simple
1575-
// rule seems to be pretty clearly safe and also still retains
1576-
// a very high hit rate (~95% when compiling rustc).
1577-
if !param_env.caller_bounds.is_empty() {
1575+
// If there are any e.g. inference variables in the `ParamEnv`, then we
1576+
// always use a cache local to this particular scope. Otherwise, we
1577+
// switch to a global cache.
1578+
if param_env.has_local_value() {
15781579
return false;
15791580
}
15801581

@@ -1602,15 +1603,15 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
16021603
let trait_ref = &cache_fresh_trait_pred.skip_binder().trait_ref;
16031604
if self.can_use_global_caches(param_env) {
16041605
let cache = tcx.selection_cache.hashmap.borrow();
1605-
if let Some(cached) = cache.get(&trait_ref) {
1606+
if let Some(cached) = cache.get(&param_env.and(*trait_ref)) {
16061607
return Some(cached.get(tcx));
16071608
}
16081609
}
16091610
self.infcx
16101611
.selection_cache
16111612
.hashmap
16121613
.borrow()
1613-
.get(trait_ref)
1614+
.get(&param_env.and(*trait_ref))
16141615
.map(|v| v.get(tcx))
16151616
}
16161617

@@ -1671,7 +1672,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
16711672
tcx.selection_cache
16721673
.hashmap
16731674
.borrow_mut()
1674-
.insert(trait_ref, WithDepNode::new(dep_node, candidate));
1675+
.insert(param_env.and(trait_ref), WithDepNode::new(dep_node, candidate));
16751676
return;
16761677
}
16771678
}
@@ -1685,7 +1686,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
16851686
.selection_cache
16861687
.hashmap
16871688
.borrow_mut()
1688-
.insert(trait_ref, WithDepNode::new(dep_node, candidate));
1689+
.insert(param_env.and(trait_ref), WithDepNode::new(dep_node, candidate));
16891690
}
16901691

16911692
fn assemble_candidates<'o>(

src/test/ui/type-alias-impl-trait/bound_reduction2.rs

+1
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ trait TraitWithAssoc {
99

1010
type Foo<V> = impl Trait<V>;
1111
//~^ ERROR could not find defining uses
12+
//~| ERROR the trait bound `T: TraitWithAssoc` is not satisfied
1213

1314
trait Trait<U> {}
1415

src/test/ui/type-alias-impl-trait/bound_reduction2.stderr

+12-2
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,14 @@
1+
error[E0277]: the trait bound `T: TraitWithAssoc` is not satisfied
2+
--> $DIR/bound_reduction2.rs:10:1
3+
|
4+
LL | type Foo<V> = impl Trait<V>;
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `TraitWithAssoc` is not implemented for `T`
6+
...
7+
LL | fn foo_desugared<T: TraitWithAssoc>(_: T) -> Foo<T::Assoc> {
8+
| -- help: consider further restricting this bound: `T: TraitWithAssoc +`
9+
110
error: defining opaque type use does not fully define opaque type: generic parameter `V` is specified as concrete type `<T as TraitWithAssoc>::Assoc`
2-
--> $DIR/bound_reduction2.rs:17:1
11+
--> $DIR/bound_reduction2.rs:18:1
312
|
413
LL | / fn foo_desugared<T: TraitWithAssoc>(_: T) -> Foo<T::Assoc> {
514
LL | | ()
@@ -12,5 +21,6 @@ error: could not find defining uses
1221
LL | type Foo<V> = impl Trait<V>;
1322
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
1423

15-
error: aborting due to 2 previous errors
24+
error: aborting due to 3 previous errors
1625

26+
For more information about this error, try `rustc --explain E0277`.

0 commit comments

Comments
 (0)