Skip to content

Commit

Permalink
Prefer explict closure sig types over expected ones
Browse files Browse the repository at this point in the history
  • Loading branch information
oli-obk committed Sep 14, 2022
1 parent a0d1df4 commit 7794ea5
Show file tree
Hide file tree
Showing 7 changed files with 87 additions and 30 deletions.
60 changes: 39 additions & 21 deletions compiler/rustc_typeck/src/check/closure.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,12 @@ struct ExpectedSig<'tcx> {
}

struct ClosureSignatures<'tcx> {
/// The signature users of the closure see.
bound_sig: ty::PolyFnSig<'tcx>,
/// The signature within the function body.
/// This mostly differs in the sense that lifetimes are now early bound and any
/// opaque types from the signature expectation are overriden in case there are
/// explicit hidden types written by the user in the closure signature.
liberated_sig: ty::FnSig<'tcx>,
}

Expand Down Expand Up @@ -444,18 +449,16 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
// Along the way, it also writes out entries for types that the user
// wrote into our typeck results, which are then later used by the privacy
// check.
match self.check_supplied_sig_against_expectation(
match self.merge_supplied_sig_with_expectation(
hir_id,
expr_def_id,
decl,
body,
&closure_sigs,
closure_sigs,
) {
Ok(infer_ok) => self.register_infer_ok_obligations(infer_ok),
Err(_) => return self.sig_of_closure_no_expectation(hir_id, expr_def_id, decl, body),
Err(_) => self.sig_of_closure_no_expectation(hir_id, expr_def_id, decl, body),
}

closure_sigs
}

