Remove cleanup scope from FunctionContext

5 files changed, 154 insertions, 98 deletions

diff --git a/src/librustc_trans/base.rs b/src/librustc_trans/base.rs
index 0d455d037de..3cbc3935cfc 100644
--- a/src/librustc_trans/base.rs
+++ b/src/librustc_trans/base.rs
@@ -765,7 +765,6 @@ impl<'blk, 'tcx> FunctionContext<'blk, 'tcx> {
             funclet_arena: TypedArena::new(),
             ccx: ccx,
             debug_context: debug_context,
-            cleanup_scope: RefCell::new(None),
         }
     }
 
diff --git a/src/librustc_trans/callee.rs b/src/librustc_trans/callee.rs
index f772ff68ac9..1a4afb4b02e 100644
--- a/src/librustc_trans/callee.rs
+++ b/src/librustc_trans/callee.rs
@@ -27,6 +27,7 @@ use base::*;
 use common::{
     self, BlockAndBuilder, CrateContext, FunctionContext, SharedCrateContext
 };
+use cleanup::CleanupScope;
 use consts;
 use declare;
 use value::Value;
@@ -389,10 +390,10 @@ fn trans_fn_once_adapter_shim<'a, 'tcx>(
 
     // Call the by-ref closure body with `self` in a cleanup scope,
     // to drop `self` when the body returns, or in case it unwinds.
-    let self_scope = fcx.schedule_drop_mem(llenv, closure_ty);
-
-    let bcx = callee.call(bcx, &llargs[self_idx..], dest, None).0;
-    fcx.pop_and_trans_custom_cleanup_scope(&bcx, self_scope);
+    let mut self_scope = fcx.schedule_drop_mem(llenv, closure_ty);
+    let bcx = trans_call_fn_once_adapter_shim(
+        bcx, callee, &llargs[self_idx..], dest, &mut self_scope);
+    fcx.trans_scope(&bcx, self_scope);
     fcx.finish(&bcx);
 
     ccx.instances().borrow_mut().insert(method_instance, lloncefn);
@@ -685,23 +686,69 @@ fn trans_call_inner<'a, 'blk, 'tcx>(bcx: BlockAndBuilder<'blk, 'tcx>,
     };
 
     let _icx = push_ctxt("invoke_");
