Skip to content

Commit 98958bc

Browse files
committed
auto merge of #18546 : bkoropoff/rust/unboxed-closures-cross-crate, r=nick29581
This fixes some metadata/AST encoding problems that lead to ICEs. The way this is currently handled will need revisiting if abstract return types are added, as unboxed closure types from extern crates could show up without being inlined into the local crate. Closes #16790 (I think this was fixed earlier by accident and just needed a test case) Closes #18378 Closes #18543 r? @pcwalton
2 parents eca8f11 + bfa5320 commit 98958bc

File tree

5 files changed

+94
-37
lines changed

5 files changed

+94
-37
lines changed

Diff for: src/librustc/metadata/tydecode.rs

+4-1
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,9 @@ pub enum DefIdSource {
5555

5656
// Identifies a region parameter (`fn foo<'X>() { ... }`).
5757
RegionParameter,
58+
59+
// Identifies an unboxed closure
60+
UnboxedClosureSource
5861
}
5962
pub type conv_did<'a> =
6063
|source: DefIdSource, ast::DefId|: 'a -> ast::DefId;
@@ -464,7 +467,7 @@ fn parse_ty(st: &mut PState, conv: conv_did) -> ty::t {
464467
}
465468
'k' => {
466469
assert_eq!(next(st), '[');
467-
let did = parse_def(st, NominalType, |x,y| conv(x,y));
470+
let did = parse_def(st, UnboxedClosureSource, |x,y| conv(x,y));
468471
let region = parse_region(st, |x,y| conv(x,y));
469472
let substs = parse_substs(st, |x,y| conv(x,y));
470473
assert_eq!(next(st), ']');

Diff for: src/librustc/middle/astencode.rs

+14-8
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ use metadata::encoder as e;
2121
use middle::region;
2222
use metadata::tydecode;
2323
use metadata::tydecode::{DefIdSource, NominalType, TypeWithId, TypeParameter};
24-
use metadata::tydecode::{RegionParameter};
24+
use metadata::tydecode::{RegionParameter, UnboxedClosureSource};
2525
use metadata::tyencode;
2626
use middle::mem_categorization::Typer;
2727
use middle::subst;
@@ -1728,12 +1728,14 @@ impl<'a> rbml_decoder_decoder_helpers for reader::Decoder<'a> {
17281728
"FnMutUnboxedClosureKind",
17291729
"FnOnceUnboxedClosureKind"
17301730
];
1731-
let kind = self.read_enum_variant(variants, |_, i| {
1732-
Ok(match i {
1733-
0 => ty::FnUnboxedClosureKind,
1734-
1 => ty::FnMutUnboxedClosureKind,
1735-
2 => ty::FnOnceUnboxedClosureKind,
1736-
_ => panic!("bad enum variant for ty::UnboxedClosureKind"),
1731+
let kind = self.read_enum("UnboxedClosureKind", |this| {
1732+
this.read_enum_variant(variants, |_, i| {
1733+
Ok(match i {
1734+
0 => ty::FnUnboxedClosureKind,
1735+
1 => ty::FnMutUnboxedClosureKind,
1736+
2 => ty::FnOnceUnboxedClosureKind,
1737+
_ => panic!("bad enum variant for ty::UnboxedClosureKind"),
1738+
})
17371739
})
17381740
}).unwrap();
17391741
ty::UnboxedClosure {
@@ -1771,13 +1773,17 @@ impl<'a> rbml_decoder_decoder_helpers for reader::Decoder<'a> {
17711773
* case. We translate them with `tr_def_id()` which will map
17721774
* the crate numbers back to the original source crate.
17731775
*
1776+
* Unboxed closures are cloned along with the function being
1777+
* inlined, and all side tables use interned node IDs, so we
1778+
* translate their def IDs accordingly.
1779+
*
17741780
* It'd be really nice to refactor the type repr to not include
17751781
* def-ids so that all these distinctions were unnecessary.
17761782
*/
17771783

17781784
let r = match source {
17791785
NominalType | TypeWithId | RegionParameter => dcx.tr_def_id(did),
1780-
TypeParameter => dcx.tr_intern_def_id(did)
1786+
TypeParameter | UnboxedClosureSource => dcx.tr_intern_def_id(did)
17811787
};
17821788
debug!("convert_def_id(source={}, did={})={}", source, did, r);
17831789
return r;

Diff for: src/librustc/middle/ty.rs

+30-28
Original file line numberDiff line numberDiff line change
@@ -4625,35 +4625,37 @@ pub struct UnboxedClosureUpvar {
46254625
// Returns a list of `UnboxedClosureUpvar`s for each upvar.
46264626
pub fn unboxed_closure_upvars(tcx: &ctxt, closure_id: ast::DefId, substs: &Substs)
46274627
-> Vec<UnboxedClosureUpvar> {
4628-
if closure_id.krate == ast::LOCAL_CRATE {
4629-
let capture_mode = tcx.capture_modes.borrow().get_copy(&closure_id.node);
4630-
match tcx.freevars.borrow().find(&closure_id.node) {
4631-
None => vec![],
4632-
Some(ref freevars) => {
4633-
freevars.iter().map(|freevar| {
4634-
let freevar_def_id = freevar.def.def_id();
4635-
let freevar_ty = node_id_to_type(tcx, freevar_def_id.node);
4636-
let mut freevar_ty = freevar_ty.subst(tcx, substs);
4637-
if capture_mode == ast::CaptureByRef {
4638-
let borrow = tcx.upvar_borrow_map.borrow().get_copy(&ty::UpvarId {
4639-
var_id: freevar_def_id.node,
4640-
closure_expr_id: closure_id.node
4641-
});
4642-
freevar_ty = mk_rptr(tcx, borrow.region, ty::mt {
4643-
ty: freevar_ty,
4644-
mutbl: borrow.kind.to_mutbl_lossy()
4645-
});
4646-
}
4647-
UnboxedClosureUpvar {
4648-
def: freevar.def,
4649-
span: freevar.span,
4650-
ty: freevar_ty
4651-
}
4652-
}).collect()
4653-
}
4628+
// Presently an unboxed closure type cannot "escape" out of a
4629+
// function, so we will only encounter ones that originated in the
4630+
// local crate or were inlined into it along with some function.
4631+
// This may change if abstract return types of some sort are
4632+
// implemented.
4633+
assert!(closure_id.krate == ast::LOCAL_CRATE);
4634+
let capture_mode = tcx.capture_modes.borrow().get_copy(&closure_id.node);
4635+
match tcx.freevars.borrow().find(&closure_id.node) {
4636+
None => vec![],
4637+
Some(ref freevars) => {
4638+
freevars.iter().map(|freevar| {
4639+
let freevar_def_id = freevar.def.def_id();
4640+
let freevar_ty = node_id_to_type(tcx, freevar_def_id.node);
4641+
let mut freevar_ty = freevar_ty.subst(tcx, substs);
4642+
if capture_mode == ast::CaptureByRef {
4643+
let borrow = tcx.upvar_borrow_map.borrow().get_copy(&ty::UpvarId {
4644+
var_id: freevar_def_id.node,
4645+
closure_expr_id: closure_id.node
4646+
});
4647+
freevar_ty = mk_rptr(tcx, borrow.region, ty::mt {
4648+
ty: freevar_ty,
4649+
mutbl: borrow.kind.to_mutbl_lossy()
4650+
});
4651+
}
4652+
UnboxedClosureUpvar {
4653+
def: freevar.def,
4654+
span: freevar.span,
4655+
ty: freevar_ty
4656+
}
4657+
}).collect()
46544658
}
4655-
} else {
4656-
tcx.sess.bug("unimplemented cross-crate closure upvars")
46574659
}
46584660
}
46594661

Diff for: src/test/auxiliary/unboxed-closures-cross-crate.rs

+26
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
// Copyright 2014 The Rust Project Developers. See the COPYRIGHT
2+
// file at the top-level directory of this distribution and at
3+
// http://rust-lang.org/COPYRIGHT.
4+
//
5+
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
6+
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
7+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
8+
// option. This file may not be copied, modified, or distributed
9+
// except according to those terms.
10+
11+
#![feature(unboxed_closures, overloaded_calls)]
12+
13+
#[inline]
14+
pub fn has_closures() -> uint {
15+
let x = 1u;
16+
let mut f = move |&mut:| x;
17+
let y = 1u;
18+
let g = |:| y;
19+
f() + g()
20+
}
21+
22+
pub fn has_generic_closures<T: Add<T,T> + Copy>(x: T, y: T) -> T {
23+
let mut f = move |&mut:| x;
24+
let g = |:| y;
25+
f() + g()
26+
}

Diff for: src/test/run-pass/unboxed-closures-cross-crate.rs

+20
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
// Copyright 2014 The Rust Project Developers. See the COPYRIGHT
2+
// file at the top-level directory of this distribution and at
3+
// http://rust-lang.org/COPYRIGHT.
4+
//
5+
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
6+
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
7+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
8+
// option. This file may not be copied, modified, or distributed
9+
// except according to those terms.
10+
11+
// Test that unboxed closures work with cross-crate inlining
12+
// Acts as a regression test for #16790, #18378 and #18543
13+
14+
// aux-build:unboxed-closures-cross-crate.rs
15+
extern crate "unboxed-closures-cross-crate" as ubcc;
16+
17+
fn main() {
18+
assert_eq!(ubcc::has_closures(), 2u);
19+
assert_eq!(ubcc::has_generic_closures(2u, 3u), 5u);
20+
}

0 commit comments

Comments
 (0)