fn sig_of_closure_with_mismatched_number_of_arguments(
Expand Down Expand Up @@ -497,21 +500,22 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
/// Enforce the user's types against the expectation. See
/// `sig_of_closure_with_expectation` for details on the overall
/// strategy.
fn check_supplied_sig_against_expectation(
#[instrument(level = "debug", skip(self, hir_id, expr_def_id, decl, body, expected_sigs))]
fn merge_supplied_sig_with_expectation(
&self,
hir_id: hir::HirId,
expr_def_id: DefId,
decl: &hir::FnDecl<'_>,
body: &hir::Body<'_>,
expected_sigs: &ClosureSignatures<'tcx>,
) -> InferResult<'tcx, ()> {
mut expected_sigs: ClosureSignatures<'tcx>,
) -> InferResult<'tcx, ClosureSignatures<'tcx>> {
// Get the signature S that the user gave.
//
// (See comment on `sig_of_closure_with_expectation` for the
// meaning of these letters.)
let supplied_sig = self.supplied_sig_of_closure(hir_id, expr_def_id, decl, body);

debug!("check_supplied_sig_against_expectation: supplied_sig={:?}", supplied_sig);
debug!(?supplied_sig);

// FIXME(#45727): As discussed in [this comment][c1], naively
// forcing equality here actually results in suboptimal error
Expand All @@ -529,23 +533,27 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
// [c2]: https://github.com/rust-lang/rust/pull/45072#issuecomment-341096796
self.commit_if_ok(|_| {
let mut all_obligations = vec![];
let inputs: Vec<_> = iter::zip(
decl.inputs,
supplied_sig.inputs().skip_binder(), // binder moved to (*) below
)
.map(|(hir_ty, &supplied_ty)| {
// Instantiate (this part of..) S to S', i.e., with fresh variables.
self.replace_bound_vars_with_fresh_vars(
hir_ty.span,
LateBoundRegionConversionTime::FnCall,
// (*) binder moved to here
supplied_sig.inputs().rebind(supplied_ty),
)
})
.collect();

// The liberated version of this signature should be a subtype
// of the liberated form of the expectation.
for ((hir_ty, &supplied_ty), expected_ty) in iter::zip(
iter::zip(
decl.inputs,
supplied_sig.inputs().skip_binder(), // binder moved to (*) below
),
iter::zip(decl.inputs, &inputs),
expected_sigs.liberated_sig.inputs(), // `liberated_sig` is E'.
) {
// Instantiate (this part of..) S to S', i.e., with fresh variables.
let supplied_ty = self.replace_bound_vars_with_fresh_vars(
hir_ty.span,
LateBoundRegionConversionTime::FnCall,
supplied_sig.inputs().rebind(supplied_ty),
); // recreated from (*) above

// Check that E' = S'.
let cause = self.misc(hir_ty.span);
let InferOk { value: (), obligations } =
Expand All @@ -564,7 +572,17 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
.eq(expected_sigs.liberated_sig.output(), supplied_output_ty)?;
all_obligations.extend(obligations);

Ok(InferOk { value: (), obligations: all_obligations })
let inputs = inputs.into_iter().map(|ty| self.resolve_vars_if_possible(ty));

expected_sigs.liberated_sig = self.tcx.mk_fn_sig(
inputs,
supplied_output_ty,
expected_sigs.liberated_sig.c_variadic,
hir::Unsafety::Normal,
Abi::RustCall,
);

Ok(InferOk { value: expected_sigs, obligations: all_obligations })
})
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ fn main::{closure#0}(_1: &[closure@main::{closure#0}], _2: &i32) -> &i32 {
StorageLive(_3); // scope 0 at $DIR/retag.rs:+1:13: +1:15
_3 = _2; // scope 0 at $DIR/retag.rs:+1:18: +1:19
Retag(_3); // scope 0 at $DIR/retag.rs:+1:18: +1:19
_0 = _2; // scope 1 at $DIR/retag.rs:+2:9: +2:10
_0 = &(*_2); // scope 1 at $DIR/retag.rs:+2:9: +2:10
Retag(_0); // scope 1 at $DIR/retag.rs:+2:9: +2:10
StorageDead(_3); // scope 0 at $DIR/retag.rs:+3:5: +3:6
return; // scope 0 at $DIR/retag.rs:+3:6: +3:6
Expand Down
12 changes: 6 additions & 6 deletions src/test/ui/closure-expected-type/expect-fn-supply-fn.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -25,26 +25,26 @@ error[E0308]: mismatched types
LL | with_closure_expecting_fn_with_free_region(|x: fn(&u32), y| {});
| ^ one type is more general than the other
|
= note: expected fn pointer `for<'r> fn(&'r u32)`
found fn pointer `fn(&u32)`
= note: expected fn pointer `fn(&u32)`
found fn pointer `for<'r> fn(&'r u32)`

error[E0308]: mismatched types
--> $DIR/expect-fn-supply-fn.rs:39:50
|
LL | with_closure_expecting_fn_with_bound_region(|x: fn(&'x u32), y| {});
| ^ one type is more general than the other
|
= note: expected fn pointer `fn(&'x u32)`
found fn pointer `for<'r> fn(&'r u32)`
= note: expected fn pointer `for<'r> fn(&'r u32)`
found fn pointer `fn(&u32)`

error[E0308]: mismatched types
--> $DIR/expect-fn-supply-fn.rs:48:50
|
LL | with_closure_expecting_fn_with_bound_region(|x: Foo<'_>, y| {
| ^ one type is more general than the other
|
= note: expected fn pointer `fn(&u32)`
found fn pointer `for<'r> fn(&'r u32)`
= note: expected fn pointer `for<'r> fn(&'r u32)`
found fn pointer `fn(&u32)`

error: aborting due to 5 previous errors

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ LL | with_closure(|x: u32, y| {});
|
help: consider giving this closure parameter an explicit type
|
LL | with_closure(|x: u32, y: B| {});
LL | with_closure(|x: u32, y: _| {});
| +++

error: aborting due to previous error
Expand Down
16 changes: 16 additions & 0 deletions src/test/ui/type-alias-impl-trait/closure_args.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// check-pass

// regression test for https://github.com/rust-lang/rust/issues/100800

#![feature(type_alias_impl_trait)]

trait Anything {}
impl<T> Anything for T {}
type Input = impl Anything;
fn run<F: FnOnce(Input) -> ()>(f: F, i: Input) {
f(i);
}

fn main() {
run(|x: u32| {println!("{x}");}, 0);
}
23 changes: 23 additions & 0 deletions src/test/ui/type-alias-impl-trait/closure_args2.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
// run-pass

#![feature(type_alias_impl_trait)]

trait Foo {
// This was reachable in https://github.com/rust-lang/rust/issues/100800
fn foo(&self) { unreachable!() }
}
impl<T> Foo for T {}

struct B;
impl B {
fn foo(&self) {}
}

type Input = impl Foo;
fn run1<F: FnOnce(Input)>(f: F, i: Input) {f(i)}
fn run2<F: FnOnce(B)>(f: F, i: B) {f(i)}

fn main() {
run1(|x: B| {x.foo()}, B);
run2(|x: B| {x.foo()}, B);
}
2 changes: 1 addition & 1 deletion src/test/ui/type-alias-impl-trait/issue-60371.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ error[E0277]: the trait bound `(): Bug` is not satisfied
--> $DIR/issue-60371.rs:10:40
|
LL | const FUN: fn() -> Self::Item = || ();
| ^ the trait `Bug` is not implemented for `()`
| ^^ the trait `Bug` is not implemented for `()`
|
= help: the trait `Bug` is implemented for `&()`

Expand Down

0 comments on commit 7794ea5

Please sign in to comment.