Revision: 7088
Author: [email protected]
Date: Tue Mar  8 03:14:25 2011
Log: Ensure the result is used for the remaining calls to SetElement

Now mark SetElement as must use result

The debugger runs inside its own context so there should be no setters hit. Which is the reason for the live-edit code asserting non-empty handles.
Review URL: http://codereview.chromium.org/6621042
http://code.google.com/p/v8/source/detail?r=7088

Modified:
 /branches/bleeding_edge/src/debug.cc
 /branches/bleeding_edge/src/handles.h
 /branches/bleeding_edge/src/liveedit.cc

=======================================
--- /branches/bleeding_edge/src/debug.cc        Thu Mar  3 16:21:52 2011
+++ /branches/bleeding_edge/src/debug.cc        Tue Mar  8 03:14:25 2011
@@ -1003,28 +1003,24 @@
 // triggered. This function returns a JSArray with the break point objects
 // which is triggered.
Handle<Object> Debug::CheckBreakPoints(Handle<Object> break_point_objects) { + // Count the number of break points hit. If there are multiple break points
+  // they are in a FixedArray.
+  Handle<FixedArray> break_points_hit;
   int break_points_hit_count = 0;
-  Handle<JSArray> break_points_hit = Factory::NewJSArray(1);
-
-  // If there are multiple break points they are in a FixedArray.
   ASSERT(!break_point_objects->IsUndefined());
   if (break_point_objects->IsFixedArray()) {
     Handle<FixedArray> array(FixedArray::cast(*break_point_objects));
+    break_points_hit = Factory::NewFixedArray(array->length());
     for (int i = 0; i < array->length(); i++) {
       Handle<Object> o(array->get(i));
       if (CheckBreakPoint(o)) {
-        SetElement(break_points_hit,
-                   break_points_hit_count++,
-                   o,
-                   kNonStrictMode);
+        break_points_hit->set(break_points_hit_count++, *o);
       }
     }
   } else {
+    break_points_hit = Factory::NewFixedArray(1);
     if (CheckBreakPoint(break_point_objects)) {
-      SetElement(break_points_hit,
-                 break_points_hit_count++,
-                 break_point_objects,
-                 kNonStrictMode);
+ break_points_hit->set(break_points_hit_count++, *break_point_objects);
     }
   }

@@ -1032,7 +1028,10 @@
   if (break_points_hit_count == 0) {
     return Factory::undefined_value();
   }
-  return break_points_hit;
+  // Return break points hit as a JSArray.
+ Handle<JSArray> result = Factory::NewJSArrayWithElements(break_points_hit);
+  result->set_length(Smi::FromInt(break_points_hit_count));
+  return result;
 }


@@ -1043,7 +1042,7 @@
   // Ignore check if break point object is not a JSObject.
   if (!break_point_object->IsJSObject()) return true;

-  // Get the function CheckBreakPoint (defined in debug.js).
+  // Get the function IsBreakPointTriggered (defined in debug-debugger.js).
   Handle<String> is_break_point_triggered_symbol =
       Factory::LookupAsciiSymbol("IsBreakPointTriggered");
   Handle<JSFunction> check_break_point =
=======================================
--- /branches/bleeding_edge/src/handles.h       Thu Mar  3 16:21:52 2011
+++ /branches/bleeding_edge/src/handles.h       Tue Mar  8 03:14:25 2011
@@ -264,10 +264,10 @@
                                           PropertyAttributes attributes,
                                           StrictModeFlag strict_mode);

-Handle<Object> SetElement(Handle<JSObject> object,
-                          uint32_t index,
-                          Handle<Object> value,
-                          StrictModeFlag strict_mode);
+MUST_USE_RESULT Handle<Object> SetElement(Handle<JSObject> object,
+                                          uint32_t index,
+                                          Handle<Object> value,
+                                          StrictModeFlag strict_mode);

 Handle<Object> SetOwnElement(Handle<JSObject> object,
                              uint32_t index,
=======================================
--- /branches/bleeding_edge/src/liveedit.cc     Thu Mar  3 16:21:52 2011
+++ /branches/bleeding_edge/src/liveedit.cc     Tue Mar  8 03:14:25 2011
@@ -46,6 +46,18 @@

 #ifdef ENABLE_DEBUGGER_SUPPORT

+
+void SetElementNonStrict(Handle<JSObject> object,
+                         uint32_t index,
+                         Handle<Object> value) {
+  // Ignore return value from SetElement. It can only be a failure if there
+ // are element setters causing exceptions and the debugger context has none
+  // of these.
+  Handle<Object> no_failure;
+  no_failure = SetElement(object, index, value, kNonStrictMode);
+  ASSERT(!no_failure.is_null());
+  USE(no_failure);
+}

 // A simple implementation of dynamic programming algorithm. It solves
// the problem of finding the difference of 2 arrays. It uses a table of results
@@ -286,18 +298,15 @@
   }

