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 type isn't inferred properly when a function returns a named closure #5983

Closed
catamorphism opened this issue Apr 21, 2013 · 11 comments
Closed
Labels
A-type-system Area: Type system P-low Low priority

Comments

@catamorphism
Copy link
Contributor

On IRC, jc-denton pointed out the following bug (original example at http://codepad.org/RFICaojj ):

fn memFib(n: int) -> @fn(int) -> int {
    let mut memo = @~[0, 1];
    let fib = |n: int| -> int {
        let mut result: int = - 1;
        if n < memo.len() as int {
            result = memo[n];
        } else {
            result = fib(n - 1) + fib(n - 2);
            memo.push(result);
        }
        return result;
    };
    return fib;
}

This yields a type error because fib's type is inferred to &fn(int) -> int. If you change the declaration of fib to have the explicit type @fn(int) -> int, it compiles.

Possibly related to #4929

@catamorphism
Copy link
Contributor Author

Reproduced with 2ff6b29 . Nominating for milestone 5, production-ready. (Note that you have to comment out the result = fib(n - 1) + fib(n - 2); line now to reproduce the original error.)

@emberian
Copy link
Member

emberian commented Aug 5, 2013

@catamorphism there's been some churn with closures. I got as far as:

fn memFib(n: int) -> @fn(int) -> int {
    let mut memo = @~[0, 1];
    let fib = |n: int| -> int {
        let mut result: int = - 1;
        if n < memo.len() as int {
            result = memo[n];
        } else {
//            result = fib(n - 1) + fib(n - 2);
            memo.push(result);
        }
        return result;
    };
    return fib;
}


fn main() {}

trying to reproduce, which gives:

foo.rs:13:11: 13:14 error: mismatched types: expected `@fn:'static(int) -> int` but found `&fn<no-bounds>(int) -> int` (expected @ closure, found & closure)
foo.rs:13     return fib;
                     ^~~

Is that the right error?

@graydon
Copy link
Contributor

graydon commented Aug 22, 2013

accepted for backwards-compatible milestone

@nikomatsakis
Copy link
Contributor

This will continue to be relevant even when @fn goes away
since we want to improve how || inference works. There must
be another open bug on this somewhere...

@catamorphism
Copy link
Contributor Author

Also see #2202 (the larger bug)

@nikomatsakis
Copy link
Contributor

Thinking more about this: the case I was thinking of is wanting to use || syntax for bare fns, in which case you might want to infer the ABI

@pcwalton
Copy link
Contributor

@nikomatsakis Do we want || for bare fns? I thought we were getting rid of all inference for || entirely.

@nikomatsakis
Copy link
Contributor

@pcwalton it can be pretty useful -- anyway even if we don't let || expand to a bare fn, we can't get rid of it entirely, still need to infer stuff like the bounds and so on. Places where || could be useful:

  • traits that act like fns (my thunk! macro would have required || to be inferable to a bare fn)
  • "contextless" callbacks, which are good because they can be sent freely between tasks and are POD etc
  • writing little callbacks to hand off to C

Shouldn't be too hard to make this infer properly, rather than based on the expected type, if there were a moment to spare on such trivialities.

Anyway, this feels like it's not a 1.0 thing, improving inference can only make more code work etc etc. It can't do any worse than it does today.

@aturon
Copy link
Member

aturon commented Sep 18, 2014

Nominated. I'm not sure if this issue should remain open, but it should not be "P-backcompat-lang". As @nikomatsakis says above:

"improving inference can only make more code work"

@pnkfelix
Copy link
Member

Assigning P-low, not 1.0 milestone.

@pnkfelix pnkfelix added P-low Low priority and removed I-nominated labels Sep 18, 2014
@pnkfelix
Copy link
Member

actually, closing as unactionable since the example is so out of date as to unhelpful

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-type-system Area: Type system P-low Low priority
Projects
None yet
Development

No branches or pull requests

7 participants