Move around code in cleanup for a more logical ordering, and fix comm…

…ents

src/librustc_trans/cleanup.rs

@@ -11,108 +11,12 @@
 //! ## The Cleanup module
 //!
 //! The cleanup module tracks what values need to be cleaned up as scopes
-//! are exited, either via panic or just normal control flow. The basic
-//! idea is that the function context maintains a stack of cleanup scopes
-//! that are pushed/popped as we traverse the AST tree. There is typically
-//! at least one cleanup scope per AST node; some AST nodes may introduce
-//! additional temporary scopes.
+//! are exited, either via panic or just normal control flow.
 //!
 //! Cleanup items can be scheduled into any of the scopes on the stack.
-//! Typically, when a scope is popped, we will also generate the code for
-//! each of its cleanups at that time. This corresponds to a normal exit
-//! from a block (for example, an expression completing evaluation
-//! successfully without panic). However, it is also possible to pop a
-//! block *without* executing its cleanups; this is typically used to
-//! guard intermediate values that must be cleaned up on panic, but not
-//! if everything goes right. See the section on custom scopes below for
-//! more details.
-//!
-//! Cleanup scopes come in three kinds:
-//!
-//! - **AST scopes:** each AST node in a function body has a corresponding
-//!   AST scope. We push the AST scope when we start generate code for an AST
-//!   node and pop it once the AST node has been fully generated.
-//! - **Loop scopes:** loops have an additional cleanup scope. Cleanups are
-//!   never scheduled into loop scopes; instead, they are used to record the
-//!   basic blocks that we should branch to when a `continue` or `break` statement
-//!   is encountered.
-//! - **Custom scopes:** custom scopes are typically used to ensure cleanup
-//!   of intermediate values.
-//!
-//! ### When to schedule cleanup
-//!
-//! Although the cleanup system is intended to *feel* fairly declarative,
-//! it's still important to time calls to `schedule_clean()` correctly.
-//! Basically, you should not schedule cleanup for memory until it has
-//! been initialized, because if an unwind should occur before the memory
-//! is fully initialized, then the cleanup will run and try to free or
-//! drop uninitialized memory. If the initialization itself produces
-//! byproducts that need to be freed, then you should use temporary custom
-//! scopes to ensure that those byproducts will get freed on unwind.  For
-//! example, an expression like `box foo()` will first allocate a box in the
-//! heap and then call `foo()` -- if `foo()` should panic, this box needs
-//! to be *shallowly* freed.
-//!
-//! ### Long-distance jumps
-//!
-//! In addition to popping a scope, which corresponds to normal control
-//! flow exiting the scope, we may also *jump out* of a scope into some
-//! earlier scope on the stack. This can occur in response to a `return`,
-//! `break`, or `continue` statement, but also in response to panic. In
-//! any of these cases, we will generate a series of cleanup blocks for
-//! each of the scopes that is exited. So, if the stack contains scopes A
-//! ... Z, and we break out of a loop whose corresponding cleanup scope is
-//! X, we would generate cleanup blocks for the cleanups in X, Y, and Z.
-//! After cleanup is done we would branch to the exit point for scope X.
-//! But if panic should occur, we would generate cleanups for all the
-//! scopes from A to Z and then resume the unwind process afterwards.
-//!
-//! To avoid generating tons of code, we cache the cleanup blocks that we
-//! create for breaks, returns, unwinds, and other jumps. Whenever a new
-//! cleanup is scheduled, though, we must clear these cached blocks. A
-//! possible improvement would be to keep the cached blocks but simply
-//! generate a new block which performs the additional cleanup and then
-//! branches to the existing cached blocks.
-//!
-//! ### AST and loop cleanup scopes
-//!
-//! AST cleanup scopes are pushed when we begin and end processing an AST
-//! node. They are used to house cleanups related to rvalue temporary that
-//! get referenced (e.g., due to an expression like `&Foo()`). Whenever an
-//! AST scope is popped, we always trans all the cleanups, adding the cleanup
-//! code after the postdominator of the AST node.
-//!
-//! AST nodes that represent breakable loops also push a loop scope; the
-//! loop scope never has any actual cleanups, it's just used to point to
-//! the basic blocks where control should flow after a "continue" or
-//! "break" statement. Popping a loop scope never generates code.
-//!
-//! ### Custom cleanup scopes
-//!
-//! Custom cleanup scopes are used for a variety of purposes. The most
-//! common though is to handle temporary byproducts, where cleanup only
-//! needs to occur on panic. The general strategy is to push a custom
-//! cleanup scope, schedule *shallow* cleanups into the custom scope, and
-//! then pop the custom scope (without transing the cleanups) when
-//! execution succeeds normally. This way the cleanups are only trans'd on
-//! unwind, and only up until the point where execution succeeded, at
-//! which time the complete value should be stored in an lvalue or some
-//! other place where normal cleanup applies.
-//!
-//! To spell it out, here is an example. Imagine an expression `box expr`.
-//! We would basically:
-//!
-//! 1. Push a custom cleanup scope C.
-//! 2. Allocate the box.
-//! 3. Schedule a shallow free in the scope C.
-//! 4. Trans `expr` into the box.
-//! 5. Pop the scope C.
-//! 6. Return the box as an rvalue.
-//!
-//! This way, if a panic occurs while transing `expr`, the custom
-//! cleanup scope C is pushed and hence the box will be freed. The trans
-//! code for `expr` itself is responsible for freeing any other byproducts
-//! that may be in play.
+//! Typically, when a scope is finished, we generate the cleanup code. This
+//! corresponds to a normal exit from a block (for example, an expression
+//! completing evaluation successfully without panic).
 
 use llvm::{BasicBlockRef, ValueRef};
 use base::{self, Lifetime};