void WriteChunk(int char_pos1, int char_pos2, int char_len1, int char_len2) {
-    SetElement(array_,
-               current_size_,
-               Handle<Object>(Smi::FromInt(char_pos1)),
-               kNonStrictMode);
-    SetElement(array_,
-               current_size_ + 1,
-               Handle<Object>(Smi::FromInt(char_pos1 + char_len1)),
-               kNonStrictMode);
-    SetElement(array_,
-               current_size_ + 2,
-               Handle<Object>(Smi::FromInt(char_pos2 + char_len2)),
-               kNonStrictMode);
+    SetElementNonStrict(array_,
+                       current_size_,
+                       Handle<Object>(Smi::FromInt(char_pos1)));
+    SetElementNonStrict(array_,
+                        current_size_ + 1,
+ Handle<Object>(Smi::FromInt(char_pos1 + char_len1)));
+    SetElementNonStrict(array_,
+                        current_size_ + 2,
+ Handle<Object>(Smi::FromInt(char_pos2 + char_len2)));
     current_size_ += 3;
   }

@@ -552,13 +561,12 @@

  protected:
   void SetField(int field_position, Handle<Object> value) {
-    SetElement(array_, field_position, value, kNonStrictMode);
+    SetElementNonStrict(array_, field_position, value);
   }
   void SetSmiValueField(int field_position, int value) {
-    SetElement(array_,
-               field_position,
-               Handle<Smi>(Smi::FromInt(value)),
-               kNonStrictMode);
+    SetElementNonStrict(array_,
+                        field_position,
+                        Handle<Smi>(Smi::FromInt(value)));
   }
   Object* GetField(int field_position) {
     return array_->GetElementNoExceptionThrown(field_position);
@@ -697,7 +705,7 @@
                               fun->end_position(), fun->num_parameters(),
                               current_parent_index_);
     current_parent_index_ = len_;
-    SetElement(result_, len_, info.GetJSArray(), kNonStrictMode);
+    SetElementNonStrict(result_, len_, info.GetJSArray());
     len_++;
   }

@@ -777,16 +785,19 @@
         list[k] = list[l];
       }
       for (int i = 0; i < j; i++) {
-        SetElement(scope_info_list, scope_info_length,
-                   list[i]->name(), kNonStrictMode);
+        SetElementNonStrict(scope_info_list,
+                            scope_info_length,
+                            list[i]->name());
         scope_info_length++;
-        SetElement(scope_info_list, scope_info_length,
-                   Handle<Smi>(Smi::FromInt(list[i]->AsSlot()->index())),
-                   kNonStrictMode);
+        SetElementNonStrict(
+            scope_info_list,
+            scope_info_length,
+            Handle<Smi>(Smi::FromInt(list[i]->AsSlot()->index())));
         scope_info_length++;
       }
-      SetElement(scope_info_list, scope_info_length,
-                 Handle<Object>(Heap::null_value()), kNonStrictMode);
+      SetElementNonStrict(scope_info_list,
+                          scope_info_length,
+                          Handle<Object>(Heap::null_value()));
       scope_info_length++;

       outer_scope = outer_scope->outer_scope();
@@ -829,7 +840,7 @@
     Handle<String> name_handle(String::cast(info->name()));
     info_wrapper.SetProperties(name_handle, info->start_position(),
                                info->end_position(), info);
-    SetElement(array, i, info_wrapper.GetJSArray(), kNonStrictMode);
+    SetElementNonStrict(array, i, info_wrapper.GetJSArray());
   }
 }

@@ -1327,7 +1338,7 @@
         SharedFunctionInfo::cast(wrapper->value()));

     if (function->shared() == *shared || IsInlined(*function, *shared)) {
- SetElement(result, i, Handle<Smi>(Smi::FromInt(status)), kNonStrictMode);
+      SetElementNonStrict(result, i, Handle<Smi>(Smi::FromInt(status)));
       return true;
     }
   }
@@ -1532,7 +1543,7 @@
         Smi::FromInt(LiveEdit::FUNCTION_BLOCKED_ON_ACTIVE_STACK)) {
       Handle<Object> replaced(
           Smi::FromInt(LiveEdit::FUNCTION_REPLACED_ON_ACTIVE_STACK));
-      SetElement(result, i, replaced, kNonStrictMode);
+      SetElementNonStrict(result, i, replaced);
     }
   }
   return NULL;
@@ -1572,9 +1583,10 @@

   // Fill the default values.
   for (int i = 0; i < len; i++) {
-    SetElement(result, i,
-               Handle<Smi>(Smi::FromInt(FUNCTION_AVAILABLE_FOR_PATCH)),
-               kNonStrictMode);
+    SetElementNonStrict(
+        result,
+        i,
+        Handle<Smi>(Smi::FromInt(FUNCTION_AVAILABLE_FOR_PATCH)));
   }


@@ -1593,7 +1605,7 @@
     // Add error message as an array extra element.
Vector<const char> vector_message(error_message, StrLength(error_message));
     Handle<String> str = Factory::NewStringFromAscii(vector_message);
-    SetElement(result, len, str, kNonStrictMode);
+    SetElementNonStrict(result, len, str);
   }
   return result;
 }

--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev

Reply via email to