Auto merge of #42443 - tommyip:better_closure_msg, r=nikomatsakis

Better closure error message

Use tracked data introduced in #42196 to provide a better closure
error message by showing why a closure implements `FnOnce`.

```
error[E0525]: expected a closure that implements the `Fn` trait, but
this closure only implements `FnOnce`
 --> $DIR/issue_26046.rs:4:19
  |
4 |       let closure = move || {
  |  ___________________^
5 | |         vec
6 | |     };
  | |_____^
  |
note: closure is `FnOnce` because it moves the variable `vec` out of
its environment
 --> $DIR/issue_26046.rs:5:9
  |
5 |         vec
  |         ^^^

error: aborting due to previous error(s)
```

Fixes #26046

r? @nikomatsakis
cc @doomrobo

7 files changed, 116 insertions, 6 deletions

diff --git a/src/librustc/traits/error_reporting.rs b/src/librustc/traits/error_reporting.rs
index 049d5e488c9..c8e99c0354a 100644
--- a/src/librustc/traits/error_reporting.rs
+++ b/src/librustc/traits/error_reporting.rs
@@ -29,7 +29,7 @@ use hir::{self, intravisit, Local, Pat, Body};
 use hir::intravisit::{Visitor, NestedVisitorMap};
 use hir::map::NodeExpr;
 use hir::def_id::DefId;
-use infer::{self, InferCtxt};
+use infer::{self, InferCtxt, InferTables, InferTablesRef};
 use infer::type_variable::TypeVariableOrigin;
 use rustc::lint::builtin::EXTRA_REQUIREMENT_IN_IMPL;
 use std::fmt;
@@ -640,16 +640,44 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
                     ty::Predicate::ClosureKind(closure_def_id, kind) => {
                         let found_kind = self.closure_kind(closure_def_id).unwrap();
                         let closure_span = self.tcx.hir.span_if_local(closure_def_id).unwrap();
+                        let node_id = self.tcx.hir.as_local_node_id(closure_def_id).unwrap();
                         let mut err = struct_span_err!(
                             self.tcx.sess, closure_span, E0525,
                             "expected a closure that implements the `{}` trait, \
                                 but this closure only implements `{}`",
                             kind,
                             found_kind);
-                        err.span_note(
+
+                        err.span_label(
                             obligation.cause.span,
-                            &format!("the requirement to implement \
-                                        `{}` derives from here", kind));
+                            format!("the requirement to implement `{}` derives from here", kind));
+
+                        let infer_tables = match self.tables {
+                            InferTables::Interned(tables) =>
+                                Some(InferTablesRef::Interned(tables)),
+                            InferTables::InProgress(tables) =>
+                                Some(InferTablesRef::InProgress(tables.borrow())),
+                            InferTables::Missing => None,
+                        };
+
+                        // Additional context information explaining why the closure only implements
+                        // a particular trait.
+                        if let Some(tables) = infer_tables {
+                            match tables.closure_kinds.get(&node_id) {
+                                Some(&(ty::ClosureKind::FnOnce, Some((span, name)))) => {
+                                    err.span_note(span, &format!(
+                                        "closure is `FnOnce` because it moves the \
+                                         variable `{}` out of its environment", name));
+                                },
+                                Some(&(ty::ClosureKind::FnMut, Some((span, name)))) => {
+                                    err.span_note(span, &format!(
+                                        "closure is `FnMut` because it mutates the \
+                                         variable `{}` here", name));
+                                },
+                                _ => {}
+                            }
+                        }
+
                         err.emit();
                         return;
                     }
