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 errors should explain **why** a closure is FnOnce etc #42065

Closed
nikomatsakis opened this issue May 17, 2017 · 2 comments
Closed

closure errors should explain **why** a closure is FnOnce etc #42065

nikomatsakis opened this issue May 17, 2017 · 2 comments
Labels
A-diagnostics Area: Messages for errors, warnings, and lints E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@nikomatsakis
Copy link
Contributor

In #41772 we were discussing improving how we could improve closure error messages by identifying an upvar that forced the closure to be FnOnce; this may apply to other messages too.

The idea is that the src/test/ui/fn_once-moved.rs test (shown below):

use std::collections::HashMap;

fn main() {
    let dict: HashMap<i32, i32> = HashMap::new();
    let debug_dump_dict = || {
        for (key, value) in dict {
            println!("{:?} - {:?}", key, value);
        }
    };
    debug_dump_dict();
    debug_dump_dict();
}

ought to give an error like:

error[E0382]: use of moved value: `debug_dump_dict`
  --> $DIR/fn_once-moved.rs:21:5
   |
16 |         for (key, value) in dict {
   |                             ^^^^ dict moved here
20 |     debug_dump_dict();
   |     --------------- value moved here
21 |     debug_dump_dict();
   |     ^^^^^^^^^^^^^^^ value used here after move
   |
   = help: closure cannot be invoked twice because it moves the variable dict out of its environment

The compiler will, however, require some refactoring to track this information.

Rough mentoring instructions

The part of the code you have to look at a bit is in librustc_typeck/check/upvar.rs. This code basically walks the AST and observes what happens, gradually "bumping up" the closure-kind to be something more specific based on what it observes. Ultimately, these bumps occur by calls to adjust_closure_kind(). Right now, that method takes zero context as to why the bump occurred.

We would want to extend it with a span, I think. Then we could extend the temp_closure_kinds table to carry not only the closure kind but also the span that caused us to change it (and maybe a bit more info). We'd have to trace back to the callers to encode this span.

We'll also have to change the closure_kinds field of TypeckTables so that it maps not only to a ClosureKind but also the Span (and whatever else) that we are tracing through. This is a bit of a shame, since this info is only relevant in the local crate; I'm wondering then if we want to consider storing the span in some other place. But TypeckTables is realy going to be the most convenient place, so I think that's ultimately where I'd put it.

@nikomatsakis nikomatsakis added A-diagnostics Area: Messages for errors, warnings, and lints E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels May 17, 2017
@tommyip
Copy link
Contributor

tommyip commented May 17, 2017

I would like to have a go at this.

@jonhoo
Copy link
Contributor

jonhoo commented May 18, 2017

Not entirely sure, but I believe this could also be tied in to #41790.

Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this issue May 31, 2017
…matsakis

Explain why a closure is `FnOnce` in closure errors.

Issue: rust-lang#42065
@nikomatsakis Am I going the right direction with this?

~~I am stuck in a few bits:~~
~~1. How to trace the code to get the upvar instead of the original variable's span?~~
~~2. How to find the node id of the upvar where the move occured?~~
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

3 participants