Skip to content

Gracefully handle overflow errors in impl rematching #122539

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

Closed
wants to merge 3 commits into from
Closed
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
5 changes: 1 addition & 4 deletions compiler/rustc_middle/src/traits/select.rs
Original file line number Diff line number Diff line change
Expand Up @@ -324,9 +324,6 @@ TrivialTypeTraversalImpls! { OverflowError }

impl<'tcx> From<OverflowError> for SelectionError<'tcx> {
fn from(overflow_error: OverflowError) -> SelectionError<'tcx> {
match overflow_error {
OverflowError::Error(e) => SelectionError::Overflow(OverflowError::Error(e)),
OverflowError::Canonical => SelectionError::Overflow(OverflowError::Canonical),
}
SelectionError::Overflow(overflow_error)
}
}
71 changes: 59 additions & 12 deletions compiler/rustc_trait_selection/src/traits/normalize.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
//! Deeply normalize types using the old trait solver.
use crate::traits::project::ProjectionNormalizationFailure;

use super::error_reporting::OverflowCause;
use super::error_reporting::TypeErrCtxtExt;
use super::SelectionContext;
use super::{project, with_replaced_escaping_bound_vars, BoundVarReplacer, PlaceholderReplacer};
use rustc_data_structures::stack::ensure_sufficient_stack;
use rustc_infer::infer::at::At;
use rustc_infer::infer::InferOk;
use rustc_infer::traits::OverflowError;
use rustc_infer::traits::PredicateObligation;
use rustc_infer::traits::{FulfillmentError, Normalized, Obligation, TraitEngine};
use rustc_middle::traits::{ObligationCause, ObligationCauseCode, Reveal};
Expand Down Expand Up @@ -97,6 +100,27 @@ where
result
}

#[instrument(level = "info", skip(selcx, param_env, cause, obligations))]
pub(crate) fn try_normalize_with_depth_to<'a, 'b, 'tcx, T>(
selcx: &'a mut SelectionContext<'b, 'tcx>,
param_env: ty::ParamEnv<'tcx>,
cause: ObligationCause<'tcx>,
depth: usize,
value: T,
obligations: &mut Vec<PredicateObligation<'tcx>>,
) -> Result<T, OverflowError>
where
T: TypeFoldable<TyCtxt<'tcx>>,
{
debug!(obligations.len = obligations.len());
let mut normalizer = AssocTypeNormalizer::new(selcx, param_env, cause, depth, obligations);
let result = ensure_sufficient_stack(|| normalizer.fold(value));
debug!(?result, obligations.len = normalizer.obligations.len());
debug!(?normalizer.obligations,);
normalizer.overflowed?;
Ok(result)
}

pub(super) fn needs_normalization<'tcx, T: TypeVisitable<TyCtxt<'tcx>>>(
value: &T,
reveal: Reveal,
Expand All @@ -121,6 +145,11 @@ struct AssocTypeNormalizer<'a, 'b, 'tcx> {
obligations: &'a mut Vec<PredicateObligation<'tcx>>,
depth: usize,
universes: Vec<Option<ty::UniverseIndex>>,
/// Signals that there was an overflow during normalization somewhere.
/// We use this side channel instead of a `FallibleTypeFolder`, because
/// most code expects to get their value back on error, and we'd have to
/// clone the original value before normalization to achieve this.
overflowed: Result<(), OverflowError>,
}

impl<'a, 'b, 'tcx> AssocTypeNormalizer<'a, 'b, 'tcx> {
Expand All @@ -132,7 +161,15 @@ impl<'a, 'b, 'tcx> AssocTypeNormalizer<'a, 'b, 'tcx> {
obligations: &'a mut Vec<PredicateObligation<'tcx>>,
) -> AssocTypeNormalizer<'a, 'b, 'tcx> {
debug_assert!(!selcx.infcx.next_trait_solver());
AssocTypeNormalizer { selcx, param_env, cause, obligations, depth, universes: vec![] }
AssocTypeNormalizer {
selcx,
param_env,
cause,
obligations,
depth,
universes: vec![],
overflowed: Ok(()),
}
}

fn fold<T: TypeFoldable<TyCtxt<'tcx>>>(&mut self, value: T) -> T {
Expand Down Expand Up @@ -243,14 +280,21 @@ impl<'a, 'b, 'tcx> TypeFolder<TyCtxt<'tcx>> for AssocTypeNormalizer<'a, 'b, 'tcx
self.depth,
self.obligations,
);
let normalized_ty = match normalized_ty {
Ok(term) => term.ty().unwrap(),
Err(oflo) => {
self.overflowed = Err(oflo);
return ty;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

slight behaviour change, before this PR the normalize_projection_type would replace the entire projection with an inference variable and set up the correct obligations. I can replicate the previous behaviour, but afaict we're in a doomed normalization anyway.

}
};
debug!(
?self.depth,
?ty,
?normalized_ty,
obligations.len = ?self.obligations.len(),
"AssocTypeNormalizer: normalized type"
);
normalized_ty.ty().unwrap()
normalized_ty
}

