Skip to content
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

Cannot infer closure type with higher-ranked lifetimes inside Box::new #20841

Open
mzabaluev opened this issue Jan 10, 2015 · 18 comments
Open

Cannot infer closure type with higher-ranked lifetimes inside Box::new #20841

mzabaluev opened this issue Jan 10, 2015 · 18 comments
Labels
A-closures Area: Closures (`|…| { … }`) A-type-system Area: Type system C-enhancement Category: An issue proposing an enhancement or a PR with one. E-medium Call for participation: Medium difficulty. Experience needed to fix: Intermediate. P-low Low priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-lang Relevant to the language team, which will review and decide on the PR/issue.

Comments

@mzabaluev
Copy link
Contributor

In the code below, type inference works in the first line of main(), but fails in the second one. Note also that the "kind" of the closure has to be given with explicit syntax when inside Box::new().

fn do_async<F>(_cb: Box<F>) where F: FnOnce(&i32) {
}

fn do_async_unboxed<F>(cb: F) where F: FnOnce(&i32) {
    do_async(Box::new(cb))
}

fn main() {
    do_async_unboxed(|x| { println!("{}", *x); });
    do_async(Box::new(|: x| { println!("{}", *x); }));
}
mzabaluev added a commit to gi-rust/grust that referenced this issue Jan 10, 2015
Convenience trumps performance: it currently is hard for the
compiler to correctly infer the types of closure's arguments
when the closure is constructed inside Box::new.

The compiler issue: rust-lang/rust#20841
@kmcallister kmcallister added A-type-system Area: Type system A-closures Area: Closures (`|…| { … }`) labels Jan 11, 2015
@steveklabnik
Copy link
Member

Triage: updated code

fn do_async<F>(_cb: Box<F>) where F: FnOnce(&i32) {
}

fn do_async_unboxed<F>(cb: F) where F: FnOnce(&i32) {
    do_async(Box::new(cb))
}

fn main() {
    do_async_unboxed(|x| { println!("{}", *x); });   // Ok
    do_async(Box::new(|x| { println!("{}", *x); }));  // ERROR the type of this value must be known in this context
}

@arielb1 arielb1 added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Nov 22, 2015
@arielb1
Copy link
Contributor

arielb1 commented Nov 22, 2015

From my investigation, the problem is that we don't relate the return type of Wrapper/Box::new to the argument type of api, because we are afraid there might be a coercion involved. This means the bound information is not passed to the closure. We could try to be better here - cc @nikomatsakis @eddyb.

@arielb1
Copy link
Contributor

arielb1 commented Nov 22, 2015

Nominating as this is makes some API patterns horribly non-ergonomic.

@nikomatsakis nikomatsakis added the T-lang Relevant to the language team, which will review and decide on the PR/issue. label Dec 1, 2015
@nikomatsakis
Copy link
Contributor

triage: P-medium

@nikomatsakis
Copy link
Contributor

I agree we could probably do better here. I'm not sure what precisely is going on or the current state of this coercion code though.

@nikomatsakis
Copy link
Contributor

triage: P-medium

@rust-highfive rust-highfive added P-medium Medium priority and removed I-nominated labels Dec 17, 2015
@eddyb
Copy link
Member

eddyb commented Jan 8, 2016

// Works:
fn foo<F: Fn(f32) -> f32>(_: F) {}
foo(|x| x.sin())

// Why it works: the inference variables look like this:
foo::<$F>((|x: $A| x.sin()): $F)
// Because $F: Fn(f32) -> f32, we can deduce $A: f32.
// Doesn't work:
fn bar<F: Fn(f32) -> f32>(_: Option<F>) {}
bar(Some(|x| x.sin()))

// Why it doesn't work: the inference variables look like this:
bar::<$F>(Some::<$T>((|x: $A| x.sin()): $T): Option<$F>)
// Because Some: T -> Option<T>, we know that $T = $F.
// But they are both inference variables, so we don't replace
// one with the other, and $T: Fn(f32) -> f32 doesn't exist.

