Replace consider_unification_despite_ambiguity with new obligation variant #32780
Conversation
|
w.r.t. the Travis failure: s'what I get for not running stage 2 tests locally, I guess. |
|
Hmm, the error is unexpected. I did not anticipate that this would affect anything like that (and it's interesting that this outdated help text still exists, too). |
| @@ -179,6 +179,9 @@ fn deduce_expectations_from_obligations<'a,'tcx>( | |||
| ty::Predicate::TypeOutlives(..) => None, | |||
| ty::Predicate::WellFormed(..) => None, | |||
| ty::Predicate::ObjectSafe(..) => None, | |||
| ty::Predicate::ClosureKind(_closure_def_id, kind) => { | |||
| return Some(kind); | |||
There was a problem hiding this comment.
@nikomatsakis a note: changing this to expressing None (as opposed to returning) causes the stage 2 librustc_typeck compile to go through without adversely affecting tests. Not sure if that's 'right' though.
|
@soltanmm I did some investigation. This fixes the problem, I believe: diff --git a/src/librustc_typeck/check/closure.rs b/src/librustc_typeck/check/closure.rs
index 84c277a..ed58070 100644
--- a/src/librustc_typeck/check/closure.rs
+++ b/src/librustc_typeck/check/closure.rs
@@ -47,7 +47,8 @@ fn check_closure<'a,'tcx>(fcx: &FnCtxt<'a,'tcx>,
expected_sig: Option<ty::FnSig<'tcx>>) {
let expr_def_id = fcx.tcx().map.local_def_id(expr.id);
- debug!("check_closure opt_kind={:?} expected_sig={:?}",
+ debug!("check_closure expr.id={:?} opt_kind={:?} expected_sig={:?}",
+ expr.id,
opt_kind,
expected_sig);
@@ -179,9 +180,16 @@ fn deduce_expectations_from_obligations<'a,'tcx>(
ty::Predicate::TypeOutlives(..) => None,
ty::Predicate::WellFormed(..) => None,
ty::Predicate::ObjectSafe(..) => None,
- ty::Predicate::ClosureKind(_closure_def_id, kind) => {
- return Some(kind);
- }
+
+ // NB: This predicate is created by breaking down a
+ // `ClosureType: FnFoo()` predicate, where
+ // `ClosureType` represents some `TyClosure`. It can't
+ // possibly be referring to the current closure,
+ // because we haven't produced the `TyClosure` for
+ // this closure yet; this is exactly why the other
+ // code is looking for a self type of a unresolved
+ // inference variable.
+ ty::Predicate::ClosureKind(..) => None,
};
opt_trait_ref
.and_then(|trait_ref| self_type_matches_expected_vid(fcx, trait_ref, expected_vid)) |
|
Heh, oh, I see you already found that :) |
|
@soltanmm I think that's the right fix, see my code for a comment as to why (which we might as well include in the code) |
| let closure_span = infcx.tcx.map.span_if_local(closure_def_id).unwrap(); | ||
| let mut err = struct_span_err!( | ||
| infcx.tcx.sess, closure_span, E0524, | ||
| "the closure implements `{}` but not `{}`", |
There was a problem hiding this comment.
Nit: I think you should say something like "expected a closure that implements the {} trait, but this closure only implement {}". This fits our usual "expected, found" order better, and helps clarify that FnMut (etc) is a trait.
|
@soltanmm sorry for dropping off the notifications grid for a few days, had to finish up a few things, but anyway ping me when you have a revision that addresses the nit and which changes the |
|
@soltanmm I'd like very much to see this land -- do you have time to rebase/fix? I could also do it for you. |
|
@bors r+ |
|
📌 Commit de82fc4 has been approved by |
Replace consider_unification_despite_ambiguity with new obligation variant Is work towards #32730. Addresses part one of #32286. Addresses #24210 and #26046 to some degree. r? @nikomatsakis
Is work towards #32730. Addresses part one of #32286. Addresses #24210 and #26046 to some degree.
r? @nikomatsakis