Skip to content

Commit

Permalink
Auto merge of #92041 - Aaron1011:remove-speculative-evaluation, r=jac…
Browse files Browse the repository at this point in the history
…kh726

Remove 'speculative evaluation' of predicates

Performing 'speculative evaluation' introduces caching bugs that
cannot be fixed without invasive changes to projection.

Hopefully, we can win back most of the performance lost by
re-adding 'cache completion'

Fixes #90662
  • Loading branch information
bors committed Dec 20, 2021
2 parents 940a97a + eee09ec commit 60f3bd7
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 18 deletions.
19 changes: 1 addition & 18 deletions compiler/rustc_trait_selection/src/traits/project.rs
Expand Up @@ -10,7 +10,6 @@ use super::PredicateObligation;
use super::Selection;
use super::SelectionContext;
use super::SelectionError;
use super::TraitQueryMode;
use super::{
ImplSourceClosureData, ImplSourceDiscriminantKindData, ImplSourceFnPointerData,
ImplSourceGeneratorData, ImplSourcePointeeData, ImplSourceUserDefinedData,
Expand Down Expand Up @@ -946,27 +945,11 @@ fn opt_normalize_projection_type<'a, 'b, 'tcx>(
};

let mut deduped: SsoHashSet<_> = Default::default();
let mut canonical =
SelectionContext::with_query_mode(selcx.infcx(), TraitQueryMode::Canonical);

result.obligations.drain_filter(|projected_obligation| {
if !deduped.insert(projected_obligation.clone()) {
return true;
}
// If any global obligations always apply, considering regions, then we don't
// need to include them. The `is_global` check rules out inference variables,
// so there's no need for the caller of `opt_normalize_projection_type`
// to evaluate them.
// Note that we do *not* discard obligations that evaluate to
// `EvaluatedtoOkModuloRegions`. Evaluating these obligations
// inside of a query (e.g. `evaluate_obligation`) can change
// 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
.evaluate_root_obligation(projected_obligation)
.map_or(false, |res| res.must_apply_considering_regions())
false
});

if use_cache {
Expand Down
34 changes: 34 additions & 0 deletions src/test/ui/traits/issue-90662-projection-caching.rs
@@ -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 60f3bd7

Please sign in to comment.