@@ -131,9 +35,17 @@ pub struct CleanupScope<'tcx> {
     pub landing_pad: Option<BasicBlockRef>,
 }
 
-#[derive(Copy, Clone, Debug)]
-pub struct CustomScopeIndex {
-    index: usize
+#[derive(Copy, Clone)]
+pub struct DropValue<'tcx> {
+    val: ValueRef,
+    ty: Ty<'tcx>,
+    skip_dtor: bool,
+}
+
+impl<'tcx> DropValue<'tcx> {
+    fn trans<'blk>(&self, funclet: Option<&'blk Funclet>, bcx: &BlockAndBuilder<'blk, 'tcx>) {
+        glue::call_drop_glue(bcx, self.val, self.ty, self.skip_dtor, funclet)
+    }
 }
 
 #[derive(Copy, Clone, Debug)]
@@ -142,6 +54,44 @@ enum UnwindKind {
     CleanupPad(ValueRef),
 }
 
+impl UnwindKind {
+    /// Generates a branch going from `bcx` to `to_llbb` where `self` is
+    /// the exit label attached to the start of `bcx`.
+    ///
+    /// Transitions from an exit label to other exit labels depend on the type
+    /// of label. For example with MSVC exceptions unwind exit labels will use
+    /// the `cleanupret` instruction instead of the `br` instruction.
+    fn branch(&self, bcx: &BlockAndBuilder, to_llbb: BasicBlockRef) {
+        match *self {
+            UnwindKind::CleanupPad(pad) => {
+                bcx.cleanup_ret(pad, Some(to_llbb));
+            }
+            UnwindKind::LandingPad => {
+                bcx.br(to_llbb);
+            }
+        }
+    }
+
+    fn get_funclet(&self, bcx: &BlockAndBuilder) -> Option<Funclet> {
+        match *self {
+            UnwindKind::CleanupPad(_) => {
+                let pad = bcx.cleanup_pad(None, &[]);
+                Funclet::msvc(pad)
+            },
+            UnwindKind::LandingPad => Funclet::gnu(),
+        }
+    }
+}
+
+impl PartialEq for UnwindKind {
+    fn eq(&self, label: &UnwindKind) -> bool {
+        match (*self, *label) {
+            (UnwindKind::LandingPad, UnwindKind::LandingPad) |
+            (UnwindKind::CleanupPad(..), UnwindKind::CleanupPad(..)) => true,
+            _ => false,
+        }
+    }
+}
 impl<'blk, 'tcx> FunctionContext<'blk, 'tcx> {
     pub fn trans_scope(
         &self,
@@ -186,9 +136,7 @@ impl<'blk, 'tcx> FunctionContext<'blk, 'tcx> {
         };
 
         debug!("schedule_drop_adt_contents(val={:?}, ty={:?}) skip_dtor={}",
-               Value(val),
-               ty,
-               drop.skip_dtor);
+               Value(val), ty, drop.skip_dtor);
 
         Some(CleanupScope::new(self, drop))
     }
