Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix normalizing in different ParamEnvs with the same InferCtxt #124203

Merged
merged 4 commits into from
Apr 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions compiler/rustc_hir_analysis/src/collect/item_bounds.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use rustc_data_structures::fx::FxIndexSet;
use rustc_hir as hir;
use rustc_infer::traits::util;
use rustc_middle::ty::GenericArgs;
use rustc_middle::ty::{self, Ty, TyCtxt, TypeFoldable, TypeFolder};
use rustc_middle::ty::{self, Ty, TyCtxt, TypeFoldable, TypeFolder, TypeSuperFoldable};
use rustc_span::def_id::{DefId, LocalDefId};
use rustc_span::Span;

Expand Down Expand Up @@ -214,7 +214,7 @@ impl<'tcx> TypeFolder<TyCtxt<'tcx>> for AssocTyToOpaque<'tcx> {
{
self.tcx.type_of(projection_ty.def_id).instantiate(self.tcx, projection_ty.args)
} else {
ty
ty.super_fold_with(self)
}
}
}
9 changes: 0 additions & 9 deletions compiler/rustc_infer/src/infer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1521,15 +1521,6 @@ impl<'tcx> InferCtxt<'tcx> {
closure_kind_ty.to_opt_closure_kind()
}

/// Clears the selection, evaluation, and projection caches. This is useful when
/// repeatedly attempting to select an `Obligation` while changing only
/// its `ParamEnv`, since `FulfillmentContext` doesn't use probing.
pub fn clear_caches(&self) {
self.selection_cache.clear();
self.evaluation_cache.clear();
self.inner.borrow_mut().projection_cache().clear();
}

pub fn universe(&self) -> ty::UniverseIndex {
self.universe.get()
}
Expand Down
5 changes: 3 additions & 2 deletions compiler/rustc_infer/src/traits/project.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,11 +78,12 @@ pub struct ProjectionCacheStorage<'tcx> {
#[derive(Copy, Clone, Debug, Hash, PartialEq, Eq)]
pub struct ProjectionCacheKey<'tcx> {
ty: ty::AliasTy<'tcx>,
param_env: ty::ParamEnv<'tcx>,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

won't that lead to tons of cache misses?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Almost all code that isn't the AutoTraitFinder will always use the same ParamEnv for a single InferCtxt, so I don't expect there to be many cache misses if any at all. But maybe start a perf run just to confirm? (I don't have permission)

}

impl<'tcx> ProjectionCacheKey<'tcx> {
pub fn new(ty: ty::AliasTy<'tcx>) -> Self {
Self { ty }
pub fn new(ty: ty::AliasTy<'tcx>, param_env: ty::ParamEnv<'tcx>) -> Self {
Self { ty, param_env }
}
}

Expand Down
5 changes: 0 additions & 5 deletions compiler/rustc_trait_selection/src/traits/auto_trait.rs
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,6 @@ impl<'tcx> AutoTraitFinder<'tcx> {
with {:?}",
trait_ref, full_env
);
infcx.clear_caches();

// At this point, we already have all of the bounds we need. FulfillmentContext is used
// to store all of the necessary region/lifetime bounds in the InferContext, as well as
Expand All @@ -176,9 +175,7 @@ impl<'tcx> AutoTraitFinder<'tcx> {

AutoTraitResult::PositiveImpl(auto_trait_callback(info))
}
}

