Skip to content

Commit

Permalink
Don't immediately error for cycles during normalization
Browse files Browse the repository at this point in the history
  • Loading branch information
matthewjasper committed Aug 20, 2020
1 parent 1c5fe50 commit 1101496
Show file tree
Hide file tree
Showing 16 changed files with 176 additions and 90 deletions.
12 changes: 8 additions & 4 deletions src/librustc_trait_selection/traits/auto_trait.rs
Original file line number Diff line number Diff line change
Expand Up @@ -729,7 +729,7 @@ impl AutoTraitFinder<'tcx> {
// and turn them into an explicit negative impl for our type.
debug!("Projecting and unifying projection predicate {:?}", predicate);

match poly_project_and_unify_type(select, &obligation.with(p)) {
match project::poly_project_and_unify_type(select, &obligation.with(p)) {
Err(e) => {
debug!(
"evaluate_nested_obligations: Unable to unify predicate \
Expand All @@ -738,7 +738,11 @@ impl AutoTraitFinder<'tcx> {
);
return false;
}
Ok(Some(v)) => {
Ok(Err(project::InProgress)) => {
debug!("evaluate_nested_obligations: recursive projection predicate");
return false;
}
Ok(Ok(Some(v))) => {
// We only care about sub-obligations
// when we started out trying to unify
// some inference variables. See the comment above
Expand All @@ -757,8 +761,8 @@ impl AutoTraitFinder<'tcx> {
}
}
}
Ok(None) => {
// It's ok not to make progress when hvave no inference variables -
Ok(Ok(None)) => {
// It's ok not to make progress when have no inference variables -
// in that case, we were only performing unifcation to check if an
// error occurred (which would indicate that it's impossible for our
// type to implement the auto trait).
Expand Down
16 changes: 11 additions & 5 deletions src/librustc_trait_selection/traits/fulfill.rs
Original file line number Diff line number Diff line change
Expand Up @@ -426,14 +426,20 @@ impl<'a, 'b, 'tcx> ObligationProcessor for FulfillProcessor<'a, 'b, 'tcx> {

ty::PredicateKind::Projection(ref data) => {
let project_obligation = obligation.with(*data);
let tcx = self.selcx.tcx();
match project::poly_project_and_unify_type(self.selcx, &project_obligation) {
Ok(None) => {
let tcx = self.selcx.tcx();
pending_obligation.stalled_on =
trait_ref_infer_vars(self.selcx, data.to_poly_trait_ref(tcx));
Ok(Ok(Some(os))) => ProcessResult::Changed(mk_pending(os)),
Ok(Ok(None)) => {
pending_obligation.stalled_on = trait_ref_infer_vars(
self.selcx,
project_obligation.predicate.to_poly_trait_ref(tcx),
);
ProcessResult::Unchanged
}
Ok(Some(os)) => ProcessResult::Changed(mk_pending(os)),
// Let the caller handle the recursion
Ok(Err(project::InProgress)) => ProcessResult::Changed(mk_pending(vec![
pending_obligation.obligation.clone(),
])),
Err(e) => ProcessResult::Error(CodeProjectionError(e)),
}
}
Expand Down
4 changes: 1 addition & 3 deletions src/librustc_trait_selection/traits/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,7 @@ pub use self::object_safety::is_vtable_safe_method;
pub use self::object_safety::MethodViolationCode;
pub use self::object_safety::ObjectSafetyViolation;
pub use self::on_unimplemented::{OnUnimplementedDirective, OnUnimplementedNote};
pub use self::project::{
normalize, normalize_projection_type, normalize_to, poly_project_and_unify_type,
};
pub use self::project::{normalize, normalize_projection_type, normalize_to};
pub use self::select::{EvaluationCache, SelectionCache, SelectionContext};
pub use self::select::{EvaluationResult, IntercrateAmbiguityCause, OverflowError};
pub use self::specialize::specialization_graph::FutureCompatOverlapError;
Expand Down
74 changes: 45 additions & 29 deletions src/librustc_trait_selection/traits/project.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ pub type ProjectionObligation<'tcx> = Obligation<'tcx, ty::ProjectionPredicate<'

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

pub(super) struct InProgress;

/// When attempting to resolve `<T as TraitRef>::Name` ...
#[derive(Debug)]
pub enum ProjectionTyError<'tcx> {
Expand Down Expand Up @@ -142,10 +144,26 @@ impl<'tcx> ProjectionTyCandidateSet<'tcx> {
///
/// If successful, this may result in additional obligations. Also returns
/// the projection cache key used to track these additional obligations.
pub fn poly_project_and_unify_type<'cx, 'tcx>(
///
/// ## Returns
///
/// - `Err(_)`: the projection can be normalized, but is not equal to the
/// expected type.
/// - `Ok(Err(InProgress))`: this is called recursively while normalizing
/// the same projection.
/// - `Ok(Ok(None))`: The projection cannot be normalized due to ambiguity
/// (resolving some inference variables in the projection may fix this).
/// - `Ok(Ok(Some(obligations)))`: The projection bound holds subject to
/// the given obligations. If the projection cannot be normalized because
/// the required trait bound doesn't hold this returned with `obligations`
/// being a predicate that cannot be proven.
pub(super) fn poly_project_and_unify_type<'cx, 'tcx>(
selcx: &mut SelectionContext<'cx, 'tcx>,
obligation: &PolyProjectionObligation<'tcx>,
) -> Result<Option<Vec<PredicateObligation<'tcx>>>, MismatchedProjectionTypes<'tcx>> {
) -> Result<
Result<Option<Vec<PredicateObligation<'tcx>>>, InProgress>,
MismatchedProjectionTypes<'tcx>,
> {
debug!("poly_project_and_unify_type(obligation={:?})", obligation);

let infcx = selcx.infcx();
Expand All @@ -164,10 +182,15 @@ pub fn poly_project_and_unify_type<'cx, 'tcx>(
/// <T as Trait>::U == V
///
/// If successful, this may result in additional obligations.
///
/// See [poly_project_and_unify_type] for an explanation of the return value.
fn project_and_unify_type<'cx, 'tcx>(
selcx: &mut SelectionContext<'cx, 'tcx>,
obligation: &ProjectionObligation<'tcx>,
) -> Result<Option<Vec<PredicateObligation<'tcx>>>, MismatchedProjectionTypes<'tcx>> {
) -> Result<
Result<Option<Vec<PredicateObligation<'tcx>>>, InProgress>,
MismatchedProjectionTypes<'tcx>,
> {
debug!("project_and_unify_type(obligation={:?})", obligation);

let mut obligations = vec![];
Expand All @@ -179,8 +202,9 @@ fn project_and_unify_type<'cx, 'tcx>(
obligation.recursion_depth,
&mut obligations,
) {
Some(n) => n,
None => return Ok(None),
Ok(Some(n)) => n,
Ok(None) => return Ok(Ok(None)),
Err(InProgress) => return Ok(Err(InProgress)),
};

debug!(
Expand All @@ -195,7 +219,7 @@ fn project_and_unify_type<'cx, 'tcx>(
{
Ok(InferOk { obligations: inferred_obligations, value: () }) => {
obligations.extend(inferred_obligations);
Ok(Some(obligations))
Ok(Ok(Some(obligations)))
}
Err(err) => {
debug!("project_and_unify_type: equating types encountered error {:?}", err);
Expand Down Expand Up @@ -418,6 +442,8 @@ pub fn normalize_projection_type<'a, 'b, 'tcx>(
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
Expand Down Expand Up @@ -454,7 +480,7 @@ fn opt_normalize_projection_type<'a, 'b, 'tcx>(
cause: ObligationCause<'tcx>,
depth: usize,
obligations: &mut Vec<PredicateObligation<'tcx>>,
) -> Option<Ty<'tcx>> {
) -> Result<Option<Ty<'tcx>>, InProgress> {
let infcx = selcx.infcx();

let projection_ty = infcx.resolve_vars_if_possible(&projection_ty);
Expand Down Expand Up @@ -486,7 +512,7 @@ fn opt_normalize_projection_type<'a, 'b, 'tcx>(
"opt_normalize_projection_type: \
found cache entry: ambiguous"
);
return None;
return Ok(None);
}
Err(ProjectionCacheEntry::InProgress) => {
// If while normalized A::B, we are asked to normalize
Expand All @@ -501,24 +527,14 @@ fn opt_normalize_projection_type<'a, 'b, 'tcx>(
// to normalize `A::B`, we will want to check the
// where-clauses in scope. So we will try to unify `A::B`
// with `A::B`, which can trigger a recursive
// normalization. In that case, I think we will want this code:
//
// ```
// let ty = selcx.tcx().mk_projection(projection_ty.item_def_id,
// projection_ty.substs;
// return Some(NormalizedTy { value: v, obligations: vec![] });
// ```
// normalization.

debug!(
"opt_normalize_projection_type: \
found cache entry: in-progress"
);

// But for now, let's classify this as an overflow:
let recursion_limit = selcx.tcx().sess.recursion_limit();
let obligation =
Obligation::with_depth(cause, recursion_limit.0, param_env, projection_ty);
selcx.infcx().report_overflow_error(&obligation, false);
return Err(InProgress);
}
Err(ProjectionCacheEntry::NormalizedTy(ty)) => {
// This is the hottest path in this function.
Expand Down Expand Up @@ -554,7 +570,7 @@ fn opt_normalize_projection_type<'a, 'b, 'tcx>(
cause,
depth,
));
return Some(ty.value);
return Ok(Some(ty.value));
}
Err(ProjectionCacheEntry::Error) => {
debug!(
Expand All @@ -563,7 +579,7 @@ fn opt_normalize_projection_type<'a, 'b, 'tcx>(
);
let result = normalize_to_error(selcx, param_env, projection_ty, cause, depth);
obligations.extend(result.obligations);
return Some(result.value);
return Ok(Some(result.value));
}
}

Expand Down Expand Up @@ -610,7 +626,7 @@ fn opt_normalize_projection_type<'a, 'b, 'tcx>(
let cache_value = prune_cache_value_obligations(infcx, &result);
infcx.inner.borrow_mut().projection_cache().insert_ty(cache_key, cache_value);
obligations.extend(result.obligations);
Some(result.value)
Ok(Some(result.value))
}
Ok(ProjectedTy::NoProgress(projected_ty)) => {
debug!(
Expand All @@ -621,15 +637,15 @@ fn opt_normalize_projection_type<'a, 'b, 'tcx>(
let result = Normalized { value: projected_ty, obligations: vec![] };
infcx.inner.borrow_mut().projection_cache().insert_ty(cache_key, result.clone());
// No need to extend `obligations`.
Some(result.value)
Ok(Some(result.value))
}
Err(ProjectionTyError::TooManyCandidates) => {
debug!(
"opt_normalize_projection_type: \
too many candidates"
);
infcx.inner.borrow_mut().projection_cache().ambiguous(cache_key);
None
Ok(None)
}
Err(ProjectionTyError::TraitSelectionError(_)) => {
debug!("opt_normalize_projection_type: ERROR");
Expand All @@ -641,7 +657,7 @@ fn opt_normalize_projection_type<'a, 'b, 'tcx>(
infcx.inner.borrow_mut().projection_cache().error(cache_key);
let result = normalize_to_error(selcx, param_env, projection_ty, cause, depth);
obligations.extend(result.obligations);
Some(result.value)
Ok(Some(result.value))
}
}
}
Expand Down Expand Up @@ -1112,11 +1128,11 @@ fn assemble_candidates_from_impls<'cx, 'tcx>(
}
super::ImplSourceAutoImpl(..) | super::ImplSourceBuiltin(..) => {
// These traits have no associated types.
span_bug!(
selcx.tcx().sess.delay_span_bug(
obligation.cause.span,
"Cannot project an associated type from `{:?}`",
impl_source
&format!("Cannot project an associated type from `{:?}`", impl_source),
);
return Err(());
}
};

Expand Down
10 changes: 8 additions & 2 deletions src/librustc_trait_selection/traits/select/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -458,7 +458,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
&ty::PredicateKind::Projection(data) => {
let project_obligation = obligation.with(data);
match project::poly_project_and_unify_type(self, &project_obligation) {
Ok(Some(mut subobligations)) => {
Ok(Ok(Some(mut subobligations))) => {
self.add_depth(subobligations.iter_mut(), obligation.recursion_depth);
let result = self.evaluate_predicates_recursively(
previous_stack,
Expand All @@ -471,7 +471,13 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
}
result
}
Ok(None) => Ok(EvaluatedToAmbig),
Ok(Ok(None)) => Ok(EvaluatedToAmbig),
// EvaluatedToRecur might also be acceptable here, but use
// Unknown for now because it means that we won't dismiss a
// selection candidate solely because it has a projection
// cycle. This is closest to the previous behavior of
// immediately erroring.
Ok(Err(project::InProgress)) => Ok(EvaluatedToUnknown),
Err(_) => Ok(EvaluatedToErr),
}
}
Expand Down
8 changes: 4 additions & 4 deletions src/test/ui/associated-types/defaults-cyclic-fail-1.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,16 +26,16 @@ impl Tr for u32 {

// ...but only if this actually breaks the cycle
impl Tr for bool {
//~^ ERROR overflow evaluating the requirement
//~^ ERROR type mismatch resolving `<bool as Tr>::B == _`
type A = Box<Self::B>;
//~^ ERROR overflow evaluating the requirement
//~^ ERROR type mismatch resolving `<bool as Tr>::B == _`
}
// (the error is shown twice for some reason)

impl Tr for usize {
//~^ ERROR overflow evaluating the requirement
//~^ ERROR type mismatch resolving `<usize as Tr>::B == _`
type B = &'static Self::A;
//~^ ERROR overflow evaluating the requirement
//~^ ERROR type mismatch resolving `<usize as Tr>::A == _`
}

fn main() {
Expand Down
21 changes: 11 additions & 10 deletions src/test/ui/associated-types/defaults-cyclic-fail-1.stderr
Original file line number Diff line number Diff line change
@@ -1,33 +1,34 @@
error[E0275]: overflow evaluating the requirement `<() as Tr>::B`
error[E0275]: overflow evaluating the requirement `<() as Tr>::B == _`
--> $DIR/defaults-cyclic-fail-1.rs:10:6
|
LL | impl Tr for () {}
| ^^

error[E0275]: overflow evaluating the requirement `<bool as Tr>::B`
error[E0271]: type mismatch resolving `<bool as Tr>::B == _`
--> $DIR/defaults-cyclic-fail-1.rs:28:6
|
LL | impl Tr for bool {
| ^^
| ^^ cyclic type of infinite size

error[E0275]: overflow evaluating the requirement `<usize as Tr>::B`
error[E0271]: type mismatch resolving `<usize as Tr>::B == _`
--> $DIR/defaults-cyclic-fail-1.rs:35:6
|
LL | impl Tr for usize {
| ^^
| ^^ cyclic type of infinite size

error[E0275]: overflow evaluating the requirement `<bool as Tr>::B`
error[E0271]: type mismatch resolving `<bool as Tr>::B == _`
--> $DIR/defaults-cyclic-fail-1.rs:30:5
|
LL | type A = Box<Self::B>;
| ^^^^^^^^^^^^^^^^^^^^^^
| ^^^^^^^^^^^^^^^^^^^^^^ cyclic type of infinite size

error[E0275]: overflow evaluating the requirement `<usize as Tr>::A`
error[E0271]: type mismatch resolving `<usize as Tr>::A == _`
--> $DIR/defaults-cyclic-fail-1.rs:37:5
|
LL | type B = &'static Self::A;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ cyclic type of infinite size

error: aborting due to 5 previous errors

For more information about this error, try `rustc --explain E0275`.
Some errors have detailed explanations: E0271, E0275.
For more information about an error, try `rustc --explain E0271`.
10 changes: 5 additions & 5 deletions src/test/ui/associated-types/defaults-cyclic-fail-2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ trait Tr {

// ...but is an error in any impl that doesn't override at least one of the defaults
impl Tr for () {}
//~^ ERROR overflow evaluating the requirement
//~^ ERROR type mismatch resolving `<() as Tr>::B == _`

// As soon as at least one is redefined, it works:
impl Tr for u8 {
Expand All @@ -28,16 +28,16 @@ impl Tr for u32 {

// ...but only if this actually breaks the cycle
impl Tr for bool {
//~^ ERROR overflow evaluating the requirement
//~^ ERROR type mismatch resolving `<bool as Tr>::B == _`
type A = Box<Self::B>;
//~^ ERROR overflow evaluating the requirement
//~^ ERROR type mismatch resolving `<bool as Tr>::B == _`
}
// (the error is shown twice for some reason)

impl Tr for usize {
//~^ ERROR overflow evaluating the requirement
//~^ ERROR type mismatch resolving `<usize as Tr>::B == _`
type B = &'static Self::A;
//~^ ERROR overflow evaluating the requirement
//~^ ERROR type mismatch resolving `<usize as Tr>::A == _`
}

fn main() {
Expand Down
Loading

0 comments on commit 1101496

Please sign in to comment.