[builtins] Remove most Builtins::Name usages in API

Using the Builtins::Name type doesn't give use any range safety benefits
over simply using int id's, and it complicates use sites by always
forcing a static_cast<Builtins::Name>(id).

Bug: v8:6624
Change-Id: Id5fcf6800c781c637145ab1d00d821f9ad473321
Reviewed-on: https://chromium-review.googlesource.com/650247
Commit-Queue: Jakob Gruber <jgruber@chromium.org>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#47823}

src/builtins/builtins.cc

@@ -155,8 +155,9 @@ void Builtins::set_builtin(int index, HeapObject* builtin) {
   builtins_[index] = builtin;
 }
 
-Handle<Code> Builtins::builtin_handle(Name name) {
-  return Handle<Code>(reinterpret_cast<Code**>(builtin_address(name)));
+Handle<Code> Builtins::builtin_handle(int index) {
+  DCHECK(IsBuiltinId(index));
+  return Handle<Code>(reinterpret_cast<Code**>(builtin_address(index)));
 }
 
 // static

src/builtins/builtins.h

@@ -74,17 +74,19 @@ class Builtins {
   // Used by BuiltinDeserializer.
   void set_builtin(int index, HeapObject* builtin);
 
-  Code* builtin(Name name) {
+  Code* builtin(int index) {
+    DCHECK(IsBuiltinId(index));
     // Code::cast cannot be used here since we access builtins
     // during the marking phase of mark sweep. See IC::Clear.
-    return reinterpret_cast<Code*>(builtins_[name]);
+    return reinterpret_cast<Code*>(builtins_[index]);
   }
 
-  Address builtin_address(Name name) {
-    return reinterpret_cast<Address>(&builtins_[name]);
+  Address builtin_address(int index) {
+    DCHECK(IsBuiltinId(index));
+    return reinterpret_cast<Address>(&builtins_[index]);
   }
 
-  V8_EXPORT_PRIVATE Handle<Code> builtin_handle(Name name);
+  V8_EXPORT_PRIVATE Handle<Code> builtin_handle(int index);
 
   // Used by lazy deserialization to determine whether a given builtin has been
   // deserialized. See the DeserializeLazy builtin.

src/isolate.cc

@@ -2682,7 +2682,7 @@ void PrintBuiltinSizes(Isolate* isolate) {
   for (int i = 0; i < Builtins::builtin_count; i++) {
     const char* name = builtins->name(i);
     const char* kind = Builtins::KindNameOf(i);
-    Code* code = builtins->builtin(static_cast<Builtins::Name>(i));
+    Code* code = builtins->builtin(i);
     PrintF(stdout, "%s Builtin, %s, %d\n", kind, name,
            code->instruction_size());
   }

src/runtime/runtime-internal.cc

@@ -514,22 +514,18 @@ RUNTIME_FUNCTION(Runtime_DeserializeLazy) {
   Handle<SharedFunctionInfo> shared(function->shared(), isolate);
   int builtin_id = shared->lazy_deserialization_builtin_id();
 
-#ifdef DEBUG
-  Builtins::Name builtin_name = static_cast<Builtins::Name>(builtin_id);
-
   // At this point, the builtins table should definitely have DeserializeLazy
   // set at the position of the target builtin. Also, we should never lazily
   // deserialize DeserializeLazy.
 
-  DCHECK_NE(Builtins::kDeserializeLazy, builtin_name);
+  DCHECK_NE(Builtins::kDeserializeLazy, builtin_id);
   DCHECK_EQ(Builtins::kDeserializeLazy,
-            isolate->builtins()->builtin(builtin_name)->builtin_index());
+            isolate->builtins()->builtin(builtin_id)->builtin_index());
 
   // The DeserializeLazy builtin tail-calls the deserialized builtin. This only
   // works with JS-linkage.
   DCHECK(Builtins::IsLazy(builtin_id));
   DCHECK_EQ(Builtins::TFJ, Builtins::KindOf(builtin_id));
-#endif  // DEBUG
 
   if (FLAG_trace_lazy_deserialization) {
     PrintF("Lazy-deserializing %s\n", Builtins::name(builtin_id));
@@ -537,7 +533,7 @@ RUNTIME_FUNCTION(Runtime_DeserializeLazy) {
 
   Code* code = Snapshot::DeserializeBuiltin(isolate, builtin_id);
   DCHECK_EQ(builtin_id, code->builtin_index());
-  DCHECK_EQ(code, isolate->builtins()->builtin(builtin_name));
+  DCHECK_EQ(code, isolate->builtins()->builtin(builtin_id));
   shared->set_code(code);
   function->set_code(code);
 

src/snapshot/builtin-deserializer.cc

@@ -56,7 +56,7 @@ void BuiltinDeserializer::DeserializeEagerBuiltins() {
       // Do nothing. These builtins have been replaced by DeserializeLazy in
       // InitializeBuiltinsTable.
       DCHECK_EQ(builtins->builtin(Builtins::kDeserializeLazy),
-                builtins->builtin(static_cast<Builtins::Name>(i)));
+                builtins->builtin(i));
     } else {
       builtins->set_builtin(i, DeserializeBuiltin(i));
     }
@@ -64,7 +64,7 @@ void BuiltinDeserializer::DeserializeEagerBuiltins() {
 
 #ifdef DEBUG
   for (int i = 0; i < Builtins::builtin_count; i++) {
-    Object* o = builtins->builtin(static_cast<Builtins::Name>(i));
+    Object* o = builtins->builtin(i);
     DCHECK(o->IsCode() && Code::cast(o)->is_builtin());
   }
 #endif
@@ -197,10 +197,7 @@ void BuiltinDeserializer::ReserveAndInitializeBuiltinsTableForBuiltin(
   DCHECK(Builtins::IsBuiltinId(builtin_id));
   DCHECK_NE(Builtins::kDeserializeLazy, builtin_id);
   DCHECK_EQ(Builtins::kDeserializeLazy,
-            isolate()
-                ->builtins()
-                ->builtin(static_cast<Builtins::Name>(builtin_id))
-                ->builtin_index());
+            isolate()->builtins()->builtin(builtin_id)->builtin_index());
 
   const uint32_t builtin_size = ExtractBuiltinSize(builtin_id);
   DCHECK_LE(builtin_size, MemoryAllocator::PageAreaSize(CODE_SPACE));
@@ -221,8 +218,7 @@ void BuiltinDeserializer::ReserveAndInitializeBuiltinsTableForBuiltin(
 Address BuiltinDeserializer::Allocate(int space_index, int size) {
   DCHECK_EQ(CODE_SPACE, space_index);
   DCHECK_EQ(ExtractBuiltinSize(current_builtin_id_), size);
-  Object* obj = isolate()->builtins()->builtin(
-      static_cast<Builtins::Name>(current_builtin_id_));
+  Object* obj = isolate()->builtins()->builtin(current_builtin_id_);
   DCHECK(Internals::HasHeapObjectTag(obj));
   return HeapObject::cast(obj)->address();
 }

src/snapshot/builtin-serializer.cc

@@ -20,9 +20,8 @@ BuiltinSerializer::~BuiltinSerializer() {
 
 void BuiltinSerializer::SerializeBuiltins() {
   for (int i = 0; i < Builtins::builtin_count; i++) {
-    Code* code = isolate()->builtins()->builtin(static_cast<Builtins::Name>(i));
     builtin_offsets_[i] = sink()->Position();
-    SerializeBuiltin(code);
+    SerializeBuiltin(isolate()->builtins()->builtin(i));
   }
   Pad();  // Pad with kNop since GetInt() might read too far.
 

src/snapshot/deserializer.cc

@@ -816,9 +816,7 @@ Object** Deserializer::ReadDataCase(Isolate* isolate, Object** current,
     } else {
       DCHECK(where == kBuiltin);
       int builtin_id = MaybeReplaceWithDeserializeLazy(source_.GetInt());
-      DCHECK(Builtins::IsBuiltinId(builtin_id));
-      Builtins::Name name = static_cast<Builtins::Name>(builtin_id);
-      new_object = isolate->builtins()->builtin(name);
+      new_object = isolate->builtins()->builtin(builtin_id);
       emit_write_barrier = false;
     }
     if (within == kInnerPointer) {

src/snapshot/snapshot-common.cc

@@ -101,8 +101,7 @@ Code* Snapshot::DeserializeBuiltin(Isolate* isolate, int builtin_id) {
   DisallowHeapAllocation no_gc;
 
   Code* code = builtin_deserializer.DeserializeBuiltin(builtin_id);
-  DCHECK_EQ(code, isolate->builtins()->builtin(
-                      static_cast<Builtins::Name>(builtin_id)));
+  DCHECK_EQ(code, isolate->builtins()->builtin(builtin_id));
 
   if (FLAG_profile_deserialization) {
     double ms = timer.Elapsed().InMillisecondsF();

test/cctest/test-debug.cc

@@ -6530,7 +6530,7 @@ TEST(BuiltinsExceptionPrediction) {
   i::Builtins* builtins = CcTest::i_isolate()->builtins();
   bool fail = false;
   for (int i = 0; i < i::Builtins::builtin_count; i++) {
-    Code* builtin = builtins->builtin(static_cast<i::Builtins::Name>(i));
+    Code* builtin = builtins->builtin(i);
 
     if (builtin->kind() != Code::BUILTIN) continue;
     if (whitelist.find(i) != whitelist.end()) continue;