impl<'tcx> AutoTraitFinder<'tcx> {
/// The core logic responsible for computing the bounds for our synthesized impl.
///
/// To calculate the bounds, we call `SelectionContext.select` in a loop. Like
Expand Down Expand Up @@ -255,8 +252,6 @@ impl<'tcx> AutoTraitFinder<'tcx> {
let dummy_cause = ObligationCause::dummy();

while let Some(pred) = predicates.pop_front() {
infcx.clear_caches();

if !already_visited.insert(pred) {
continue;
}
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_trait_selection/src/traits/fulfill.rs
Original file line number Diff line number Diff line change
Expand Up @@ -758,9 +758,9 @@ impl<'a, 'tcx> FulfillProcessor<'a, 'tcx> {
// no type variables present, can use evaluation for better caching.
// FIXME: consider caching errors too.
if self.selcx.infcx.predicate_must_hold_considering_regions(obligation) {
if let Some(key) = ProjectionCacheKey::from_poly_projection_predicate(
if let Some(key) = ProjectionCacheKey::from_poly_projection_obligation(
&mut self.selcx,
project_obligation.predicate,
&project_obligation,
) {
// If `predicate_must_hold_considering_regions` succeeds, then we've
// evaluated all sub-obligations. We can therefore mark the 'root'
Expand Down
13 changes: 7 additions & 6 deletions compiler/rustc_trait_selection/src/traits/project.rs
Original file line number Diff line number Diff line change
Expand Up @@ -344,7 +344,7 @@ pub(super) fn opt_normalize_projection_type<'a, 'b, 'tcx>(
let use_cache = !selcx.is_intercrate();

let projection_ty = infcx.resolve_vars_if_possible(projection_ty);
let cache_key = ProjectionCacheKey::new(projection_ty);
let cache_key = ProjectionCacheKey::new(projection_ty, param_env);

// FIXME(#20304) For now, I am caching here, which is good, but it
// means we don't capture the type variables that are created in
Expand Down Expand Up @@ -2105,27 +2105,28 @@ fn assoc_ty_own_obligations<'cx, 'tcx>(
}

pub(crate) trait ProjectionCacheKeyExt<'cx, 'tcx>: Sized {
fn from_poly_projection_predicate(
fn from_poly_projection_obligation(
selcx: &mut SelectionContext<'cx, 'tcx>,
predicate: ty::PolyProjectionPredicate<'tcx>,
obligation: &PolyProjectionObligation<'tcx>,
) -> Option<Self>;
}

impl<'cx, 'tcx> ProjectionCacheKeyExt<'cx, 'tcx> for ProjectionCacheKey<'tcx> {
fn from_poly_projection_predicate(
fn from_poly_projection_obligation(
selcx: &mut SelectionContext<'cx, 'tcx>,
predicate: ty::PolyProjectionPredicate<'tcx>,
obligation: &PolyProjectionObligation<'tcx>,
) -> Option<Self> {
let infcx = selcx.infcx;
// We don't do cross-snapshot caching of obligations with escaping regions,
// so there's no cache key to use
predicate.no_bound_vars().map(|predicate| {
obligation.predicate.no_bound_vars().map(|predicate| {
ProjectionCacheKey::new(
// We don't attempt to match up with a specific type-variable state
// from a specific call to `opt_normalize_projection_type` - if
// there's no precise match, the original cache entry is "stranded"
// anyway.
infcx.resolve_vars_if_possible(predicate.projection_ty),
obligation.param_env,
)
})
}
Expand Down
10 changes: 7 additions & 3 deletions compiler/rustc_trait_selection/src/traits/select/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -815,7 +815,10 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
// `EvaluatedToOkModuloRegions`), and skip re-evaluating the
// sub-obligations.
if let Some(key) =
ProjectionCacheKey::from_poly_projection_predicate(self, data)
ProjectionCacheKey::from_poly_projection_obligation(
self,
&project_obligation,
)
{
if let Some(cached_res) = self
.infcx
Expand Down Expand Up @@ -844,8 +847,9 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
&& (eval_rslt == EvaluatedToOk
|| eval_rslt == EvaluatedToOkModuloRegions)
&& let Some(key) =
ProjectionCacheKey::from_poly_projection_predicate(
self, data,
ProjectionCacheKey::from_poly_projection_obligation(
self,
&project_obligation,
)
{
// If the result is something that we can cache, then mark this
Expand Down
4 changes: 4 additions & 0 deletions tests/ui/impl-trait/in-trait/nested-rpitit-bounds.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@ trait Foo {
fn foo() -> impl Deref<Target = impl Deref<Target = impl Sized>> {
&&()
}

fn bar() -> impl Deref<Target = Option<impl Sized>> {
&Some(())
}
}

fn main() {}
Loading