Skip to content

Commit

Permalink
Auto merge of #21805 - nikomatsakis:closure-inference-refactor-1, r=e…
Browse files Browse the repository at this point in the history
…ddyb

Currently, we only infer the kind of a closure based on the expected type or explicit annotation. If neither applies, we currently report an error. This pull request changes that case to defer the decision until we are able to analyze the actions of the closure: closures which mutate their environment require `FnMut`, closures which move out of their environment require `FnOnce`.

This PR is not the end of the story:

- It does not remove the explicit annotations nor disregard them. The latter is the logical next step to removing them (we'll need a snapshot before we can do anything anyhow). Disregarding explicit annotations might expose more bugs since right now all closures in libstd/rustc use explicit annotations or the expected type, so this inference never kicks in.
- The interaction with instantiating type parameter fallbacks leaves something to be desired. This is mostly just saying that the algorithm from rust-lang/rfcs#213 needs to be implemented, which is a separate bug. There are some semi-subtle interactions though because not knowing whether a closure is `Fn` vs `FnMut` prevents us from resolving obligations like `F : FnMut(...)`, which can in turn prevent unification of some type parameters, which might (in turn) lead to undesired fallback. We can improve this situation however -- even if we don't know whether (or just how) `F : FnMut(..)` holds or not for some closure type `F`, we can still perform unification since we *do* know the argument and return types. Once kind inference is done, we can complete the `F : FnMut(..)` analysis -- which might yield an error if (e.g.) the `F` moves out of its environment. 

r? @nick29581
  • Loading branch information
bors committed Feb 1, 2015
2 parents f1f9cb7 + 870aea2 commit 0ab8d5d
Show file tree
Hide file tree
Showing 37 changed files with 1,201 additions and 415 deletions.
14 changes: 6 additions & 8 deletions src/librustc/metadata/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -139,10 +139,11 @@ pub enum astencode_tag { // Reserves 0x40 -- 0x5f
tag_table_adjustments = 0x51,
tag_table_moves_map = 0x52,
tag_table_capture_map = 0x53,
tag_table_closures = 0x54,
tag_table_upvar_capture_map = 0x55,
tag_table_capture_modes = 0x56,
tag_table_object_cast_map = 0x57,
tag_table_closure_tys = 0x54,
tag_table_closure_kinds = 0x55,
tag_table_upvar_capture_map = 0x56,
tag_table_capture_modes = 0x57,
tag_table_object_cast_map = 0x58,
}

static first_astencode_tag: uint = tag_ast as uint;
Expand Down Expand Up @@ -225,10 +226,7 @@ pub struct LinkMeta {
pub crate_hash: Svh,
}

pub const tag_closures: uint = 0x95;
pub const tag_closure: uint = 0x96;
pub const tag_closure_type: uint = 0x97;
pub const tag_closure_kind: uint = 0x98;
// GAP 0x94...0x98

pub const tag_struct_fields: uint = 0x99;
pub const tag_struct_field: uint = 0x9a;
Expand Down
37 changes: 0 additions & 37 deletions src/librustc/metadata/encoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -618,17 +618,6 @@ fn encode_visibility(rbml_w: &mut Encoder, visibility: ast::Visibility) {
rbml_w.end_tag();
}

fn encode_closure_kind(rbml_w: &mut Encoder, kind: ty::ClosureKind) {
rbml_w.start_tag(tag_closure_kind);
let ch = match kind {
ty::FnClosureKind => 'f',
ty::FnMutClosureKind => 'm',
ty::FnOnceClosureKind => 'o',
};
rbml_w.wr_str(&ch.to_string()[]);
rbml_w.end_tag();
}

fn encode_explicit_self(rbml_w: &mut Encoder,
explicit_self: &ty::ExplicitSelfCategory) {
rbml_w.start_tag(tag_item_trait_method_explicit_self);
Expand Down Expand Up @@ -1843,24 +1832,6 @@ fn encode_macro_defs(rbml_w: &mut Encoder,
rbml_w.end_tag();
}

fn encode_closures<'a>(ecx: &'a EncodeContext, rbml_w: &'a mut Encoder) {
rbml_w.start_tag(tag_closures);
for (closure_id, closure) in ecx.tcx.closures.borrow().iter() {
if closure_id.krate != ast::LOCAL_CRATE {
continue
}

rbml_w.start_tag(tag_closure);
encode_def_id(rbml_w, *closure_id);
rbml_w.start_tag(tag_closure_type);
write_closure_type(ecx, rbml_w, &closure.closure_type);
rbml_w.end_tag();
encode_closure_kind(rbml_w, closure.kind);
rbml_w.end_tag();
}
rbml_w.end_tag();
}

fn encode_struct_field_attrs(rbml_w: &mut Encoder, krate: &ast::Crate) {
struct StructFieldVisitor<'a, 'b:'a> {
rbml_w: &'a mut Encoder<'b>,
Expand Down Expand Up @@ -2069,7 +2040,6 @@ fn encode_metadata_inner(wr: &mut SeekableMemWriter,
native_lib_bytes: u64,
plugin_registrar_fn_bytes: u64,
macro_defs_bytes: u64,
closure_bytes: u64,
impl_bytes: u64,
misc_bytes: u64,
item_bytes: u64,
Expand All @@ -2084,7 +2054,6 @@ fn encode_metadata_inner(wr: &mut SeekableMemWriter,
native_lib_bytes: 0,
plugin_registrar_fn_bytes: 0,
macro_defs_bytes: 0,
closure_bytes: 0,
impl_bytes: 0,
misc_bytes: 0,
item_bytes: 0,
Expand Down Expand Up @@ -2154,11 +2123,6 @@ fn encode_metadata_inner(wr: &mut SeekableMemWriter,
encode_macro_defs(&mut rbml_w, krate);
stats.macro_defs_bytes = rbml_w.writer.tell().unwrap() - i;

// Encode the types of all closures in this crate.
i = rbml_w.writer.tell().unwrap();
encode_closures(&ecx, &mut rbml_w);
stats.closure_bytes = rbml_w.writer.tell().unwrap() - i;

// Encode the def IDs of impls, for coherence checking.
i = rbml_w.writer.tell().unwrap();
encode_impls(&ecx, krate, &mut rbml_w);
Expand Down Expand Up @@ -2199,7 +2163,6 @@ fn encode_metadata_inner(wr: &mut SeekableMemWriter,
println!(" native bytes: {}", stats.native_lib_bytes);
println!("plugin registrar bytes: {}", stats.plugin_registrar_fn_bytes);
println!(" macro def bytes: {}", stats.macro_defs_bytes);
println!(" closure bytes: {}", stats.closure_bytes);
println!(" impl bytes: {}", stats.impl_bytes);
println!(" misc bytes: {}", stats.misc_bytes);
println!(" item bytes: {}", stats.item_bytes);
Expand Down
97 changes: 39 additions & 58 deletions src/librustc/middle/astencode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -647,30 +647,7 @@ impl<'tcx> tr for MethodOrigin<'tcx> {
}

pub fn encode_closure_kind(ebml_w: &mut Encoder, kind: ty::ClosureKind) {
use serialize::Encoder;

ebml_w.emit_enum("ClosureKind", |ebml_w| {
match kind {
ty::FnClosureKind => {
ebml_w.emit_enum_variant("FnClosureKind", 0, 3, |_| {
Ok(())
})
}
ty::FnMutClosureKind => {
ebml_w.emit_enum_variant("FnMutClosureKind", 1, 3, |_| {
Ok(())
})
}
ty::FnOnceClosureKind => {
ebml_w.emit_enum_variant("FnOnceClosureKind",
2,
3,
|_| {
Ok(())
})
}
}
}).unwrap()
kind.encode(ebml_w).unwrap();
}

pub trait vtable_decoder_helpers<'tcx> {
Expand Down Expand Up @@ -1310,12 +1287,20 @@ fn encode_side_tables_for_id(ecx: &e::EncodeContext,
})
}

for closure in tcx.closures.borrow().get(&ast_util::local_def(id)).iter() {
rbml_w.tag(c::tag_table_closures, |rbml_w| {
for &closure_type in tcx.closure_tys.borrow().get(&ast_util::local_def(id)).iter() {
rbml_w.tag(c::tag_table_closure_tys, |rbml_w| {
rbml_w.id(id);
rbml_w.tag(c::tag_table_val, |rbml_w| {
rbml_w.emit_closure_type(ecx, closure_type);
})
})
}

for &&closure_kind in tcx.closure_kinds.borrow().get(&ast_util::local_def(id)).iter() {
rbml_w.tag(c::tag_table_closure_kinds, |rbml_w| {
rbml_w.id(id);
rbml_w.tag(c::tag_table_val, |rbml_w| {
rbml_w.emit_closure_type(ecx, &closure.closure_type);
encode_closure_kind(rbml_w, closure.kind)
encode_closure_kind(rbml_w, closure_kind)
})
})
}
Expand Down Expand Up @@ -1354,8 +1339,10 @@ trait rbml_decoder_decoder_helpers<'tcx> {
-> subst::Substs<'tcx>;
fn read_auto_adjustment<'a, 'b>(&mut self, dcx: &DecodeContext<'a, 'b, 'tcx>)
-> ty::AutoAdjustment<'tcx>;
fn read_closure<'a, 'b>(&mut self, dcx: &DecodeContext<'a, 'b, 'tcx>)
-> ty::Closure<'tcx>;
fn read_closure_kind<'a, 'b>(&mut self, dcx: &DecodeContext<'a, 'b, 'tcx>)
-> ty::ClosureKind;
fn read_closure_ty<'a, 'b>(&mut self, dcx: &DecodeContext<'a, 'b, 'tcx>)
-> ty::ClosureTy<'tcx>;
fn read_auto_deref_ref<'a, 'b>(&mut self, dcx: &DecodeContext<'a, 'b, 'tcx>)
-> ty::AutoDerefRef<'tcx>;
fn read_autoref<'a, 'b>(&mut self, dcx: &DecodeContext<'a, 'b, 'tcx>)
Expand Down Expand Up @@ -1782,35 +1769,23 @@ impl<'a, 'tcx> rbml_decoder_decoder_helpers<'tcx> for reader::Decoder<'a> {
}).unwrap()
}

fn read_closure<'b, 'c>(&mut self, dcx: &DecodeContext<'b, 'c, 'tcx>)
-> ty::Closure<'tcx> {
let closure_type = self.read_opaque(|this, doc| {
fn read_closure_kind<'b, 'c>(&mut self, _dcx: &DecodeContext<'b, 'c, 'tcx>)
-> ty::ClosureKind
{
Decodable::decode(self).ok().unwrap()
}

fn read_closure_ty<'b, 'c>(&mut self, dcx: &DecodeContext<'b, 'c, 'tcx>)
-> ty::ClosureTy<'tcx>
{
self.read_opaque(|this, doc| {
Ok(tydecode::parse_ty_closure_data(
doc.data,
dcx.cdata.cnum,
doc.start,
dcx.tcx,
|s, a| this.convert_def_id(dcx, s, a)))
}).unwrap();
let variants = &[
"FnClosureKind",
"FnMutClosureKind",
"FnOnceClosureKind"
];
let kind = self.read_enum("ClosureKind", |this| {
this.read_enum_variant(variants, |_, i| {
Ok(match i {
0 => ty::FnClosureKind,
1 => ty::FnMutClosureKind,
2 => ty::FnOnceClosureKind,
_ => panic!("bad enum variant for ty::ClosureKind"),
})
})
}).unwrap();
ty::Closure {
closure_type: closure_type,
kind: kind,
}
}).unwrap()
}

/// Converts a def-id that appears in a type. The correct
Expand Down Expand Up @@ -1937,11 +1912,17 @@ fn decode_side_tables(dcx: &DecodeContext,
let adj: ty::AutoAdjustment = val_dsr.read_auto_adjustment(dcx);
dcx.tcx.adjustments.borrow_mut().insert(id, adj);
}
c::tag_table_closures => {
let closure =
val_dsr.read_closure(dcx);
dcx.tcx.closures.borrow_mut().insert(ast_util::local_def(id),
closure);
c::tag_table_closure_tys => {
let closure_ty =
val_dsr.read_closure_ty(dcx);
dcx.tcx.closure_tys.borrow_mut().insert(ast_util::local_def(id),
closure_ty);
}
c::tag_table_closure_kinds => {
let closure_kind =
val_dsr.read_closure_kind(dcx);
dcx.tcx.closure_kinds.borrow_mut().insert(ast_util::local_def(id),
closure_kind);
}
_ => {
dcx.tcx.sess.bug(
Expand Down
6 changes: 2 additions & 4 deletions src/librustc/middle/expr_use_visitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -260,12 +260,10 @@ impl OverloadedCallType {
fn from_closure(tcx: &ty::ctxt, closure_did: ast::DefId)
-> OverloadedCallType {
let trait_did =
tcx.closures
tcx.closure_kinds
.borrow()
.get(&closure_did)
.expect("OverloadedCallType::from_closure: didn't \
find closure id")
.kind
.expect("OverloadedCallType::from_closure: didn't find closure id")
.trait_did(tcx);
OverloadedCallType::from_trait_id(tcx, trait_did)
}
Expand Down
12 changes: 10 additions & 2 deletions src/librustc/middle/mem_categorization.rs
Original file line number Diff line number Diff line change
Expand Up @@ -594,8 +594,16 @@ impl<'t,'tcx,TYPER:Typer<'tcx>> MemCategorizationContext<'t,TYPER> {
let ty = try!(self.node_ty(fn_node_id));
match ty.sty {
ty::ty_closure(closure_id, _, _) => {
let kind = self.typer.closure_kind(closure_id);
self.cat_upvar(id, span, var_id, fn_node_id, kind)
match self.typer.closure_kind(closure_id) {
Some(kind) => {
self.cat_upvar(id, span, var_id, fn_node_id, kind)
}
None => {
self.tcx().sess.span_bug(
span,
&*format!("No closure kind for {:?}", closure_id));
}
}
}
_ => {
self.tcx().sess.span_bug(
Expand Down
17 changes: 11 additions & 6 deletions src/librustc/middle/traits/select.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1024,12 +1024,17 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
kind,
obligation.repr(self.tcx()));

let closure_kind = self.closure_typer.closure_kind(closure_def_id);

debug!("closure_kind = {:?}", closure_kind);

if closure_kind == kind {
candidates.vec.push(ClosureCandidate(closure_def_id, substs.clone()));
match self.closure_typer.closure_kind(closure_def_id) {
Some(closure_kind) => {
debug!("assemble_unboxed_candidates: closure_kind = {:?}", closure_kind);
if closure_kind == kind {
candidates.vec.push(ClosureCandidate(closure_def_id, substs.clone()));
}
}
None => {
debug!("assemble_unboxed_candidates: closure_kind not yet known");
candidates.ambiguous = true;
}
}

Ok(())
Expand Down
Loading

0 comments on commit 0ab8d5d

Please sign in to comment.