auto merge of rust-lang#18688 : bkoropoff/rust/unboxed-closure-subst-fixes, r=nikomatsakis

This resolves some issues that remained after adding support for monomorphizing unboxed closures in trans.

There were a few places where a set of substitutions for an unboxed closure type were dropped on the floor and later recalculated from scratch based on the def ID, but this failed spectacularly when the closure originated from a different param environment.  The substitutions are now plumbed through end-to-end.  Closes rust-lang#18661

There was also a conflict in the meaning of the self param space within the body of the unboxed closure.  Trans attempted to insert the unboxed closure type as the self type, but this could conflict with the self type from the param environment when an unboxed closure was used within a default method on a trait.  Since the body of an unboxed closure cannot refer to its own self type or value, there's no need for it to actually use the self space.  The downstream consumers of the substitutions in trans do not seem to need it either since they look up the type of the closure some other way, so I just stopped setting it.  Closes rust-lang#18685.

r? @pcwalton @nikomatsakis

Author: bors

src/librustc/middle/traits/mod.rs

@@ -176,7 +176,7 @@ pub enum Vtable<N> {
     /// 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<N> Vtable<N> {
     pub fn iter_nested(&self) -> Items<N> {
         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<N> Vtable<N> {
     pub fn map_nested<M>(&self, op: |&N| -> M) -> Vtable<M> {
         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<N> Vtable<N> {
     pub fn map_move_nested<M>(self, op: |N| -> M) -> Vtable<M> {
         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)),
         }

src/librustc/middle/traits/select.rs

@@ -1582,9 +1582,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))
             }
         }
     }

src/librustc/middle/traits/util.rs

@@ -311,9 +311,10 @@ impl<N:Repr> Repr for super::Vtable<N> {
             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)),

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,
     }

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,15 +421,21 @@ 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<ValueRef> {
     let ccx = bcx.ccx();
     if !ccx.tcx().unboxed_closures.borrow().contains_key(&closure_id) {
         // Not an unboxed closure.
         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]

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);
                     }
                 }
 

src/librustc/middle/ty_fold.rs

@@ -414,7 +414,9 @@ impl<N:TypeFoldable> TypeFoldable for traits::Vtable<N> {
     fn fold_with<'tcx, F:TypeFolder<'tcx>>(&self, folder: &mut F) -> traits::Vtable<N> {
         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)),
         }

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 <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.
+
+// Test that param substitutions from the correct environment are
+// used when translating unboxed closure calls.
+
+#![feature(unboxed_closures)]
+
+pub fn inside<F: Fn()>(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<A,B>() {
+    inside(|&:| {});
+}
+
+fn main() {
+    outside::<(),()>();
+}

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 <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.
+
+// 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();
+}