diff --git a/src/test/ui/closure_context/issue-26046-fn-mut.rs b/src/test/ui/closure_context/issue-26046-fn-mut.rs
new file mode 100644
index 00000000000..5ed7ace5437
--- /dev/null
+++ b/src/test/ui/closure_context/issue-26046-fn-mut.rs
@@ -0,0 +1,21 @@
+// Copyright 2017 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+fn foo() -> Box<Fn()> {
+    let num = 5;
+
+    let closure = || {
+        num += 1;
+    };
+
+    Box::new(closure)
+}
+
+fn main() {}
diff --git a/src/test/ui/closure_context/issue-26046-fn-mut.stderr b/src/test/ui/closure_context/issue-26046-fn-mut.stderr
new file mode 100644
index 00000000000..dbf702e4503
--- /dev/null
+++ b/src/test/ui/closure_context/issue-26046-fn-mut.stderr
@@ -0,0 +1,20 @@
+error[E0525]: expected a closure that implements the `Fn` trait, but this closure only implements `FnMut`
+  --> $DIR/issue-26046-fn-mut.rs:14:19
+   |
+14 |       let closure = || {
+   |  ___________________^
+15 | |         num += 1;
+16 | |     };
+   | |_____^
+17 | 
+18 |       Box::new(closure)
+   |       ----------------- the requirement to implement `Fn` derives from here
+   |
+note: closure is `FnMut` because it mutates the variable `num` here
+  --> $DIR/issue-26046-fn-mut.rs:15:9
+   |
+15 |         num += 1;
+   |         ^^^
+
+error: aborting due to previous error(s)
+
diff --git a/src/test/ui/closure_context/issue-26046-fn-once.rs b/src/test/ui/closure_context/issue-26046-fn-once.rs
new file mode 100644
index 00000000000..de06de530c6
--- /dev/null
+++ b/src/test/ui/closure_context/issue-26046-fn-once.rs
@@ -0,0 +1,21 @@
+// Copyright 2017 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+fn get_closure() -> Box<Fn() -> Vec<u8>> {
+    let vec = vec![1u8, 2u8];
+
+    let closure = move || {
+        vec
+    };
+
+    Box::new(closure)
+}
+
+fn main() {}
diff --git a/src/test/ui/closure_context/issue-26046-fn-once.stderr b/src/test/ui/closure_context/issue-26046-fn-once.stderr
new file mode 100644
index 00000000000..3ec3f0cc9aa
--- /dev/null
+++ b/src/test/ui/closure_context/issue-26046-fn-once.stderr
@@ -0,0 +1,20 @@
+error[E0525]: expected a closure that implements the `Fn` trait, but this closure only implements `FnOnce`
+  --> $DIR/issue-26046-fn-once.rs:14:19
+   |
+14 |       let closure = move || {
+   |  ___________________^
+15 | |         vec
+16 | |     };
+   | |_____^
+17 | 
+18 |       Box::new(closure)
+   |       ----------------- the requirement to implement `Fn` derives from here
+   |
+note: closure is `FnOnce` because it moves the variable `vec` out of its environment
+  --> $DIR/issue-26046-fn-once.rs:15:9
+   |
+15 |         vec
+   |         ^^^
+
+error: aborting due to previous error(s)
+
diff --git a/src/test/ui/fn_once-moved.rs b/src/test/ui/closure_context/issue-42065.rs
index 409964082f2..409964082f2 100644
--- a/src/test/ui/fn_once-moved.rs
+++ b/src/test/ui/closure_context/issue-42065.rs
diff --git a/src/test/ui/fn_once-moved.stderr b/src/test/ui/closure_context/issue-42065.stderr
index 27b7d91d1d4..5bbd372adb6 100644
--- a/src/test/ui/fn_once-moved.stderr
+++ b/src/test/ui/closure_context/issue-42065.stderr
@@ -1,5 +1,5 @@
 error[E0382]: use of moved value: `debug_dump_dict`
-  --> $DIR/fn_once-moved.rs:21:5
+  --> $DIR/issue-42065.rs:21:5
    |
 20 |     debug_dump_dict();
    |     --------------- value moved here
@@ -7,7 +7,7 @@ error[E0382]: use of moved value: `debug_dump_dict`
    |     ^^^^^^^^^^^^^^^ value used here after move
    |
 note: closure cannot be invoked more than once because it moves the variable `dict` out of its environment
-  --> $DIR/fn_once-moved.rs:16:29
+  --> $DIR/issue-42065.rs:16:29
    |
 16 |         for (key, value) in dict {
    |                             ^^^^