Remove FixedArray::GetValueChecked

This method is rarely used, and has several problems:
1) It CHECKs that the value is not undefined, then creates a
   {Handle<T>} which again DCHECKs that the value is of type {T}.
2) It is called on a raw {FixedArray} but returns a handle.
3) It is often used when no handle is actually needed, adding
   unnecessary overhead.
4) It adds complexity and hides actual checks and handlification.

This CL removes that method, replacing some uses by explicit CHECKs (in
tests) and relying on the DCHECKs in the casts otherwise.

R=mstarzinger@chromium.org

Bug: v8:9183
Change-Id: I90ff59e8b78c909a9a207029d8cc9ab16c0c7b56
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1621939
Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
Reviewed-by: Michael Starzinger <mstarzinger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#61710}

src/objects/fixed-array-inl.h

@@ -128,13 +128,6 @@ Handle<Object> FixedArray::get(FixedArray array, int index, Isolate* isolate) {
   return handle(array->get(index), isolate);
 }
 
-template <class T>
-Handle<T> FixedArray::GetValueChecked(Isolate* isolate, int index) const {
-  Object obj = get(index);
-  CHECK(!obj->IsUndefined(isolate));
-  return Handle<T>(T::cast(obj), isolate);
-}
-
 bool FixedArray::is_the_hole(Isolate* isolate, int index) {
   return get(index)->IsTheHole(isolate);
 }

src/objects/fixed-array.h

@@ -116,11 +116,6 @@ class FixedArray : public FixedArrayBase {
   static inline Handle<Object> get(FixedArray array, int index,
                                    Isolate* isolate);
 
-  // CHECK that the element at {index} is not undefined and return that element
-  // in a handle.
-  template <class T>
-  Handle<T> GetValueChecked(Isolate* isolate, int index) const;
-
   // Return a grown copy if the index is bigger than the array's length.
   V8_EXPORT_PRIVATE static Handle<FixedArray> SetAndGrow(
       Isolate* isolate, Handle<FixedArray> array, int index,

src/wasm/module-instantiate.cc

@@ -1413,8 +1413,8 @@ void InstanceBuilder::ProcessExports(Handle<WasmInstanceObject> instance) {
           Handle<FixedArray> buffers_array(
               instance->imported_mutable_globals_buffers(), isolate_);
           if (ValueTypes::IsReferenceType(global.type)) {
-            tagged_buffer = buffers_array->GetValueChecked<FixedArray>(
+            tagged_buffer = handle(
-                isolate_, global.index);
+                FixedArray::cast(buffers_array->get(global.index)), isolate_);
             // For anyref globals we store the relative offset in the
             // imported_mutable_globals array instead of an absolute address.
             Address addr = instance->imported_mutable_globals()[global.index];
@@ -1422,8 +1422,9 @@ void InstanceBuilder::ProcessExports(Handle<WasmInstanceObject> instance) {
                                 std::numeric_limits<uint32_t>::max()));
             offset = static_cast<uint32_t>(addr);
           } else {
-            untagged_buffer = buffers_array->GetValueChecked<JSArrayBuffer>(
+            untagged_buffer =
-                isolate_, global.index);
+                handle(JSArrayBuffer::cast(buffers_array->get(global.index)),
+                       isolate_);
             Address global_addr =
                 instance->imported_mutable_globals()[global.index];
 

test/cctest/compiler/test-code-generator.cc

@@ -263,7 +263,7 @@ void PrintStateValue(std::ostream& os, Isolate* isolate, Handle<Object> value,
       FixedArray vector = FixedArray::cast(*value);
       os << "[";
       for (int lane = 0; lane < 4; lane++) {
-        os << Smi::cast(*vector->GetValueChecked<Smi>(isolate, lane))->value();
+        os << Smi::cast(vector->get(lane))->value();
         if (lane < 3) {
           os << ", ";
         }
@@ -752,8 +752,7 @@ class TestEnvironment : public HandleAndZoneScope {
         state_out->set(to_index, *constant_value);
       } else {
         int from_index = OperandToStatePosition(AllocatedOperand::cast(from));
-        state_out->set(to_index, *state_out->GetValueChecked<Object>(
+        state_out->set(to_index, state_out->get(from_index));
-                                     main_isolate(), from_index));
       }
     }
     return state_out;
@@ -773,10 +772,8 @@ class TestEnvironment : public HandleAndZoneScope {
           OperandToStatePosition(AllocatedOperand::cast(swap->destination()));
       int rhs_index =
           OperandToStatePosition(AllocatedOperand::cast(swap->source()));
-      Handle<Object> lhs =
+      Handle<Object> lhs{state_out->get(lhs_index), main_isolate()};
-          state_out->GetValueChecked<Object>(main_isolate(), lhs_index);
+      Handle<Object> rhs{state_out->get(rhs_index), main_isolate()};
-      Handle<Object> rhs =
-          state_out->GetValueChecked<Object>(main_isolate(), rhs_index);
       state_out->set(lhs_index, *rhs);
       state_out->set(rhs_index, *lhs);
     }
@@ -786,10 +783,8 @@ class TestEnvironment : public HandleAndZoneScope {
   // Compare the given state with a reference.
   void CheckState(Handle<FixedArray> actual, Handle<FixedArray> expected) {
     for (int i = 0; i < static_cast<int>(layout_.size()); i++) {
-      Handle<Object> actual_value =
+      Handle<Object> actual_value{actual->get(i), main_isolate()};
-          actual->GetValueChecked<Object>(main_isolate(), i);
+      Handle<Object> expected_value{expected->get(i), main_isolate()};
-      Handle<Object> expected_value =
-          expected->GetValueChecked<Object>(main_isolate(), i);
       if (!CompareValues(actual_value, expected_value,
                          layout_[i].representation())) {
         std::ostringstream expected_str;
@@ -812,13 +807,11 @@ class TestEnvironment : public HandleAndZoneScope {
         return actual->StrictEquals(*expected);
       case MachineRepresentation::kSimd128:
         for (int lane = 0; lane < 4; lane++) {
-          Handle<Smi> actual_lane =
+          int actual_lane =
-              FixedArray::cast(*actual)->GetValueChecked<Smi>(main_isolate(),
+              Smi::cast(FixedArray::cast(*actual)->get(lane))->value();
-                                                              lane);
+          int expected_lane =
-          Handle<Smi> expected_lane =
+              Smi::cast(FixedArray::cast(*expected)->get(lane))->value();
-              FixedArray::cast(*expected)->GetValueChecked<Smi>(main_isolate(),
+          if (actual_lane != expected_lane) {
-                                                                lane);
-          if (*actual_lane != *expected_lane) {
             return false;
           }
         }