ty::Projection => {
Expand All @@ -277,21 +321,24 @@ impl<'a, 'b, 'tcx> TypeFolder<TyCtxt<'tcx>> for AssocTypeNormalizer<'a, 'b, 'tcx
self.cause.clone(),
self.depth,
self.obligations,
)
.ok()
.flatten()
.map(|term| term.ty().unwrap())
.map(|normalized_ty| {
PlaceholderReplacer::replace_placeholders(
);
let normalized_ty = match normalized_ty {
Ok(Some(term)) => PlaceholderReplacer::replace_placeholders(
infcx,
mapped_regions,
mapped_types,
mapped_consts,
&self.universes,
normalized_ty,
)
})
.unwrap_or_else(|| ty.super_fold_with(self));
term.ty().unwrap(),
),
Ok(None) | Err(ProjectionNormalizationFailure::InProgress) => {
ty.super_fold_with(self)
}
Err(ProjectionNormalizationFailure::Overflow(oflo)) => {
self.overflowed = Err(oflo);
return ty;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

we don't even bother to recurse with super_fold_with, could probably do that, as it's what callers expect.

}
};

debug!(
?self.depth,
Expand Down
57 changes: 39 additions & 18 deletions compiler/rustc_trait_selection/src/traits/project.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,14 @@ pub type ProjectionObligation<'tcx> = Obligation<'tcx, ty::ProjectionPredicate<'

pub type ProjectionTyObligation<'tcx> = Obligation<'tcx, ty::AliasTy<'tcx>>;

pub(super) struct InProgress;
/// Signals the various ways in which we can fail to normalize a projection
#[derive(Debug)]
pub enum ProjectionNormalizationFailure {
/// Blocked on inference variables resolving
InProgress,
/// Fatal failure, overflow occurred.
Overflow(OverflowError),
}

/// When attempting to resolve `<T as TraitRef>::Name` ...
#[derive(Debug)]
Expand Down Expand Up @@ -252,7 +259,10 @@ fn project_and_unify_type<'cx, 'tcx>(
) {
Ok(Some(n)) => n,
Ok(None) => return ProjectAndUnifyResult::FailedNormalization,
Err(InProgress) => return ProjectAndUnifyResult::Recursive,
Err(
ProjectionNormalizationFailure::InProgress
| ProjectionNormalizationFailure::Overflow(_),
) => return ProjectAndUnifyResult::Recursive,
};
debug!(?normalized, ?obligations, "project_and_unify_type result");
let actual = obligation.predicate.term;
Expand Down Expand Up @@ -298,24 +308,29 @@ pub fn normalize_projection_type<'a, 'b, 'tcx>(
cause: ObligationCause<'tcx>,
depth: usize,
obligations: &mut Vec<PredicateObligation<'tcx>>,
) -> Term<'tcx> {
opt_normalize_projection_type(
) -> Result<Term<'tcx>, OverflowError> {
let normalized = opt_normalize_projection_type(
selcx,
param_env,
projection_ty,
cause.clone(),
depth,
obligations,
)
.ok()
.flatten()
.unwrap_or_else(move || {
// if we bottom out in ambiguity, create a type variable
// and a deferred predicate to resolve this when more type
// information is available.

selcx.infcx.infer_projection(param_env, projection_ty, cause, depth + 1, obligations).into()
})
);
match normalized {
Ok(Some(term)) => Ok(term),
Ok(None) | Err(ProjectionNormalizationFailure::InProgress) => {
// if we bottom out in ambiguity, create a type variable
// and a deferred predicate to resolve this when more type
// information is available.

Ok(selcx
.infcx
.infer_projection(param_env, projection_ty, cause, depth + 1, obligations)
.into())
}
Err(ProjectionNormalizationFailure::Overflow(oflo)) => Err(oflo),
}
}