-    let (llret, bcx) = if bcx.fcx().needs_invoke(lpad.is_some()) {
-        debug!("invoking {:?} at {:?}", Value(llfn), bcx.llbb());
-        for &llarg in &llargs {
-            debug!("arg: {:?}", Value(llarg));
+    let llret = bcx.call(llfn, &llargs[..], lpad);
+    fn_ty.apply_attrs_callsite(llret);
+
+    // If the function we just called does not use an outpointer,
+    // store the result into the rust outpointer. Cast the outpointer
+    // type to match because some ABIs will use a different type than
+    // the Rust type. e.g., a {u32,u32} struct could be returned as
+    // u64.
+    if !fn_ty.ret.is_indirect() {
+        if let Some(llretslot) = opt_llretslot {
+            fn_ty.ret.store(&bcx, llret, llretslot);
+        }
+    }
+
+    if fn_ret.0.is_never() {
+        bcx.unreachable();
+    }
+
+    (bcx, llret)
+}
+
+// This is a cleaned up version of trans_call_inner.
+fn trans_call_fn_once_adapter_shim<'a, 'blk, 'tcx>(
+    bcx: BlockAndBuilder<'blk, 'tcx>,
+    callee: Callee<'tcx>,
+    args: &[ValueRef],
+    opt_llretslot: Option<ValueRef>,
+    cleanup_scope: &mut Option<CleanupScope<'tcx>>,
+) -> BlockAndBuilder<'blk, 'tcx> {
+    let fn_ret = callee.ty.fn_ret();
+    let fn_ty = callee.direct_fn_type(bcx.ccx(), &[]);
+
+    // If there no destination, return must be direct, with no cast.
+    if opt_llretslot.is_none() {
+        assert!(!fn_ty.ret.is_indirect() && fn_ty.ret.cast.is_none());
+    }
+
+    let mut llargs = Vec::new();
+
+    if fn_ty.ret.is_indirect() {
+        let mut llretslot = opt_llretslot.unwrap();
+        if let Some(ty) = fn_ty.ret.cast {
+            llretslot = bcx.pointercast(llretslot, ty.ptr_to());
         }
+        llargs.push(llretslot);
+    }
+
+    llargs.extend_from_slice(args);
+
+    let llfn = match callee.data {
+        Fn(f) => f,
+        _ => bug!("expected fn pointer callee, found {:?}", callee)
+    };
+
+    let _icx = push_ctxt("invoke_");
+    let (llret, bcx) = if cleanup_scope.is_some() && !bcx.sess().no_landing_pads() {
         let normal_bcx = bcx.fcx().build_new_block("normal-return");
-        let landing_pad = bcx.fcx().get_landing_pad();
+        let landing_pad = bcx.fcx().get_landing_pad(cleanup_scope);
 
-        let llresult = bcx.invoke(llfn, &llargs[..], normal_bcx.llbb(), landing_pad, lpad);
+        let llresult = bcx.invoke(llfn, &llargs[..], normal_bcx.llbb(), landing_pad, None);
         (llresult, normal_bcx)
     } else {
-        debug!("calling {:?} at {:?}", Value(llfn), bcx.llbb());
-        for &llarg in &llargs {
-            debug!("arg: {:?}", Value(llarg));
-        }
-
-        let llresult = bcx.call(llfn, &llargs[..], lpad);
+        let llresult = bcx.call(llfn, &llargs[..], None);
         (llresult, bcx)
     };
     fn_ty.apply_attrs_callsite(llret);
@@ -721,5 +768,5 @@ fn trans_call_inner<'a, 'blk, 'tcx>(bcx: BlockAndBuilder<'blk, 'tcx>,
         bcx.unreachable();
     }
 
-    (bcx, llret)
+    bcx
 }
