Skip to content

Handle recursion limit for subtype and well-formed predicates #117754

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

Merged
merged 1 commit into from
Dec 2, 2023
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
35 changes: 30 additions & 5 deletions compiler/rustc_trait_selection/src/traits/fulfill.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use crate::infer::{InferCtxt, TyOrConstInferVar};
use crate::traits::error_reporting::TypeErrCtxtExt;
use rustc_data_structures::captures::Captures;
use rustc_data_structures::obligation_forest::ProcessResult;
use rustc_data_structures::obligation_forest::{Error, ForestObligation, Outcome};
Expand Down Expand Up @@ -410,6 +411,29 @@ impl<'a, 'tcx> ObligationProcessor for FulfillProcessor<'a, 'tcx> {
}
}

ty::PredicateKind::Ambiguous => ProcessResult::Unchanged,
ty::PredicateKind::AliasRelate(..) => {
bug!("AliasRelate is only used for new solver")
}

// General case overflow check. Allow `process_trait_obligation`
// and `process_projection_obligation` to handle checking for
// the recursion limit themselves. Also don't check some
// predicate kinds that don't give further obligations.
_ if !self
.selcx
.tcx()
.recursion_limit()
.value_within_limit(obligation.recursion_depth) =>
{
self.selcx.infcx.err_ctxt().report_overflow_error(
&obligation.predicate,
obligation.cause.span,
false,
|_| {},
);
}

