use `evaluate_obligation` to decide when to do unsized coercions

[nikomatsakis]

As part of the coercion logic, we sometimes invoke coerce_unsized:

rust/src/librustc_typeck/check/coercion.rs

Line 457 in cb1ce7d

fn coerce_unsized(&self, source: Ty<'tcx>, target: Ty<'tcx>) -> CoerceResult<'tcx> {

This would e.g. coerce from &[T; 32] to &[T] or from Arc<T> to Arc<dyn Trait> where T: Trait. To decide whether or not we are going to do this, we want to check if Arc<T>: CoerceUnsized<Arc<dyn Trait>> is implemented (or something like that). We do this in this funky little loop here:

rust/src/librustc_typeck/check/coercion.rs

Lines 538 to 552 in cb1ce7d

	let mut selcx = traits::SelectionContext::new(self);
	// Use a FIFO queue for this custom fulfillment procedure.
	let mut queue = VecDeque::new();
	// Create an obligation for `Source: CoerceUnsized<Target>`.
	let cause = ObligationCause::misc(self.cause.span, self.body_id);
	queue.push_back(self.tcx.predicate_for_trait_def(self.fcx.param_env,
	cause,
	coerce_unsized_did,
	0,
	coerce_source,
	&[coerce_target]));
	let mut has_unsized_tuple_coercion = false;

This kind of pulls out all the (transitive) requires to prove CoerceUnsized and ignores the rest. However, if we ever hope to define coercions in terms of the trait system, what we really ought to be doing is using an evaluate_obligation check.

Hopefully we can get away with this backwards compatibly.

@eddyb told me at some point -- iirc -- that the reason this loop exists is for diagnostics. So making this change may require some work on that point.

cc @leodasvacas @mikeyhew

[mikeyhew]

It looks like the only reason this is done in a loop right now is to check for the unsized tuple coercion, which is currently unstable. As @nikomatsakis pointed out on Discord, that check can probably be done during trait selection instead. I'll take a look and hopefully open a PR soon.

[eddyb]

to check for the unsized tuple coercion

The loop predates unsized tuples, so that's not it. I added the loop because:

1. unsizing should be ignored if other coercions (e.g. deref) apply instead
   • this means that some work needs to be done up-front to check if unsizing really applies
2. errors from conditions of unsizing coercions shouldn't show up as type mismatches
   • that is, if no coercions apply, the error is a (maybe {un,mis}informative) type mismatch
   • this means that once we're "sure" unsizing should apply, we commit to further errors
   • e.g. &T: &Trait results in a &T: CoerceUnsized<&Trait> obligation, which further results in a T: Unsize<Trait> one - we force-evaluate both of those, which leaves us with a T: Trait obligation, at which point, without checking whether T really implements Trait, we apply the coercion, so that the user can get a T: Trait error, instead of silently ignoring it.

All of this would work itself out within the trait system, as each kind of coercion would potentially result in a candidate, and the list of candidates would be refined until no ambiguities remain, and any errors caused by further evaluating an unambiguous candidate could be presented to the user.

EDIT: I missed one important bit. Among those coercions, but with higher priority, would be unifying the two types. That is, T: Coerce<T>. However, we can't pick that if inference is inconclusive.

[alexreg]

@mikeyhew Still interested in tackling this by chance? Mainly out of curiosity, since I'm too busy with other PRs right now, and it would be a while until I could get to this...

[mikeyhew]

@alexreg I actually tried replacing the loop with predicate_must_hold and predicate_may_hold, but the former was too restrictive and the latter had false positives that resulted in code failing to compile. I'm not sure what to do at this point.

[alexreg]

@mikeyhew Hmm, sounds tricky. Maybe @eddyb or @nikomatsakis could advise?

[basil-cow]

I wasn't able to come up with a reasonable solution to this (yet?), but I would like to provide some context for future reference to those who might delve into this.

There are two ways in which this loop bypasses the trait system.
a) When proving, it's only interested in Unsize and CoerceUnsized trait obligations. All other obligations are postponed (as niko mentioned) and a lot of the time they are not provable when the coercion is taking place (that is why predicate_must_hold is too restrictive).
b) When we stumble into a $0: Unsize<dyn Trait> obligation where $0: Sized, we commit to the unsizing coercion (see 2. in eddyb's comment). Besides diagnostics, it also affects type inference. These tests are for that specifically. I'll copy the relevant part here for convenience.

trait Xyz {}
struct S;
struct T;
impl Xyz for S {}
impl Xyz for T {}

fn foo_no_never() {
    let mut x /* : Option<S> */ = None;
    let mut first_iter = false;
    loop {
        if !first_iter {
            let y: Box<dyn Xyz>
                = /* Box<$0> is coerced to Box<Xyz> here */ Box::new(x.unwrap());
        }

        x = Some(S);
        first_iter = true;
    }

    let mut y : Option<S> = None;
    // assert types are equal
    std::mem::swap(&mut x, &mut y);
}

Without the Unsize<dyn Trait> special case, type of x is inferred to be dyn Xyz and it all comes crashing down (x doesn't have a size known at compile time blah blah).

My opinion is that in order to address this some deeper change to inference/coercion system is needed because I don't see a way to emulate a) or b) as a user of trait system. Ideally it's possible to tinker inference so that a) and b) are not needed, but I don't know how much truth there is to that idea.