diff --git a/src/librustc_trans/cleanup.rs b/src/librustc_trans/cleanup.rs
index 8952fe9d8b8..d9e8b795cb1 100644
--- a/src/librustc_trans/cleanup.rs
+++ b/src/librustc_trans/cleanup.rs
@@ -149,24 +149,19 @@ struct CachedEarlyExit {
 }
 
 impl<'blk, 'tcx> FunctionContext<'blk, 'tcx> {
-    /// Removes the top cleanup scope from the stack, which must be a temporary scope, and
-    /// generates the code to do its cleanups for normal exit.
-    pub fn pop_and_trans_custom_cleanup_scope(&self,
-                                              bcx: &BlockAndBuilder<'blk, 'tcx>,
-                                              custom_scope: Option<()>) {
-        debug!("pop_and_trans_custom_cleanup_scope({:?})", custom_scope);
-
-        if custom_scope.is_none() {
-            return;
+    pub fn trans_scope(
+        &self,
+        bcx: &BlockAndBuilder<'blk, 'tcx>,
+        custom_scope: Option<CleanupScope<'tcx>>
+    ) {
+        if let Some(scope) = custom_scope {
+            scope.cleanup.trans(bcx.funclet(), &bcx);
         }
-
-        let scope = self.pop_scope();
-        scope.cleanup.trans(bcx.funclet(), &bcx);
     }
 
     /// Schedules a (deep) drop of `val`, which is a pointer to an instance of
     /// `ty`
-    pub fn schedule_drop_mem(&self, val: ValueRef, ty: Ty<'tcx>) -> Option<()> {
+    pub fn schedule_drop_mem(&self, val: ValueRef, ty: Ty<'tcx>) -> Option<CleanupScope<'tcx>> {
         if !self.type_needs_drop(ty) { return None; }
         let drop = DropValue {
             val: val,
@@ -176,7 +171,7 @@ impl<'blk, 'tcx> FunctionContext<'blk, 'tcx> {
 
         debug!("schedule_drop_mem(val={:?}, ty={:?}) skip_dtor={}", Value(val), ty, drop.skip_dtor);
 
-        Some(self.set_scope(CleanupScope::new(drop)))
+        Some(CleanupScope::new(drop))
     }
 
     /// Issue #23611: Schedules a (deep) drop of the contents of
@@ -184,7 +179,8 @@ impl<'blk, 'tcx> FunctionContext<'blk, 'tcx> {
     /// `ty`. The scheduled code handles extracting the discriminant
     /// and dropping the contents associated with that variant
     /// *without* executing any associated drop implementation.
-    pub fn schedule_drop_adt_contents(&self, val: ValueRef, ty: Ty<'tcx>) -> Option<()> {
+    pub fn schedule_drop_adt_contents(&self, val: ValueRef, ty: Ty<'tcx>)
+        -> Option<CleanupScope<'tcx>> {
         // `if` below could be "!contents_needs_drop"; skipping drop
         // is just an optimization, so sound to be conservative.
         if !self.type_needs_drop(ty) { return None; }
@@ -200,16 +196,7 @@ impl<'blk, 'tcx> FunctionContext<'blk, 'tcx> {
                ty,
                drop.skip_dtor);
 
-        Some(self.set_scope(CleanupScope::new(drop)))
-    }
-
-    /// Returns true if there are pending cleanups that should execute on panic.
-    pub fn needs_invoke(&self, lpad_present: bool) -> bool {
-        if self.ccx.sess().no_landing_pads() || lpad_present {
-            false
-        } else {
-            self.has_scope()
-        }
+        Some(CleanupScope::new(drop))
     }
 
     /// Creates a landing pad for the top scope, if one does not exist. The
@@ -220,22 +207,21 @@ impl<'blk, 'tcx> FunctionContext<'blk, 'tcx> {
     ///
     /// (The cleanups and resume instruction are created by
     /// `trans_cleanups_to_exit_scope()`, not in this function itself.)
-    pub fn get_landing_pad(&'blk self) -> BasicBlockRef {
-        let mut pad_bcx;
+    pub fn get_landing_pad(&'blk self, scope: &mut Option<CleanupScope<'tcx>>) -> BasicBlockRef {
+        // TODO: Factor out and take a CleanupScope.
+        assert!(scope.is_some());
 
         debug!("get_landing_pad");
 
         // Check if a landing pad block exists; if not, create one.
-        {
-            let mut last_scope = self.cleanup_scope.borrow_mut();
-            let mut last_scope = last_scope.as_mut().unwrap();
-            match last_scope.cached_landing_pad {
-                Some(llbb) => return llbb,
-                None => {
-                    let name = last_scope.block_name("unwind");
-                    pad_bcx = self.build_new_block(&name[..]);
-                    last_scope.cached_landing_pad = Some(pad_bcx.llbb());
-                }
+        let mut scope = scope.as_mut().unwrap();
+        let mut pad_bcx = match scope.cached_landing_pad {
+            Some(llbb) => return llbb,
+            None => {
+                let name = scope.block_name("unwind");
+                let pad_bcx = self.build_new_block(&name[..]);
+                scope.cached_landing_pad = Some(pad_bcx.llbb());
+                pad_bcx
             }
         };
 
@@ -278,30 +264,12 @@ impl<'blk, 'tcx> FunctionContext<'blk, 'tcx> {
         };
 
         // Generate the cleanup block and branch to it.
-        let cleanup_llbb = self.trans_cleanups_to_exit_scope(val);
+        let cleanup_llbb = self.trans_cleanups_to_exit_scope(val, scope);
         val.branch(&mut pad_bcx, cleanup_llbb);
 
         return pad_bcx.llbb();
     }
 
-    fn has_scope(&self) -> bool {
-        self.cleanup_scope.borrow().is_some()
-    }
-
-    fn set_scope(&self, scope: CleanupScope<'tcx>) {
-        assert!(self.cleanup_scope.borrow().is_none());
-        *self.cleanup_scope.borrow_mut() = Some(scope);
-    }
-
-    fn pop_scope(&self) -> CleanupScope<'tcx> {
-        debug!("took cleanup scope {}", self.top_scope(|s| s.block_name("")));
-        self.cleanup_scope.borrow_mut().take().unwrap()
-    }
-
-    fn top_scope<R, F>(&self, f: F) -> R where F: FnOnce(&CleanupScope<'tcx>) -> R {
-        f(self.cleanup_scope.borrow().as_ref().unwrap())
-    }
-
     fn generate_resume_block(&self, label: UnwindKind) -> BasicBlockRef {
         // Generate a block that will resume unwinding to the calling function
         let bcx = self.build_new_block("resume");
@@ -328,18 +296,12 @@ impl<'blk, 'tcx> FunctionContext<'blk, 'tcx> {
     /// break, continue, or unwind. This function will generate all cleanups
     /// between the top of the stack and the exit `label` and return a basic
     /// block that the caller can branch to.
-    fn trans_cleanups_to_exit_scope(&'blk self, label: UnwindKind) -> BasicBlockRef {
-        debug!("trans_cleanups_to_exit_scope label={:?} has_scope={}", label, self.has_scope());
-
-        // If there is no current scope, then there are no cleanups to run, so we should
-        // simply generate a resume block which will branch to the label.
-        if !self.has_scope() {
-            debug!("trans_cleanups_to_exit_scope: returning new block scope");
-            return self.generate_resume_block(label);
-        }
-
-        // Pop off the scope, since we may be generating unwinding code for it.
-        let mut scope = self.pop_scope();
+    fn trans_cleanups_to_exit_scope(
+        &'blk self,
+        label: UnwindKind,
+        scope: &mut CleanupScope<'tcx>
+    ) -> BasicBlockRef {
+        debug!("trans_cleanups_to_exit_scope label={:?}`", label);
         let cached_exit = scope.cached_early_exit(label);
 
         // Check if we have already cached the unwinding of this
@@ -361,9 +323,6 @@ impl<'blk, 'tcx> FunctionContext<'blk, 'tcx> {
         // FIXME: Can this get called more than once per scope? If not, no need to cache.
         scope.add_cached_early_exit(label, cleanup.llbb());
 
-        // Put the scope back
-        self.set_scope(scope);
-
         debug!("trans_cleanups_to_exit_scope: llbb={:?}", cleanup.llbb());
 
         cleanup.llbb()
diff --git a/src/librustc_trans/common.rs b/src/librustc_trans/common.rs
index 0ef56895ece..8637bb322ca 100644
--- a/src/librustc_trans/common.rs
+++ b/src/librustc_trans/common.rs
@@ -28,7 +28,6 @@ use abi::{Abi, FnType};
 use base;
 use builder::Builder;
 use callee::Callee;
-use cleanup;
 use consts;
 use debuginfo;
 use declare;
@@ -48,7 +47,7 @@ use std::borrow::Cow;
 use std::iter;
 use std::ops::Deref;
 use std::ffi::CString;
-use std::cell::{Cell, RefCell, Ref};
+use std::cell::{Cell, Ref};
 
 use syntax::ast;
 use syntax::symbol::{Symbol, InternedString};
@@ -315,9 +314,6 @@ pub struct FunctionContext<'a, 'tcx: 'a> {
 
     // Used and maintained by the debuginfo module.
     pub debug_context: debuginfo::FunctionDebugContext,
-
-    // Cleanup scopes.
-    pub cleanup_scope: RefCell<Option<cleanup::CleanupScope<'tcx>>>,
 }
 
 impl<'a, 'tcx> FunctionContext<'a, 'tcx> {
diff --git a/src/librustc_trans/glue.rs b/src/librustc_trans/glue.rs
index 1265381ff21..dc3f75d52b9 100644
--- a/src/librustc_trans/glue.rs
+++ b/src/librustc_trans/glue.rs
@@ -22,7 +22,9 @@ use rustc::traits;
 use rustc::ty::{self, AdtKind, Ty, TyCtxt, TypeFoldable};
 use adt;
 use base::*;
-use callee::{Callee};
+use callee::{Callee, CalleeData};
+use cleanup::CleanupScope;
+use meth;
 use common::*;
 use machine::*;
 use monomorphize;
@@ -241,7 +243,7 @@ fn trans_custom_dtor<'blk, 'tcx>(bcx: BlockAndBuilder<'blk, 'tcx>,
     // might well consider changing below to more direct code.
     // Issue #23611: schedule cleanup of contents, re-inspecting the
     // discriminant (if any) in case of variant swap in drop code.
-    let contents_scope = if !shallow_drop {
+    let mut contents_scope = if !shallow_drop {
         bcx.fcx().schedule_drop_adt_contents(v0, t)
     } else {
         None
@@ -269,8 +271,61 @@ fn trans_custom_dtor<'blk, 'tcx>(bcx: BlockAndBuilder<'blk, 'tcx>,
         _ => bug!("dtor for {:?} is not an impl???", t)
     };
     let dtor_did = def.destructor().unwrap();
-    let bcx = Callee::def(bcx.ccx(), dtor_did, vtbl.substs).call(bcx, args, None, None).0;
-    bcx.fcx().pop_and_trans_custom_cleanup_scope(&bcx, contents_scope);
+    let callee = Callee::def(bcx.ccx(), dtor_did, vtbl.substs);
+    let bcx = trans_call_custom_dtor(bcx, callee, args, &mut contents_scope);
+    bcx.fcx().trans_scope(&bcx, contents_scope);
+    bcx
+}
+
+// Inlined and simplified version of callee::trans_call_inner
+fn trans_call_custom_dtor<'a, 'blk, 'tcx>(
+    bcx: BlockAndBuilder<'blk, 'tcx>,
+    callee: Callee<'tcx>,
+    args: &[ValueRef],
+    cleanup_scope: &mut Option<CleanupScope<'tcx>>,
+) -> BlockAndBuilder<'blk, 'tcx> {
+    let fn_ret = callee.ty.fn_ret();
+    let fn_ty = callee.direct_fn_type(bcx.ccx(), &[]);
+
+    // Return must be direct, with no cast.
+    assert!(!fn_ty.ret.is_indirect() && fn_ty.ret.cast.is_none());
+
+    let mut llargs = Vec::new();
+
+    let llfn = match callee.data {
+        CalleeData::Virtual(idx) => {
+            llargs.push(args[0]);
+
+            let fn_ptr = meth::get_virtual_method(&bcx, args[1], idx);
+            let llty = fn_ty.llvm_type(&bcx.ccx()).ptr_to();
+            let llfn = bcx.pointercast(fn_ptr, llty);
+            llargs.extend_from_slice(&args[2..]);
+            llfn
+        }
+        CalleeData::Fn(f) => {
+            llargs.extend_from_slice(args);
+            f
+        }
+        _ => bug!("Expected virtual or fn pointer callee, found {:?}", callee)
+    };
+
+    let _icx = push_ctxt("invoke_");
+    let (llret, bcx) = if cleanup_scope.is_some() && !bcx.sess().no_landing_pads() {
+        let normal_bcx = bcx.fcx().build_new_block("normal-return");
+        let landing_pad = bcx.fcx().get_landing_pad(cleanup_scope);
+
+        let llresult = bcx.invoke(llfn, &llargs[..], normal_bcx.llbb(), landing_pad, None);
+        (llresult, normal_bcx)
+    } else {
+        let llresult = bcx.call(llfn, &llargs[..], None);
+        (llresult, bcx)
+    };
+    fn_ty.apply_attrs_callsite(llret);
+
+    if fn_ret.0.is_never() {
+        bcx.unreachable();
+    }
+
     bcx
 }