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

Closure inference is based on expected type #2190

Closed
brson opened this issue Apr 12, 2012 · 22 comments
Closed

Closure inference is based on expected type #2190

brson opened this issue Apr 12, 2012 · 22 comments
Assignees
Labels
A-typesystem Area: The type system

Comments

@brson
Copy link
Contributor

brson commented Apr 12, 2012

UPDATE: I updated the title to reflect the root problem. "Closure inference" refers to detailing the specifics of a closure type, such as its bounds and so forth. Ideally, only the argument types would derive from the expected type (with a fallback to fresh type variables).

original issue report follows:

type t = {
    f: fn~()
};

fn main() {
    let _t: t = { f: {||()} };
}
../src/test/run-pass/test.rs:6:16: 6:29 error: mismatched types: expected `t` but found `{f: fn@()}` (in field `f`, closure protocol mismatch (fn~ vs fn@))
../src/test/run-pass/test.rs:6     let _t: t = { f: {||()} };
@nikomatsakis
Copy link
Contributor

There are two separate problems here. The first is that we do not push the expected type through records into their fields. The second is that our inference of the closure type is based purely on the expected type rather than generating some sort of variable that can later be unified. I think we should fix both (but probably the first one first).

@brson
Copy link
Contributor Author

brson commented Apr 12, 2012

FWIW this actually demonstrates a second problem with closure inference - the nil in {||()} should be inferred.

@ghost ghost assigned nikomatsakis Apr 12, 2012
@bblum
Copy link
Contributor

bblum commented Jul 17, 2012

Don't think this has to do with records. The closure here infers as a fn@ if the ret is removed, but works with the ret present.

const generations: uint = 1024+256+128+49;

fn child_no(x: uint) -> fn~() {
    ret || {
        if x < generations {
            task::spawn(child_no(x+1));
        }
    }
}

fn main() {
    task::spawn(child_no(0));
}

@nikomatsakis
Copy link
Contributor

Actually the original bug may have been fixed since marijn did some work towards pushing expected types through records. I think the problem bblum points out is a separate but related issue.

@nikomatsakis
Copy link
Contributor

Indeed, what bblum points out (as brson pointed out earlier but I didn't understand it) is a problem that the "expected type" does not appear to be propagated to the expr in tail position in a record.

@nikomatsakis
Copy link
Contributor

sorry, tail position in a block

@bblum
Copy link
Contributor

bblum commented Aug 1, 2012

This seems more extensive than just records - I'm running into trouble inferring closures inside of options.

This works:

fn foo(f: fn&()) { f() }
fn bar() {}
fn main() { foo(||bar()); }

Now I try to put the closure in an option. It works if I just use the name bar:

fn foo(f: option<fn&()>) { f.iter(|x|x()) }
fn bar() {}
fn main() { foo(some(bar)); }

But if I use the bars syntax, it infers that as a fn@, and dies:

fn foo(f: option<fn&()>) { f.iter(|x|x()) }
fn bar() {}
fn main() { foo(some(||bar())); }

@nikomatsakis
Copy link
Contributor

Give people an inch and they want a mile. :) Closure inference is broken in many ways, because it's a quick hack based on the expected type. I hope to replace with something more principled along the lines of what we do for integer literal inference though.

@bblum
Copy link
Contributor

bblum commented Aug 2, 2012

(that commit has a case of the code in my above commit; tagged with a fixme. should be easy to get rid of the fixme after this bug.)

@catamorphism
Copy link
Contributor

Reproduced as of 3edccc3

catamorphism added a commit that referenced this issue Nov 15, 2012
@pcwalton
Copy link
Contributor

I don't believe this is backwards incompatible, renominating.

@graydon
Copy link
Contributor

graydon commented May 30, 2013

blocked on resolution of #6308

@graydon
Copy link
Contributor

graydon commented May 30, 2013

sub-bug of #6834

@graydon
Copy link
Contributor

graydon commented May 30, 2013

just a bug, removing milestone/nomination.

@nikomatsakis
Copy link
Contributor

Related to #6834

@catamorphism
Copy link
Contributor

Visiting for triage. Sub-bug of #6834, therefore doesn't need to be on a milestone itself.

@thestinger
Copy link
Contributor

Closure inference is gone, because there are only borrowed stack closures now.

@nikomatsakis
Copy link
Contributor

Not so. There are still things we might need to infer, such as closure bounds and so on.

@nikomatsakis nikomatsakis reopened this Dec 16, 2013
@nikomatsakis
Copy link
Contributor

I updated the title to reflect the root problem.

@steveklabnik
Copy link
Member

@nikomatsakis is this issue still relevant with all of the changes to closures?

@nikomatsakis
Copy link
Contributor

Not really. The current state roughly corresponds with what I described as the "ideal" scenario above, though I'd not call it ideal ideal. ;)

@steveklabnik
Copy link
Member

😄

bors added a commit to rust-lang-ci/rust that referenced this issue Sep 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-typesystem Area: The type system
Projects
None yet
Development

No branches or pull requests

8 participants