Skip to content

Explain why a closure is FnOnce in closure errors. #42196

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

Merged
merged 5 commits into from
May 31, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion src/librustc/infer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1682,7 +1682,11 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
{
if let InferTables::InProgress(tables) = self.tables {
if let Some(id) = self.tcx.hir.as_local_node_id(def_id) {
return tables.borrow().closure_kinds.get(&id).cloned();
return tables.borrow()
.closure_kinds
.get(&id)
.cloned()
.map(|(kind, _)| kind);
}
}

Expand Down
6 changes: 4 additions & 2 deletions src/librustc/ty/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ use syntax::abi;
use syntax::ast::{self, Name, NodeId};
use syntax::attr;
use syntax::symbol::{Symbol, keywords};
use syntax_pos::Span;

use hir;

Expand Down Expand Up @@ -229,8 +230,9 @@ pub struct TypeckTables<'tcx> {
/// Records the type of each closure.
pub closure_tys: NodeMap<ty::PolyFnSig<'tcx>>,

/// Records the kind of each closure.
pub closure_kinds: NodeMap<ty::ClosureKind>,
/// Records the kind of each closure and the span and name of the variable
/// that caused the closure to be this kind.
pub closure_kinds: NodeMap<(ty::ClosureKind, Option<(Span, ast::Name)>)>,

/// For each fn, records the "liberated" types of its arguments
/// and return type. Liberated means that all bound regions
Expand Down
14 changes: 9 additions & 5 deletions src/librustc_borrowck/borrowck/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,6 @@ use rustc::middle::free_region::RegionRelations;
use rustc::ty::{self, TyCtxt};
use rustc::ty::maps::Providers;

use syntax_pos::DUMMY_SP;

use std::fmt;
use std::rc::Rc;
use std::hash::{Hash, Hasher};
Expand Down Expand Up @@ -594,9 +592,15 @@ impl<'a, 'tcx> BorrowckCtxt<'a, 'tcx> {
verb, msg, nl);
let need_note = match lp.ty.sty {
ty::TypeVariants::TyClosure(id, _) => {
if let Ok(ty::ClosureKind::FnOnce) =
ty::queries::closure_kind::try_get(self.tcx, DUMMY_SP, id) {
err.help("closure was moved because it only implements `FnOnce`");
let node_id = self.tcx.hir.as_local_node_id(id).unwrap();
if let Some(&(ty::ClosureKind::FnOnce, Some((span, name)))) =
self.tables.closure_kinds.get(&node_id)
{
err.span_note(span, &format!(
"closure cannot be invoked more than once because \
it moves the variable `{}` out of its environment",
name
));
false
} else {
true
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_typeck/check/closure.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
self.tables.borrow_mut().closure_tys.insert(expr.id, sig);
match opt_kind {
Some(kind) => {
self.tables.borrow_mut().closure_kinds.insert(expr.id, kind);
self.tables.borrow_mut().closure_kinds.insert(expr.id, (kind, None));
}
None => {}
}
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_typeck/check/method/probe.rs
Original file line number Diff line number Diff line change
Expand Up @@ -802,7 +802,7 @@ impl<'a, 'gcx, 'tcx> ProbeContext<'a, 'gcx, 'tcx> {

let closure_kinds = &self.tables.borrow().closure_kinds;
let closure_kind = match closure_kinds.get(&closure_id) {
Some(&k) => k,
Some(&(k, _)) => k,
None => {
return Err(MethodError::ClosureAmbiguity(trait_def_id));
}
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_typeck/check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -702,7 +702,7 @@ fn closure_kind<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
def_id: DefId)
-> ty::ClosureKind {
let node_id = tcx.hir.as_local_node_id(def_id).unwrap();
tcx.typeck_tables_of(def_id).closure_kinds[&node_id]
tcx.typeck_tables_of(def_id).closure_kinds[&node_id].0
}

fn adt_destructor<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
Expand Down
57 changes: 38 additions & 19 deletions src/librustc_typeck/check/upvar.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {

struct SeedBorrowKind<'a, 'gcx: 'a+'tcx, 'tcx: 'a> {
fcx: &'a FnCtxt<'a, 'gcx, 'tcx>,
temp_closure_kinds: NodeMap<ty::ClosureKind>,
temp_closure_kinds: NodeMap<(ty::ClosureKind, Option<(Span, ast::Name)>)>,
}

impl<'a, 'gcx, 'tcx> Visitor<'gcx> for SeedBorrowKind<'a, 'gcx, 'tcx> {
Expand Down Expand Up @@ -107,7 +107,7 @@ impl<'a, 'gcx, 'tcx> SeedBorrowKind<'a, 'gcx, 'tcx> {
capture_clause: hir::CaptureClause)
{
if !self.fcx.tables.borrow().closure_kinds.contains_key(&expr.id) {
self.temp_closure_kinds.insert(expr.id, ty::ClosureKind::Fn);
self.temp_closure_kinds.insert(expr.id, (ty::ClosureKind::Fn, None));
debug!("check_closure: adding closure {:?} as Fn", expr.id);
}

Expand Down Expand Up @@ -143,12 +143,12 @@ impl<'a, 'gcx, 'tcx> SeedBorrowKind<'a, 'gcx, 'tcx> {

struct AdjustBorrowKind<'a, 'gcx: 'a+'tcx, 'tcx: 'a> {
fcx: &'a FnCtxt<'a, 'gcx, 'tcx>,
temp_closure_kinds: NodeMap<ty::ClosureKind>,
temp_closure_kinds: NodeMap<(ty::ClosureKind, Option<(Span, ast::Name)>)>,
}

impl<'a, 'gcx, 'tcx> AdjustBorrowKind<'a, 'gcx, 'tcx> {
fn new(fcx: &'a FnCtxt<'a, 'gcx, 'tcx>,
temp_closure_kinds: NodeMap<ty::ClosureKind>)
temp_closure_kinds: NodeMap<(ty::ClosureKind, Option<(Span, ast::Name)>)>)
-> AdjustBorrowKind<'a, 'gcx, 'tcx> {
AdjustBorrowKind { fcx: fcx, temp_closure_kinds: temp_closure_kinds }
}
Expand Down Expand Up @@ -211,8 +211,8 @@ impl<'a, 'gcx, 'tcx> AdjustBorrowKind<'a, 'gcx, 'tcx> {

// If we are also inferred the closure kind here, update the
// main table and process any deferred resolutions.
if let Some(&kind) = self.temp_closure_kinds.get(&id) {
self.fcx.tables.borrow_mut().closure_kinds.insert(id, kind);
if let Some(&(kind, context)) = self.temp_closure_kinds.get(&id) {
self.fcx.tables.borrow_mut().closure_kinds.insert(id, (kind, context));
let closure_def_id = self.fcx.tcx.hir.local_def_id(id);
debug!("closure_kind({:?}) = {:?}", closure_def_id, kind);

Expand Down Expand Up @@ -272,6 +272,8 @@ impl<'a, 'gcx, 'tcx> AdjustBorrowKind<'a, 'gcx, 'tcx> {
euv::Move(_) => { }
}

let tcx = self.fcx.tcx;

// watch out for a move of the deref of a borrowed pointer;
// for that to be legal, the upvar would have to be borrowed
// by value instead
Expand All @@ -289,7 +291,9 @@ impl<'a, 'gcx, 'tcx> AdjustBorrowKind<'a, 'gcx, 'tcx> {

// to move out of an upvar, this must be a FnOnce closure
self.adjust_closure_kind(upvar_id.closure_expr_id,
ty::ClosureKind::FnOnce);
ty::ClosureKind::FnOnce,
guarantor.span,
tcx.hir.name(upvar_id.var_id));

let upvar_capture_map =
&mut self.fcx.tables.borrow_mut().upvar_capture_map;
Expand All @@ -303,7 +307,9 @@ impl<'a, 'gcx, 'tcx> AdjustBorrowKind<'a, 'gcx, 'tcx> {
// to be a FnOnce closure to permit moves out
// of the environment.
self.adjust_closure_kind(upvar_id.closure_expr_id,
ty::ClosureKind::FnOnce);
ty::ClosureKind::FnOnce,
guarantor.span,
tcx.hir.name(upvar_id.var_id));
}
mc::NoteNone => {
}
Expand Down Expand Up @@ -331,7 +337,7 @@ impl<'a, 'gcx, 'tcx> AdjustBorrowKind<'a, 'gcx, 'tcx> {

Categorization::Deref(base, _, mc::BorrowedPtr(..)) |
Categorization::Deref(base, _, mc::Implicit(..)) => {
if !self.try_adjust_upvar_deref(&cmt.note, ty::MutBorrow) {
if !self.try_adjust_upvar_deref(cmt, ty::MutBorrow) {
// assignment to deref of an `&mut`
// borrowed pointer implies that the
// pointer itself must be unique, but not
Expand Down Expand Up @@ -365,7 +371,7 @@ impl<'a, 'gcx, 'tcx> AdjustBorrowKind<'a, 'gcx, 'tcx> {

Categorization::Deref(base, _, mc::BorrowedPtr(..)) |
Categorization::Deref(base, _, mc::Implicit(..)) => {
if !self.try_adjust_upvar_deref(&cmt.note, ty::UniqueImmBorrow) {
if !self.try_adjust_upvar_deref(cmt, ty::UniqueImmBorrow) {
// for a borrowed pointer to be unique, its
// base must be unique
self.adjust_upvar_borrow_kind_for_unique(base);
Expand All @@ -382,7 +388,7 @@ impl<'a, 'gcx, 'tcx> AdjustBorrowKind<'a, 'gcx, 'tcx> {
}

fn try_adjust_upvar_deref(&mut self,
note: &mc::Note,
cmt: mc::cmt<'tcx>,
borrow_kind: ty::BorrowKind)
-> bool
{
Expand All @@ -394,7 +400,9 @@ impl<'a, 'gcx, 'tcx> AdjustBorrowKind<'a, 'gcx, 'tcx> {
ty::ImmBorrow => false,
});

match *note {
let tcx = self.fcx.tcx;

match cmt.note {
mc::NoteUpvarRef(upvar_id) => {
// if this is an implicit deref of an
// upvar, then we need to modify the
Expand All @@ -407,15 +415,21 @@ impl<'a, 'gcx, 'tcx> AdjustBorrowKind<'a, 'gcx, 'tcx> {
}

// also need to be in an FnMut closure since this is not an ImmBorrow
self.adjust_closure_kind(upvar_id.closure_expr_id, ty::ClosureKind::FnMut);
self.adjust_closure_kind(upvar_id.closure_expr_id,
ty::ClosureKind::FnMut,
cmt.span,
tcx.hir.name(upvar_id.var_id));

true
}
mc::NoteClosureEnv(upvar_id) => {
// this kind of deref occurs in a `move` closure, or
// for a by-value upvar; in either case, to mutate an
// upvar, we need to be an FnMut closure
self.adjust_closure_kind(upvar_id.closure_expr_id, ty::ClosureKind::FnMut);
self.adjust_closure_kind(upvar_id.closure_expr_id,
ty::ClosureKind::FnMut,
cmt.span,
tcx.hir.name(upvar_id.var_id));

true
}
Expand Down Expand Up @@ -462,11 +476,13 @@ impl<'a, 'gcx, 'tcx> AdjustBorrowKind<'a, 'gcx, 'tcx> {

fn adjust_closure_kind(&mut self,
closure_id: ast::NodeId,
new_kind: ty::ClosureKind) {
debug!("adjust_closure_kind(closure_id={}, new_kind={:?})",
closure_id, new_kind);
new_kind: ty::ClosureKind,
upvar_span: Span,
var_name: ast::Name) {
debug!("adjust_closure_kind(closure_id={}, new_kind={:?}, upvar_span={:?}, var_name={})",
closure_id, new_kind, upvar_span, var_name);

if let Some(&existing_kind) = self.temp_closure_kinds.get(&closure_id) {
if let Some(&(existing_kind, _)) = self.temp_closure_kinds.get(&closure_id) {
debug!("adjust_closure_kind: closure_id={}, existing_kind={:?}, new_kind={:?}",
closure_id, existing_kind, new_kind);

Expand All @@ -482,7 +498,10 @@ impl<'a, 'gcx, 'tcx> AdjustBorrowKind<'a, 'gcx, 'tcx> {
(ty::ClosureKind::Fn, ty::ClosureKind::FnOnce) |
(ty::ClosureKind::FnMut, ty::ClosureKind::FnOnce) => {
// new kind is stronger than the old kind
self.temp_closure_kinds.insert(closure_id, new_kind);
self.temp_closure_kinds.insert(
closure_id,
(new_kind, Some((upvar_span, var_name)))
);
}
}
}
Expand Down
3 changes: 2 additions & 1 deletion src/test/ui/fn_once-moved.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,5 +20,6 @@ fn main() {
debug_dump_dict();
debug_dump_dict();
//~^ ERROR use of moved value: `debug_dump_dict`
//~| NOTE closure was moved because it only implements `FnOnce`
//~| NOTE closure cannot be invoked more than once because it moves the
//~| variable `dict` out of its environment
}
6 changes: 5 additions & 1 deletion src/test/ui/fn_once-moved.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,11 @@ error[E0382]: use of moved value: `debug_dump_dict`
21 | debug_dump_dict();
| ^^^^^^^^^^^^^^^ value used here after move
|
= help: closure was moved because it only implements `FnOnce`
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
|
16 | for (key, value) in dict {
| ^^^^

error: aborting due to previous error