@@ -259,27 +207,9 @@ impl<'tcx> CleanupScope<'tcx> {
             UnwindKind::LandingPad
         };
 
-        // Generate the cleanup block and branch to it.
-        let cleanup_llbb = CleanupScope::trans_cleanups_to_exit_scope(fcx, val, drop_val);
-        val.branch(&mut pad_bcx, cleanup_llbb);
-
-        return pad_bcx.llbb();
-    }
-
-    /// Used when the caller wishes to jump to an early exit, such as a return,
-    /// 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<'a>(
-        fcx: &FunctionContext<'a, 'tcx>,
-        label: UnwindKind,
-        drop_val: &DropValue<'tcx>
-    ) -> BasicBlockRef {
-        debug!("trans_cleanups_to_exit_scope label={:?}`", label);
-
         // Generate a block that will resume unwinding to the calling function
         let bcx = fcx.build_new_block("resume");
-        match label {
+        match val {
             UnwindKind::LandingPad => {
                 let addr = fcx.landingpad_alloca.get().unwrap();
                 let lp = bcx.load(addr);
@@ -299,68 +229,14 @@ impl<'tcx> CleanupScope<'tcx> {
         let mut cleanup = fcx.build_new_block("clean_custom_");
 
         // Insert cleanup instructions into the cleanup block
-        drop_val.trans(label.get_funclet(&cleanup).as_ref(), &cleanup);
+        drop_val.trans(val.get_funclet(&cleanup).as_ref(), &cleanup);
 
         // Insert instruction into cleanup block to branch to the exit
-        label.branch(&mut cleanup, bcx.llbb());
-
-        debug!("trans_cleanups_to_exit_scope: llbb={:?}", cleanup.llbb());
-
-        cleanup.llbb()
-    }
-}
-
-impl UnwindKind {
-    /// Generates a branch going from `bcx` to `to_llbb` where `self` is
-    /// the exit label attached to the start of `bcx`.
-    ///
-    /// Transitions from an exit label to other exit labels depend on the type
-    /// of label. For example with MSVC exceptions unwind exit labels will use
-    /// the `cleanupret` instruction instead of the `br` instruction.
-    fn branch(&self, bcx: &BlockAndBuilder, to_llbb: BasicBlockRef) {
-        match *self {
-            UnwindKind::CleanupPad(pad) => {
-                bcx.cleanup_ret(pad, Some(to_llbb));
-            }
-            UnwindKind::LandingPad => {
-                bcx.br(to_llbb);
-            }
-        }
-    }
-
-    fn get_funclet(&self, bcx: &BlockAndBuilder) -> Option<Funclet> {
-        match *self {
-            UnwindKind::CleanupPad(_) => {
-                let pad = bcx.cleanup_pad(None, &[]);
-                Funclet::msvc(pad)
-            },
-            UnwindKind::LandingPad => Funclet::gnu(),
-        }
-    }
-}
+        val.branch(&mut cleanup, bcx.llbb());
 
-impl PartialEq for UnwindKind {
-    fn eq(&self, label: &UnwindKind) -> bool {
-        match (*self, *label) {
-            (UnwindKind::LandingPad, UnwindKind::LandingPad) |
-            (UnwindKind::CleanupPad(..), UnwindKind::CleanupPad(..)) => true,
-            _ => false,
-        }
-    }
-}
-
-///////////////////////////////////////////////////////////////////////////
-// Cleanup types
+        // Branch into the cleanup block
+        val.branch(&mut pad_bcx, cleanup.llbb());
 
-#[derive(Copy, Clone)]
-pub struct DropValue<'tcx> {
-    val: ValueRef,
-    ty: Ty<'tcx>,
-    skip_dtor: bool,
-}
-
-impl<'tcx> DropValue<'tcx> {
-    fn trans<'blk>(&self, funclet: Option<&'blk Funclet>, bcx: &BlockAndBuilder<'blk, 'tcx>) {
-        glue::call_drop_glue(bcx, self.val, self.ty, self.skip_dtor, funclet)
+        return pad_bcx.llbb();
     }
 }