/// The guts of `normalize`: normalize a specific projection like `<T
Expand All @@ -328,15 +343,15 @@ pub fn normalize_projection_type<'a, 'b, 'tcx>(
/// often immediately appended to another obligations vector. So now this
/// function takes an obligations vector and appends to it directly, which is
/// slightly uglier but avoids the need for an extra short-lived allocation.
#[instrument(level = "debug", skip(selcx, param_env, cause, obligations))]
#[instrument(level = "debug", skip(selcx, param_env, cause, obligations), ret)]
pub(super) fn opt_normalize_projection_type<'a, 'b, 'tcx>(
selcx: &'a mut SelectionContext<'b, 'tcx>,
param_env: ty::ParamEnv<'tcx>,
projection_ty: ty::AliasTy<'tcx>,
cause: ObligationCause<'tcx>,
depth: usize,
obligations: &mut Vec<PredicateObligation<'tcx>>,
) -> Result<Option<Term<'tcx>>, InProgress> {
) -> Result<Option<Term<'tcx>>, ProjectionNormalizationFailure> {
let infcx = selcx.infcx;
debug_assert!(!selcx.infcx.next_trait_solver());
// Don't use the projection cache in intercrate mode -
Expand Down Expand Up @@ -387,11 +402,11 @@ pub(super) fn opt_normalize_projection_type<'a, 'b, 'tcx>(
if use_cache {
infcx.inner.borrow_mut().projection_cache().recur(cache_key);
}
return Err(InProgress);
return Err(ProjectionNormalizationFailure::InProgress);
}
Err(ProjectionCacheEntry::Recur) => {
debug!("recur cache");
return Err(InProgress);
return Err(ProjectionNormalizationFailure::InProgress);
}
Err(ProjectionCacheEntry::NormalizedTy { ty, complete: _ }) => {
// This is the hottest path in this function.
Expand Down Expand Up @@ -471,6 +486,12 @@ pub(super) fn opt_normalize_projection_type<'a, 'b, 'tcx>(
}
Ok(None)
}
Err(ProjectionError::TraitSelectionError(SelectionError::Overflow(oflo))) => {
if use_cache {
infcx.inner.borrow_mut().projection_cache().error(cache_key);
}
Err(ProjectionNormalizationFailure::Overflow(oflo))
}
Err(ProjectionError::TraitSelectionError(_)) => {
debug!("opt_normalize_projection_type: ERROR");
// if we got an error processing the `T as Trait` part,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -925,6 +925,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
// since we don't actually use them.
&mut vec![],
)
.ok()?
Copy link
Contributor Author

Choose a reason for hiding this comment

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

if normalization overflowed, we will not do the migration. To combine both infallible overflowing normalization and this migration, some truly cursed shenanigans are necessary, if possible at all.

.ty()
.unwrap();

Expand Down
13 changes: 7 additions & 6 deletions compiler/rustc_trait_selection/src/traits/select/confirmation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ use rustc_data_structures::stack::ensure_sufficient_stack;
use rustc_hir::lang_items::LangItem;
use rustc_infer::infer::HigherRankedType;
use rustc_infer::infer::{DefineOpaqueTypes, InferOk};
use rustc_infer::traits::OverflowError;
use rustc_middle::traits::{BuiltinImplSource, SignatureMismatchData};
use rustc_middle::ty::{
self, GenericArgs, GenericArgsRef, GenericParamDefKind, ToPolyTraitRef, ToPredicate,
Expand Down Expand Up @@ -63,7 +64,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
}

ImplCandidate(impl_def_id) => {
ImplSource::UserDefined(self.confirm_impl_candidate(obligation, impl_def_id))
ImplSource::UserDefined(self.confirm_impl_candidate(obligation, impl_def_id)?)
}

AutoImplCandidate => {
Expand Down Expand Up @@ -441,14 +442,14 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
&mut self,
obligation: &PolyTraitObligation<'tcx>,
impl_def_id: DefId,
) -> ImplSourceUserDefinedData<'tcx, PredicateObligation<'tcx>> {
) -> Result<ImplSourceUserDefinedData<'tcx, PredicateObligation<'tcx>>, OverflowError> {
debug!(?obligation, ?impl_def_id, "confirm_impl_candidate");

// First, create the generic parameters by matching the impl again,
// this time not in a probe.
let args = self.rematch_impl(impl_def_id, obligation);
let args = self.rematch_impl(impl_def_id, obligation)?;
debug!(?args, "impl args");
ensure_sufficient_stack(|| {
Ok(ensure_sufficient_stack(|| {
self.vtable_impl(
impl_def_id,
args,
Expand All @@ -457,7 +458,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
obligation.param_env,
obligation.predicate,
)
})
}))
}

fn vtable_impl(
Expand Down Expand Up @@ -1370,7 +1371,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
trait_pred.trait_ref.def_id = drop_trait;
trait_pred
});
let args = self.rematch_impl(impl_def_id, &new_obligation);
let args = self.rematch_impl(impl_def_id, &new_obligation)?;
debug!(?args, "impl args");

let cause = obligation.derived_cause(|derived| {
Expand Down
Loading