If my analysis is correct, it should be possible to fix this bug by computing inference variable equivalence via union-find instead of using simple equality.

@arielb1
Copy link
Contributor

arielb1 commented Jan 10, 2016

@eddyb

Except that the variables are only related via a coercion.

@arielb1
Copy link
Contributor

arielb1 commented Jan 10, 2016

That looks like a case of things being so loosely coupled they fall apart.

@eddyb
Copy link
Member

eddyb commented Jan 10, 2016

@arielb1 There's no coercion magic going on, this is the "expected" type being propagated down. Coercions are applied after the type is known. Try replacing the strict equality with equivalence and it should just start working.

@arielb1
Copy link
Contributor

arielb1 commented Jan 10, 2016

I mean, we don't equate $T and $F, but rather we will coerce them after the call is type-checked.

@eddyb
Copy link
Member

eddyb commented Jan 10, 2016

@arielb1 We do to obtain the expected type for the arguments of a call.

@arielb1
Copy link
Contributor

arielb1 commented Jan 10, 2016

@eddyb

That would be a subtype relation, which is destroyed by the commit_regions_if_ok call.

@eddyb
Copy link
Member

eddyb commented Jan 10, 2016

@arielb1 Let's try to be clearer using pseudosyntax:

// Initial expression.
bar(Some((|x| x.sin())))
// Add type variables for bar.
bar::<$F>(Some((|x| x.sin())))
// Propagate expected type from signature of bar.
bar::<$F>(Some((|x| x.sin())) expects Option<$F>)
// Add type variables for Some.
bar::<$F>(Some::<$T>((|x| x.sin())))
// Propagate expected type from signature of Some.
bar::<$F>(Some::<$T>((|x| x.sin()) expects $T) expects Option<$F>)

Well, that's embarrassing, turns out I was being unreasonable. A relationship between $T and $F cannot exist before assigning the actual types.

Presumably it might be possible to end up with:

bar::<$F>(Some::<$T>((|x| x.sin()) expects $F) expects Option<$F>)

What confused me was that the following snippet works seemed to work when I tried it initially:

fn foo<F: Fn(f32) -> f32>(_: F) {}
fn id<T>(x: T) -> T {x}
foo(id(|x| x.sin()))

Given that it doesn't actually work, it's a better testcase.
If we end up with:

foo::<$F>(id::<$T>((|x| x.sin()) expects $F) expects $F)

Then that would work, but consider:

fn foo<F>(_: F) {}
fn id<T: Fn(f32) -> f32>(x: T) -> T {x}
foo(id(|x| x.sin()))

Where $T has the Fn bound but $F doesn't.

@arielb1 @nikomatsakis Would it make sense to have a set of inference variable "assignments" used for the expected type?
So that $T = $F is known but doesn't affect code which doesn't explicitly deal with expected types, as we only need to know that X: Fn(f32) -> f32 applies to the closure whether X is $F or $T.

@arielb1
Copy link
Contributor

arielb1 commented Jan 11, 2016

@eddyb

That might fix this, but it may cause confusing issues if a coercion were actually to occur. Maybe have hints solely for closure inference? That looks like a hack.

@eddyb
Copy link
Member

eddyb commented Jan 11, 2016

@arielb1 Nothing other than closure inference looks at bounds which refer to the expected type, AFAICT.

@brson brson added P-low Low priority E-medium Call for participation: Medium difficulty. Experience needed to fix: Intermediate. and removed P-medium Medium priority labels Aug 25, 2016
@brson brson added the C-enhancement Category: An issue proposing an enhancement or a PR with one. label Aug 25, 2016
@steveklabnik
Copy link
Member

Triage: not aware of any movement on this issue

@Spoonbender
Copy link

Triage: still reproduces on 2021 edition, tested using rustc 1.59.0 (9d1b2106e 2022-02-23)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-closures Area: Closures (`|…| { … }`) A-type-system Area: Type system C-enhancement Category: An issue proposing an enhancement or a PR with one. E-medium Call for participation: Medium difficulty. Experience needed to fix: Intermediate. P-low Low priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

9 participants