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

doc: Be more specific about how to box closures #26014

Closed
wants to merge 1 commit into from

Conversation

richo
Copy link
Contributor

@richo richo commented Jun 4, 2015

The old error was still confusing, when you try boxing you still get the same error: eg, http://is.gd/yDwxPF

@rust-highfive
Copy link
Collaborator

r? @brson

(rust_highfive has picked a reviewer for you, use r? to override)

@richo richo force-pushed the box-closure-advice branch from 0286bc3 to 839170f Compare June 4, 2015 20:47
@brson
Copy link
Contributor

brson commented Jun 4, 2015

This revision is less precise than the former, since the former tells you two options, and latter just one. Creating &Trait is not 'boxing' it, since boxing means moving something onto the heap.

I also don't quite see how this description makes the behavior in your example, where boxing the closure doesn't work, less confusing. If I saw the new message I would probably wrote what you wrote that doesn't work.

So I think we can probably do better still. r? @steveklabnik

@rust-highfive rust-highfive assigned steveklabnik and unassigned brson Jun 4, 2015
@richo
Copy link
Contributor Author

richo commented Jun 4, 2015

So to be clear, the solution I wound up with here was that:

fn main() {
    let f = match 1 {
        1 => Box::new(|c| c + 1),
        _ => Box::new(|c| c - 1),
    };

    println!("{}", f(1));
}

Doesn't work but:

fn main() {
    let f: Box<Fn(i32) -> i32> = match 1 {
        1 => Box::new(|c| c + 1),
        _ => Box::new(|c| c - 1),
    };

    println!("{}", f(1));
}

Does, eg, it needed to be boxed and coerced to a trait object. It was unclear to me that I could coerce it as a reference, since that would intuitively seem like trying to put DST on the stack.

@steveklabnik
Copy link
Member

agree with @brson here, these are two options that are being reduced to one. Boxing is one way of making a trait object, &Trait is another

@richo
Copy link
Contributor Author

richo commented Jun 4, 2015

So, ignoring the content of this PR, my gripe is that the help told me to box them, and then after I put them all in boxes it told me the same thing. I'm not really fussed about this specific wording, but I think it should be more explicit.

@steveklabnik
Copy link
Member

yeah, I'm also not saying this error message is great, for sure

@bors
Copy link
Contributor

bors commented Jun 27, 2015

☔ The latest upstream changes (presumably #26575) made this pull request unmergeable. Please resolve the merge conflicts.

@richo richo closed this Jun 27, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants