rustc_typeck: unify expected return types with formal return types to propagate coercions through calls of generic functions. #20415
Conversation
|
r? @luqmana (rust_highfive has picked a reviewer for you, use r? to override) |
|
assigning myself as reviewer |
|
This looks really nice but not quite there. Look at what I wrote and let me know if you have questions. |
| fcx.infcx().probe(|_| { | ||
| use rustc::middle::infer::combine::Combine; | ||
| let trace = infer::TypeTrace::dummy(fcx.tcx()); | ||
| match fcx.infcx().equate(false, trace).tys(ret_ty, formal_ret_ty) { |
There was a problem hiding this comment.
One last comment: I'd rather not see the Combine trait used outside of infer. You can just invoke infer::mk_eqty() instead, it's equivalent and shorter.
There was a problem hiding this comment.
(Better yet, move it to a method :)
There was a problem hiding this comment.
It's not equivalent, what I have here could be with_eqty or resolve_with_eqty.
There was a problem hiding this comment.
in what way is it not equivalent? it looks equivalent to me.
There was a problem hiding this comment.
I execute code in the Ok case, before rolling back the snapshot, while mk_eqty would commit it. Unless you mean it's okay to have nested snapshots like this, in which case I have to wonder what the cost is.
There was a problem hiding this comment.
It is fine to have nested snapshots and the cost is nothing.
There was a problem hiding this comment.
Commit within a nested snapshot in particular is basically a no-op.
eddyb
force-pushed
the
unify-expected-return
branch
from
d23810a to
b1027b9
Compare
|
I've switched back to subtyping and used a modified version of |
| @@ -613,6 +613,36 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { | |||
| self.commit_unconditionally(move || self.try(move |_| f())) | |||
| } | |||
|
|
|||
| /// Execute `f` and commit the region bindings if successful | |||
There was a problem hiding this comment.
We probably want a more detailed comment here. "...commit only the region bindings if successful. The function f must be very careful not to leak any non-region variables that get created."
|
@eddyb ok I have to admit, I have my reservations but this looks pretty nice. r+ modulo nits (and maybe a cleaned up commit history?) |
| @@ -2750,7 +2773,25 @@ fn check_argument_types<'a, 'tcx>(fcx: &FnCtxt<'a, 'tcx>, | |||
| AutorefArgs::No => {} | |||
| } | |||
|
|
|||
| check_expr_coercable_to_type(fcx, &***arg, formal_ty); | |||
| // The special-cased logic below has three functions: | |||
| // 1. Provide as good of an expectated type as possible. | |||
…ap_to_option with to_option.
… propagate coercions through calls of generic functions.
eddyb
force-pushed
the
unify-expected-return
branch
from
a25307b to
4748721
Compare
rustc_typeck: unify expected return types with formal return types to propagate coercions through calls of generic functions. Reviewed-by: nikomatsakis
This allows passing
Some(free_fn)to something expectingOption<fn()>, dealing with most of the fallout from #19891.Fixes #20178.