Skip to content

Commit

Permalink
Use a fresh InferCtxt when we 'speculatively' evaluate predicates
Browse files Browse the repository at this point in the history
The existing `InferCtxt` may already have in-progress projections
for some of the predicates we may (recursively evaluate). This can
cause us to incorrect produce (and cache!) `EvaluatedToRecur`, leading
us to incorrectly mark a predicate as unimplemented.

We now create a fresh `InferCtxt` for each sub-obligation that we
'speculatively' evaluate. This causes us to miss out on some
legitimate caching opportunities, but ensures that our speculative
evaluation cannot pollute any of the caches from the 'real' `InferCtxt`.
The evaluation can still update *global* caches, but our global caches
don't have any notion of 'in progress', so this is fine.

Fixes #90662
  • Loading branch information
Aaron1011 committed Nov 26, 2021
1 parent 1e79d79 commit 04964d7
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 4 deletions.
20 changes: 16 additions & 4 deletions compiler/rustc_trait_selection/src/traits/project.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ use super::{
ImplSourceGeneratorData, ImplSourcePointeeData, ImplSourceUserDefinedData,
};
use super::{Normalized, NormalizedTy, ProjectionCacheEntry, ProjectionCacheKey};
use rustc_infer::infer::TyCtxtInferExt;

use crate::infer::type_variable::{TypeVariableOrigin, TypeVariableOriginKind};
use crate::infer::{InferCtxt, InferOk, LateBoundRegionConversionTime};
Expand Down Expand Up @@ -944,8 +945,8 @@ fn opt_normalize_projection_type<'a, 'b, 'tcx>(
Normalized { value: projected_ty, obligations: projected_obligations }
};

let mut canonical =
SelectionContext::with_query_mode(selcx.infcx(), TraitQueryMode::Canonical);
let tcx = selcx.infcx().tcx;

result.obligations.drain_filter(|projected_obligation| {
// If any global obligations always apply, considering regions, then we don't
// need to include them. The `is_global` check rules out inference variables,
Expand All @@ -957,10 +958,21 @@ fn opt_normalize_projection_type<'a, 'b, 'tcx>(
// the result to `EvaluatedToOkModuloRegions`, while an
// `EvaluatedToOk` obligation will never change the result.
// See #85360 for more details
projected_obligation.is_global(canonical.tcx())
&& canonical
if !projected_obligation.is_global(tcx) {
return false;
}

// We create a fresh `InferCtxt` for each predicate we speculatively evaluate,
// so that we won't create (and cache) any spurious projection cycles in the main
// `InferCtxt`
tcx.infer_ctxt().enter(|infcx| {
let mut canonical =
SelectionContext::with_query_mode(&infcx, TraitQueryMode::Canonical);

canonical
.evaluate_root_obligation(projected_obligation)
.map_or(false, |res| res.must_apply_considering_regions())
})
});

if use_cache {
Expand Down
34 changes: 34 additions & 0 deletions src/test/ui/traits/issue-90662-projection-caching.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
// check-pass

// Regression test for issue #90662
// Tests that projection caching does not cause a spurious error

trait HasProvider<T: ?Sized> {}
trait Provider<M> {
type Interface: ?Sized;
}

trait Repository {}
trait Service {}

struct DbConnection;
impl<M> Provider<M> for DbConnection {
type Interface = DbConnection;
}

struct RepositoryImpl;
impl<M: HasProvider<DbConnection>> Provider<M> for RepositoryImpl {
type Interface = dyn Repository;
}

struct ServiceImpl;
impl<M: HasProvider<dyn Repository>> Provider<M> for ServiceImpl {
type Interface = dyn Service;
}

struct TestModule;
impl HasProvider<<DbConnection as Provider<Self>>::Interface> for TestModule {}
impl HasProvider<<RepositoryImpl as Provider<Self>>::Interface> for TestModule {}
impl HasProvider<<ServiceImpl as Provider<Self>>::Interface> for TestModule {}

fn main() {}

0 comments on commit 04964d7

Please sign in to comment.