Rename member variables and functions and clarify comments to try to

make the operation of LabelShadows more understandable.
Review URL: http://codereview.chromium.org/7868

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@556 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

diff --git a/src/assembler.h b/src/assembler.h
index e5b9f04..137c37c 100644 (file)
--- a/src/assembler.h
+++ b/src/assembler.h
@@ -48,7 +48,7 @@ namespace v8 { namespace internal {
 // unknown pc location. Assembler::bind() is used to bind a label to the
 // current pc. A label can be bound only once.
 
-class Label : public ZoneObject {  // ShadowLables are dynamically allocated.
+class Label : public ZoneObject {  // LabelShadows are dynamically allocated.
  public:
   INLINE(Label())                 { Unuse(); }
   INLINE(~Label())                { ASSERT(!is_linked()); }
@@ -87,17 +87,19 @@ class Label : public ZoneObject {  // ShadowLables are dynamically allocated.
 };
 
 
-// A LabelShadow is a label that temporarily shadows another label. It
-// is used to catch linking and binding of labels in certain scopes,
-// e.g. try blocks. LabelShadows are themselves labels which can be
-// used (only) after they are not shadowing anymore.
-class LabelShadow: public Label {
+// A LabelShadow represents a label that is temporarily shadowed by another
+// label (represented by the original label during shadowing). They are used
+// to catch jumps to labels in certain contexts, e.g. try blocks.  After
+// shadowing ends, the formerly shadowed label is again represented by the
+// original label and the LabelShadow can be used as a label in its own
+// right, representing the formerly shadowing label.
+class LabelShadow : public Label {
  public:
-  explicit LabelShadow(Label* shadowed) {
-    ASSERT(shadowed != NULL);
-    shadowed_ = shadowed;
-    shadowed_pos_ = shadowed->pos_;
-    shadowed->Unuse();
+  explicit LabelShadow(Label* original) {
+    ASSERT(original != NULL);
+    original_label_ = original;
+    original_pos_ = original->pos_;
+    original->Unuse();
 #ifdef DEBUG
     is_shadowing_ = true;
 #endif
@@ -109,18 +111,23 @@ class LabelShadow: public Label {
 
   void StopShadowing() {
     ASSERT(is_shadowing_ && is_unused());
-    pos_ = shadowed_->pos_;
-    shadowed_->pos_ = shadowed_pos_;
+    pos_ = original_label_->pos_;
+    original_label_->pos_ = original_pos_;
 #ifdef DEBUG
     is_shadowing_ = false;
 #endif
   }
 
-  Label* shadowed() const { return shadowed_; }
+  Label* original_label() const { return original_label_; }
 
  private:
-  Label* shadowed_;
-  int shadowed_pos_;
+  // During shadowing, the currently shadowing label.  After shadowing, the
+  // label that was shadowed.
+  Label* original_label_;
+
+  // During shadowing, the saved state of the original label.
+  int original_pos_;
+
 #ifdef DEBUG
   bool is_shadowing_;
 #endif

diff --git a/src/codegen-arm.cc b/src/codegen-arm.cc
index 8832b83..2d8923b 100644 (file)
--- a/src/codegen-arm.cc
+++ b/src/codegen-arm.cc
@@ -1634,9 +1634,13 @@ void CodeGenerator::VisitTryCatch(TryCatch* node) {
 
   __ PushTryHandler(IN_JAVASCRIPT, TRY_CATCH_HANDLER);
 
-  // Introduce shadow labels for all escapes from the try block,
-  // including returns. We should probably try to unify the escaping
-  // labels and the return label.
+  // Shadow the labels for all escapes from the try block, including
+  // returns. During shadowing, the original label is hidden as the
+  // LabelShadow and operations on the original actually affect the
+  // shadowing label.
+  //
+  // We should probably try to unify the escaping labels and the return
+  // label.
   int nof_escapes = node->escaping_labels()->length();
   List<LabelShadow*> shadows(1 + nof_escapes);
   shadows.Add(new LabelShadow(&function_return_));
@@ -1649,6 +1653,8 @@ void CodeGenerator::VisitTryCatch(TryCatch* node) {
   __ pop(r0);  // Discard the result.
 
   // Stop the introduced shadowing and count the number of required unlinks.
+  // After shadowing stops, the original labels are unshadowed and the
+  // LabelShadows represent the formerly shadowing labels.
   int nof_unlinks = 0;
   for (int i = 0; i <= nof_escapes; i++) {
     shadows[i]->StopShadowing();
@@ -1667,7 +1673,8 @@ void CodeGenerator::VisitTryCatch(TryCatch* node) {
   // Code slot popped.
   if (nof_unlinks > 0) __ b(&exit);
 
-  // Generate unlink code for all used shadow labels.
+  // Generate unlink code for the (formerly) shadowing labels that have been
+  // jumped to.
   for (int i = 0; i <= nof_escapes; i++) {
     if (shadows[i]->is_linked()) {
       // Unlink from try chain;
@@ -1684,7 +1691,7 @@ void CodeGenerator::VisitTryCatch(TryCatch* node) {
       __ add(sp, sp, Operand(StackHandlerConstants::kSize - kPointerSize));
       // Code slot popped.
 
-      __ b(shadows[i]->shadowed());
+      __ b(shadows[i]->original_label());
     }
   }
 
@@ -1715,9 +1722,12 @@ void CodeGenerator::VisitTryFinally(TryFinally* node) {
 
   __ PushTryHandler(IN_JAVASCRIPT, TRY_FINALLY_HANDLER);
 
-  // Introduce shadow labels for all escapes from the try block,
-  // including returns. We should probably try to unify the escaping
-  // labels and the return label.
+  // Shadow the labels for all escapes from the try block, including
+  // returns.  Shadowing hides the original label as the LabelShadow and
+  // operations on the original actually affect the shadowing label.
+  //
+  // We should probably try to unify the escaping labels and the return
+  // label.
   int nof_escapes = node->escaping_labels()->length();
   List<LabelShadow*> shadows(1 + nof_escapes);
   shadows.Add(new LabelShadow(&function_return_));
@@ -1728,8 +1738,9 @@ void CodeGenerator::VisitTryFinally(TryFinally* node) {
   // Generate code for the statements in the try block.
   VisitStatements(node->try_block()->statements());
 
-  // Stop the introduced shadowing and count the number of required
-  // unlinks.
+  // Stop the introduced shadowing and count the number of required unlinks.
+  // After shadowing stops, the original labels are unshadowed and the
+  // LabelShadows represent the formerly shadowing labels.
   int nof_unlinks = 0;
   for (int i = 0; i <= nof_escapes; i++) {
     shadows[i]->StopShadowing();
@@ -1742,14 +1753,17 @@ void CodeGenerator::VisitTryFinally(TryFinally* node) {
   __ mov(r2, Operand(Smi::FromInt(FALLING)));
   if (nof_unlinks > 0) __ b(&unlink);
 
-  // Generate code that sets the state for all used shadow labels.
+  // Generate code to set the state for the (formerly) shadowing labels that
+  // have been jumped to.
   for (int i = 0; i <= nof_escapes; i++) {
     if (shadows[i]->is_linked()) {
       __ bind(shadows[i]);
-      if (shadows[i]->shadowed() == &function_return_) {
-        __ push(r0);  // Materialize the return value on the stack
+      if (shadows[i]->original_label() == &function_return_) {
+        // If this label shadowed the function return, materialize the
+        // return value on the stack.
+        __ push(r0);
       } else {
-        // Fake TOS for break and continue (not return).
+        // Fake TOS for labels that shadowed breaks and continues.
         __ mov(r0, Operand(Factory::undefined_value()));
         __ push(r0);
       }
@@ -1796,18 +1810,18 @@ void CodeGenerator::VisitTryFinally(TryFinally* node) {
   __ pop(r0);
   break_stack_height_ -= kFinallyStackSize;
 
-  // Generate code that jumps to the right destination for all used
-  // shadow labels.
+  // Generate code to jump to the right destination for all used (formerly)
+  // shadowing labels.
   for (int i = 0; i <= nof_escapes; i++) {
     if (shadows[i]->is_bound()) {
       __ cmp(r2, Operand(Smi::FromInt(JUMPING + i)));
-      if (shadows[i]->shadowed() != &function_return_) {
+      if (shadows[i]->original_label() != &function_return_) {
         Label next;
         __ b(ne, &next);
-        __ b(shadows[i]->shadowed());
+        __ b(shadows[i]->original_label());
         __ bind(&next);
       } else {
-        __ b(eq, shadows[i]->shadowed());
+        __ b(eq, shadows[i]->original_label());
       }
     }
   }

diff --git a/src/codegen-ia32.cc b/src/codegen-ia32.cc
index a2f8d64..f000725 100644 (file)
--- a/src/codegen-ia32.cc
+++ b/src/codegen-ia32.cc
@@ -2000,9 +2000,13 @@ void CodeGenerator::VisitTryCatch(TryCatch* node) {
   // TODO(1222589): remove the reliance of PushTryHandler on a cached TOS
   frame_->Push(eax);  //
 
-  // Introduce shadow labels for all escapes from the try block,
-  // including returns. We should probably try to unify the escaping
-  // labels and the return label.
+  // Shadow the labels for all escapes from the try block, including
+  // returns.  During shadowing, the original label is hidden as the
+  // LabelShadow and operations on the original actually affect the
+  // shadowing label.
+  //
+  // We should probably try to unify the escaping labels and the return
+  // label.
   int nof_escapes = node->escaping_labels()->length();
   List<LabelShadow*> shadows(1 + nof_escapes);
   shadows.Add(new LabelShadow(&function_return_));
@@ -2017,6 +2021,8 @@ void CodeGenerator::VisitTryCatch(TryCatch* node) {
   is_inside_try_ = was_inside_try;
 
   // Stop the introduced shadowing and count the number of required unlinks.
+  // After shadowing stops, the original labels are unshadowed and the
+  // LabelShadows represent the formerly shadowing labels.
   int nof_unlinks = 0;
   for (int i = 0; i <= nof_escapes; i++) {
     shadows[i]->StopShadowing();
@@ -2042,7 +2048,8 @@ void CodeGenerator::VisitTryCatch(TryCatch* node) {
   // next_sp popped.
   if (nof_unlinks > 0) __ jmp(&exit);
 
-  // Generate unlink code for all used shadow labels.
+  // Generate unlink code for the (formerly) shadowing labels that have been
+  // jumped to.
   for (int i = 0; i <= nof_escapes; i++) {
     if (shadows[i]->is_linked()) {
       // Unlink from try chain; be careful not to destroy the TOS.
@@ -2058,7 +2065,7 @@ void CodeGenerator::VisitTryCatch(TryCatch* node) {
       frame_->Pop(Operand::StaticVariable(handler_address));
       frame_->Drop(StackHandlerConstants::kSize / kPointerSize - 1);
       // next_sp popped.
-      __ jmp(shadows[i]->shadowed());
+      __ jmp(shadows[i]->original_label());
     }
   }
 
@@ -2091,9 +2098,13 @@ void CodeGenerator::VisitTryFinally(TryFinally* node) {
   // TODO(1222589): remove the reliance of PushTryHandler on a cached TOS
   frame_->Push(eax);
 
-  // Introduce shadow labels for all escapes from the try block,
-  // including returns. We should probably try to unify the escaping
-  // labels and the return label.
+  // Shadow the labels for all escapes from the try block, including
+  // returns.  During shadowing, the original label is hidden as the
+  // LabelShadow and operations on the original actually affect the
+  // shadowing label.
+  //
+  // We should probably try to unify the escaping labels and the return
+  // label.
   int nof_escapes = node->escaping_labels()->length();
   List<LabelShadow*> shadows(1 + nof_escapes);
   shadows.Add(new LabelShadow(&function_return_));
@@ -2107,8 +2118,9 @@ void CodeGenerator::VisitTryFinally(TryFinally* node) {
   VisitStatements(node->try_block()->statements());
   is_inside_try_ = was_inside_try;
 
-  // Stop the introduced shadowing and count the number of required
-  // unlinks.
+  // Stop the introduced shadowing and count the number of required unlinks.
+  // After shadowing stops, the original labels are unshadowed and the
+  // LabelShadows represent the formerly shadowing labels.
   int nof_unlinks = 0;
   for (int i = 0; i <= nof_escapes; i++) {
     shadows[i]->StopShadowing();
@@ -2120,15 +2132,17 @@ void CodeGenerator::VisitTryFinally(TryFinally* node) {
   __ Set(ecx, Immediate(Smi::FromInt(FALLING)));
   if (nof_unlinks > 0) __ jmp(&unlink);
 
-  // Generate code that sets the state for all used shadow labels.
+  // Generate code to set the state for the (formerly) shadowing labels that
+  // have been jumped to.
   for (int i = 0; i <= nof_escapes; i++) {
     if (shadows[i]->is_linked()) {
       __ bind(shadows[i]);
-      if (shadows[i]->shadowed() == &function_return_) {
-        // Materialize the return value on the stack.
+      if (shadows[i]->original_label() == &function_return_) {
+        // If this label shadowed the function return, materialize the
+        // return value on the stack.
         frame_->Push(eax);
       } else {
-        // Fake TOS for break and continue.
+        // Fake TOS for labels that shadowed breaks and continues.
         frame_->Push(Immediate(Factory::undefined_value()));
       }
       __ Set(ecx, Immediate(Smi::FromInt(JUMPING + i)));
@@ -2175,12 +2189,12 @@ void CodeGenerator::VisitTryFinally(TryFinally* node) {
   frame_->Pop(eax);
   break_stack_height_ -= kFinallyStackSize;
 
-  // Generate code that jumps to the right destination for all used
-  // shadow labels.
+  // Generate code to jump to the right destination for all used (formerly)
+  // shadowing labels.
   for (int i = 0; i <= nof_escapes; i++) {
     if (shadows[i]->is_bound()) {
       __ cmp(Operand(ecx), Immediate(Smi::FromInt(JUMPING + i)));
-      __ j(equal, shadows[i]->shadowed());
+      __ j(equal, shadows[i]->original_label());
     }
   }