ty::PredicateKind::Clause(ty::ClauseKind::WellFormed(arg)) => {
match wf::obligations(
self.selcx.infcx,
Expand Down Expand Up @@ -440,7 +464,12 @@ impl<'a, 'tcx> ObligationProcessor for FulfillProcessor<'a, 'tcx> {
vec![TyOrConstInferVar::Ty(a), TyOrConstInferVar::Ty(b)];
ProcessResult::Unchanged
}
Ok(Ok(ok)) => ProcessResult::Changed(mk_pending(ok.obligations)),
Ok(Ok(mut ok)) => {
for subobligation in &mut ok.obligations {
subobligation.set_depth_from_parent(obligation.recursion_depth);
}
ProcessResult::Changed(mk_pending(ok.obligations))
}
Ok(Err(err)) => {
let expected_found =
ExpectedFound::new(subtype.a_is_expected, subtype.a, subtype.b);
Expand Down Expand Up @@ -611,10 +640,6 @@ impl<'a, 'tcx> ObligationProcessor for FulfillProcessor<'a, 'tcx> {
}
}
}
ty::PredicateKind::Ambiguous => ProcessResult::Unchanged,
ty::PredicateKind::AliasRelate(..) => {
bug!("AliasRelate is only used for new solver")
}
ty::PredicateKind::Clause(ty::ClauseKind::ConstArgHasType(ct, ty)) => {
match self.selcx.infcx.at(&obligation.cause, obligation.param_env).eq(
DefineOpaqueTypes::No,
Expand Down
17 changes: 17 additions & 0 deletions tests/ui/traits/subtype-recursion-limit.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// Variant of #117151 when the overflow comes entirely from subtype predicates.

#![allow(unreachable_code)]

use std::ptr;

fn main() {
// Give x and y completely unconstrained types. Using a function call
// or `as` cast would create a well-formed predicate.
let x = return;
let y = return;
let mut w = (x, y);
//~^ ERROR overflow evaluating the requirement
// Avoid creating lifetimes, `Sized` bounds or function calls.
let a = (ptr::addr_of!(y), ptr::addr_of!(x));
w = a;
}
9 changes: 9 additions & 0 deletions tests/ui/traits/subtype-recursion-limit.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
error[E0275]: overflow evaluating the requirement `_ <: *const _`
--> $DIR/subtype-recursion-limit.rs:12:17
|
LL | let mut w = (x, y);
| ^^^^^^

error: aborting due to 1 previous error

For more information about this error, try `rustc --explain E0275`.
27 changes: 27 additions & 0 deletions tests/ui/traits/well-formed-recursion-limit.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
// Regression test for #117151, this used to hang the compiler

pub type ISO<A: 'static, B: 'static> = (Box<dyn Fn(A) -> B>, Box<dyn Fn(B) -> A>);
pub fn iso<A: 'static, B: 'static, F1, F2>(a: F1, b: F2) -> ISO<A, B>
where
F1: 'static + Fn(A) -> B,
F2: 'static + Fn(B) -> A,
{
(Box::new(a), Box::new(b))
}
pub fn iso_un_option<A: 'static, B: 'static>(i: ISO<Option<A>, Option<B>>) -> ISO<A, B> {
let (ab, ba) = (i.ab, i.ba);
//~^ ERROR no field `ab` on type
//~| ERROR no field `ba` on type
let left = move |o_a| match o_a {
//~^ ERROR overflow evaluating the requirement
None => panic!("absured"),
Some(a) => a,
};
let right = move |o_b| match o_b {
None => panic!("absurd"),
Some(b) => b,
};
iso(left, right)
}

fn main() {}
22 changes: 22 additions & 0 deletions tests/ui/traits/well-formed-recursion-limit.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
error[E0609]: no field `ab` on type `(Box<(dyn Fn(Option<A>) -> Option<B> + 'static)>, Box<(dyn Fn(Option<B>) -> Option<A> + 'static)>)`
--> $DIR/well-formed-recursion-limit.rs:12:23
|
LL | let (ab, ba) = (i.ab, i.ba);
| ^^ unknown field

error[E0609]: no field `ba` on type `(Box<(dyn Fn(Option<A>) -> Option<B> + 'static)>, Box<(dyn Fn(Option<B>) -> Option<A> + 'static)>)`
--> $DIR/well-formed-recursion-limit.rs:12:29
|
LL | let (ab, ba) = (i.ab, i.ba);
| ^^ unknown field

error[E0275]: overflow evaluating the requirement `_ <: Option<_>`
--> $DIR/well-formed-recursion-limit.rs:15:33
|
LL | let left = move |o_a| match o_a {
| ^^^

error: aborting due to 3 previous errors

Some errors have detailed explanations: E0275, E0609.
For more information about an error, try `rustc --explain E0275`.
25 changes: 25 additions & 0 deletions tests/ui/type-inference/generalize-subtyped-variables.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
// Test for specific details of how we handle higher-ranked subtyping to make
// sure that any changes are made deliberately.
//
// - `let y = x` creates a `Subtype` obligation that is deferred for later.
// - `w = a` sets the type of `x` to `Option<for<'a> fn(&'a ())>` and generalizes
// `z` first to `Option<_>` and then to `Option<fn(&'0 ())>`.
// - The various subtyping obligations are then processed.
//
// This requires that
// 1. the `Subtype` obligation from `y = x` isn't processed while the types of
// `w` and `a` are being unified.
// 2. the pending subtype obligation isn't considered when determining the type
// to generalize `z` to first (when related to the type of `y`).
//
// Found when considering fixes to #117151
// check-pass
Copy link
Contributor

Choose a reason for hiding this comment

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

This test feels a bit :/ currently the way we generalize higher ranked types is wrong when subtyping.

we currently do the following (using x for the infer var in x: None<_>):

  • emit (and stall on) a Subtype(x <: y) obligation
  • the w = a assignment sets x to for<'a> fn(&'a ()), emits a Subtype(y <: z) obligation and generalizes z to fn(&'0 ()).
  • we then prove Subtype(x <: y) setting y to for<'a> fn(&'a ()) and then prove Subtype(y <: z) which is now trivial as both types are rigid.

If we were to prove the Subtype obligations before generalizing z we'd get an incorrect error, e.g:

fn main() {
    let mut x = None;
    let y = x;
    let z = Default::default();
    let mut w = (&mut x, z, z);
    let mut temp = None::<for<'a> fn(&'a ())>;
    w.0 = &mut temp;
    w.1 = y;
    w.2 = None::<fn(&'static ())>; //~ ERROR mismatched types
}

It feels like this test mostly tests a very specific impl detail of our subtyping algorithm where we do not try to eagerly make progress on previous Subtype obligations while relating.

I think the explanation here is somewhat misleading. The reason this works is because generalization does not care about subtype relations (apart from the occurs check). I would instead use sth like "Subtyping of inference variables is currently handled via delayed Subtype obligations. We do not immediately process them after constraining one of the involved inference variables. We'd otherwise incorrectly constrain the type of z as generalization does not handle binders correctly".


fn main() {
let mut x = None;
let y = x;
let z = Default::default();
let mut w = (&mut x, z, z);
let a = (&mut None::<fn(&())>, y, None::<fn(&'static ())>);
w = a;
}