From daa215e8c536a1940d9f0f668610c67e6aa8542e Mon Sep 17 00:00:00 2001 From: Brian Koropoff Date: Wed, 5 Nov 2014 23:50:10 -0800 Subject: [PATCH 1/3] Fix handling of unboxed closure type param substitutions - When selecting an implicit trait impl for an unboxed closure, plumb through and use the substitutions from impl selection instead of using those from the current param environment in trans, which may be incorrect. - When generating a function declaration for an unboxed closure, plumb through the substitutions from the param environment of the closure as above. Also normalize the type to avoid generating duplicate declarations due to regions being inconsistently replaced with ReStatic elsewhere. - Do not place the closure type in the self param space when translating the unboxed closure callee, etc. It is not actually used, and doing so conflicts with the self substitution from default trait methods. Closes #18661 Closes #18685 --- src/librustc/middle/traits/mod.rs | 8 ++--- src/librustc/middle/traits/select.rs | 6 ++-- src/librustc/middle/traits/util.rs | 7 +++-- src/librustc/middle/trans/callee.rs | 4 ++- src/librustc/middle/trans/closure.rs | 15 ++++++--- src/librustc/middle/trans/meth.rs | 47 ++++++++-------------------- src/librustc/middle/ty_fold.rs | 4 ++- 7 files changed, 41 insertions(+), 50 deletions(-) diff --git a/src/librustc/middle/traits/mod.rs b/src/librustc/middle/traits/mod.rs index 76715561b0397..c4a5b14303e3f 100644 --- a/src/librustc/middle/traits/mod.rs +++ b/src/librustc/middle/traits/mod.rs @@ -176,7 +176,7 @@ pub enum Vtable { /// ID is the ID of the closure expression. This is a `VtableImpl` /// in spirit, but the impl is generated by the compiler and does /// not appear in the source. - VtableUnboxedClosure(ast::DefId), + VtableUnboxedClosure(ast::DefId, subst::Substs), /// Successful resolution to an obligation provided by the caller /// for some type parameter. @@ -338,7 +338,7 @@ impl Vtable { pub fn iter_nested(&self) -> Items { match *self { VtableImpl(ref i) => i.iter_nested(), - VtableUnboxedClosure(_) => (&[]).iter(), + VtableUnboxedClosure(..) => (&[]).iter(), VtableParam(_) => (&[]).iter(), VtableBuiltin(ref i) => i.iter_nested(), } @@ -347,7 +347,7 @@ impl Vtable { pub fn map_nested(&self, op: |&N| -> M) -> Vtable { match *self { VtableImpl(ref i) => VtableImpl(i.map_nested(op)), - VtableUnboxedClosure(d) => VtableUnboxedClosure(d), + VtableUnboxedClosure(d, ref s) => VtableUnboxedClosure(d, s.clone()), VtableParam(ref p) => VtableParam((*p).clone()), VtableBuiltin(ref i) => VtableBuiltin(i.map_nested(op)), } @@ -356,7 +356,7 @@ impl Vtable { pub fn map_move_nested(self, op: |N| -> M) -> Vtable { match self { VtableImpl(i) => VtableImpl(i.map_move_nested(op)), - VtableUnboxedClosure(d) => VtableUnboxedClosure(d), + VtableUnboxedClosure(d, s) => VtableUnboxedClosure(d, s), VtableParam(p) => VtableParam(p), VtableBuiltin(i) => VtableBuiltin(i.map_move_nested(op)), } diff --git a/src/librustc/middle/traits/select.rs b/src/librustc/middle/traits/select.rs index 9a2d5b4d4dc0a..0a1b3bc65cf98 100644 --- a/src/librustc/middle/traits/select.rs +++ b/src/librustc/middle/traits/select.rs @@ -1558,9 +1558,9 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { Ok(VtableImpl(vtable_impl)) } - UnboxedClosureCandidate(closure_def_id, ref substs) => { - try!(self.confirm_unboxed_closure_candidate(obligation, closure_def_id, substs)); - Ok(VtableUnboxedClosure(closure_def_id)) + UnboxedClosureCandidate(closure_def_id, substs) => { + try!(self.confirm_unboxed_closure_candidate(obligation, closure_def_id, &substs)); + Ok(VtableUnboxedClosure(closure_def_id, substs)) } } } diff --git a/src/librustc/middle/traits/util.rs b/src/librustc/middle/traits/util.rs index 9ee2c3aa14907..0ecfa99ee6da5 100644 --- a/src/librustc/middle/traits/util.rs +++ b/src/librustc/middle/traits/util.rs @@ -311,9 +311,10 @@ impl Repr for super::Vtable { super::VtableImpl(ref v) => v.repr(tcx), - super::VtableUnboxedClosure(ref d) => - format!("VtableUnboxedClosure({})", - d.repr(tcx)), + super::VtableUnboxedClosure(ref d, ref s) => + format!("VtableUnboxedClosure({},{})", + d.repr(tcx), + s.repr(tcx)), super::VtableParam(ref v) => format!("VtableParam({})", v.repr(tcx)), diff --git a/src/librustc/middle/trans/callee.rs b/src/librustc/middle/trans/callee.rs index 6addd9144409c..987866ed243e5 100644 --- a/src/librustc/middle/trans/callee.rs +++ b/src/librustc/middle/trans/callee.rs @@ -490,7 +490,9 @@ pub fn trans_fn_ref_with_substs( }; // If this is an unboxed closure, redirect to it. - match closure::get_or_create_declaration_if_unboxed_closure(bcx, def_id) { + match closure::get_or_create_declaration_if_unboxed_closure(bcx, + def_id, + &substs) { None => {} Some(llfn) => return llfn, } diff --git a/src/librustc/middle/trans/closure.rs b/src/librustc/middle/trans/closure.rs index decd238627c20..4e05d8dd15922 100644 --- a/src/librustc/middle/trans/closure.rs +++ b/src/librustc/middle/trans/closure.rs @@ -27,6 +27,7 @@ use middle::trans::monomorphize::MonoId; use middle::trans::type_of::*; use middle::trans::type_::Type; use middle::ty; +use middle::subst::{Subst, Substs}; use util::ppaux::Repr; use util::ppaux::ty_to_string; @@ -420,7 +421,8 @@ pub fn trans_expr_fn<'blk, 'tcx>(bcx: Block<'blk, 'tcx>, /// Returns the LLVM function declaration for an unboxed closure, creating it /// if necessary. If the ID does not correspond to a closure ID, returns None. pub fn get_or_create_declaration_if_unboxed_closure<'blk, 'tcx>(bcx: Block<'blk, 'tcx>, - closure_id: ast::DefId) + closure_id: ast::DefId, + substs: &Substs) -> Option { let ccx = bcx.ccx(); if !ccx.tcx().unboxed_closures.borrow().contains_key(&closure_id) { @@ -428,7 +430,12 @@ pub fn get_or_create_declaration_if_unboxed_closure<'blk, 'tcx>(bcx: Block<'blk, return None } - let function_type = node_id_type(bcx, closure_id.node); + let function_type = ty::node_id_to_type(bcx.tcx(), closure_id.node); + let function_type = function_type.subst(bcx.tcx(), substs); + + // Normalize type so differences in regions and typedefs don't cause + // duplicate declarations + let function_type = ty::normalize_ty(bcx.tcx(), function_type); let params = match ty::get(function_type).sty { ty::ty_unboxed_closure(_, _, ref substs) => substs.types.clone(), _ => unreachable!() @@ -447,7 +454,6 @@ pub fn get_or_create_declaration_if_unboxed_closure<'blk, 'tcx>(bcx: Block<'blk, None => {} } - let function_type = node_id_type(bcx, closure_id.node); let symbol = ccx.tcx().map.with_path(closure_id.node, |path| { mangle_internal_name_by_path_and_seq(path, "unboxed_closure") }); @@ -480,7 +486,8 @@ pub fn trans_unboxed_closure<'blk, 'tcx>( let closure_id = ast_util::local_def(id); let llfn = get_or_create_declaration_if_unboxed_closure( bcx, - closure_id).unwrap(); + closure_id, + &bcx.fcx.param_substs.substs).unwrap(); let unboxed_closures = bcx.tcx().unboxed_closures.borrow(); let function_type = (*unboxed_closures)[closure_id] diff --git a/src/librustc/middle/trans/meth.rs b/src/librustc/middle/trans/meth.rs index fbd4db959ce0c..9911755b875ee 100644 --- a/src/librustc/middle/trans/meth.rs +++ b/src/librustc/middle/trans/meth.rs @@ -355,16 +355,13 @@ fn trans_monomorphized_callee<'blk, 'tcx>(bcx: Block<'blk, 'tcx>, Callee { bcx: bcx, data: Fn(llfn) } } - traits::VtableUnboxedClosure(closure_def_id) => { - let self_ty = node_id_type(bcx, closure_def_id.node); - let callee_substs = get_callee_substitutions_for_unboxed_closure( - bcx, - self_ty); - + traits::VtableUnboxedClosure(closure_def_id, substs) => { + // The substitutions should have no type parameters remaining + // after passing through fulfill_obligation let llfn = trans_fn_ref_with_substs(bcx, closure_def_id, MethodCall(method_call), - callee_substs); + substs); Callee { bcx: bcx, @@ -518,24 +515,6 @@ pub fn trans_trait_callee_from_llval<'blk, 'tcx>(bcx: Block<'blk, 'tcx>, }; } -/// Looks up the substitutions for an unboxed closure and adds the -/// self type -fn get_callee_substitutions_for_unboxed_closure(bcx: Block, - self_ty: ty::t) - -> subst::Substs { - match ty::get(self_ty).sty { - ty::ty_unboxed_closure(_, _, ref substs) => { - substs.with_self_ty(ty::mk_rptr(bcx.tcx(), - ty::ReStatic, - ty::mt { - ty: self_ty, - mutbl: ast::MutMutable, - })) - }, - _ => unreachable!() - } -} - /// Creates a returns a dynamic vtable for the given type and vtable origin. /// This is used only for objects. /// @@ -580,19 +559,19 @@ pub fn get_vtable(bcx: Block, nested: _ }) => { emit_vtable_methods(bcx, id, substs).into_iter() } - traits::VtableUnboxedClosure(closure_def_id) => { - let self_ty = node_id_type(bcx, closure_def_id.node); - - let callee_substs = - get_callee_substitutions_for_unboxed_closure( - bcx, - self_ty.clone()); + traits::VtableUnboxedClosure(closure_def_id, substs) => { + // Look up closure type + let self_ty = ty::node_id_to_type(bcx.tcx(), closure_def_id.node); + // Apply substitutions from closure param environment. + // The substitutions should have no type parameters + // remaining after passing through fulfill_obligation + let self_ty = self_ty.subst(bcx.tcx(), &substs); let mut llfn = trans_fn_ref_with_substs( bcx, closure_def_id, ExprId(0), - callee_substs.clone()); + substs.clone()); { let unboxed_closures = bcx.tcx() @@ -645,7 +624,7 @@ pub fn get_vtable(bcx: Block, llfn, &closure_type, closure_def_id, - callee_substs); + substs); } } diff --git a/src/librustc/middle/ty_fold.rs b/src/librustc/middle/ty_fold.rs index a96e81ce20bb4..6a72574aaaa1d 100644 --- a/src/librustc/middle/ty_fold.rs +++ b/src/librustc/middle/ty_fold.rs @@ -414,7 +414,9 @@ impl TypeFoldable for traits::Vtable { fn fold_with<'tcx, F:TypeFolder<'tcx>>(&self, folder: &mut F) -> traits::Vtable { match *self { traits::VtableImpl(ref v) => traits::VtableImpl(v.fold_with(folder)), - traits::VtableUnboxedClosure(d) => traits::VtableUnboxedClosure(d), + traits::VtableUnboxedClosure(d, ref s) => { + traits::VtableUnboxedClosure(d, s.fold_with(folder)) + } traits::VtableParam(ref p) => traits::VtableParam(p.fold_with(folder)), traits::VtableBuiltin(ref d) => traits::VtableBuiltin(d.fold_with(folder)), } From d317039b222f0c73f80434f8d248f8adb19e3fdd Mon Sep 17 00:00:00 2001 From: Brian Koropoff Date: Thu, 6 Nov 2014 00:06:28 -0800 Subject: [PATCH 2/3] Add regression test for #18661 --- src/test/run-pass/issue-18661.rs | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) create mode 100644 src/test/run-pass/issue-18661.rs diff --git a/src/test/run-pass/issue-18661.rs b/src/test/run-pass/issue-18661.rs new file mode 100644 index 0000000000000..6a2f73a787a48 --- /dev/null +++ b/src/test/run-pass/issue-18661.rs @@ -0,0 +1,28 @@ +// Copyright 2014 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 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// Test that param substitutions from the correct environment are +// used when translating unboxed closure calls. + +#![feature(unboxed_closures)] + +pub fn inside(c: F) { + c.call(()); +} + +// Use different number of type parameters and closure type to trigger +// an obvious ICE when param environments are mixed up +pub fn outside() { + inside(|&:| {}); +} + +fn main() { + outside::<(),()>(); +} From ddb17b239b5b2b05932516bfb6b7679915fd4c27 Mon Sep 17 00:00:00 2001 From: Brian Koropoff Date: Thu, 6 Nov 2014 00:06:41 -0800 Subject: [PATCH 3/3] Add regression test for #18685 --- src/test/run-pass/issue-18685.rs | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) create mode 100644 src/test/run-pass/issue-18685.rs diff --git a/src/test/run-pass/issue-18685.rs b/src/test/run-pass/issue-18685.rs new file mode 100644 index 0000000000000..1f74e2f847497 --- /dev/null +++ b/src/test/run-pass/issue-18685.rs @@ -0,0 +1,30 @@ +// Copyright 2014 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 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// Test that the self param space is not used in a conflicting +// manner by unboxed closures within a default method on a trait + +#![feature(unboxed_closures, overloaded_calls)] + +trait Tr { + fn foo(&self); + + fn bar(&self) { + (|:| { self.foo() })() + } +} + +impl Tr for () { + fn foo(&self) {} +} + +fn main() { + ().bar(); +}