Revert "[runtime] Adds LocalNameIterator"

This reverts commit f605d778.

Reason for revert: Segfaults: https://ci.chromium.org/ui/p/v8/builders/ci/V8%20Linux%20-%20gc%20stress/36908/overview

Original change's description:
> [runtime] Adds LocalNameIterator
>
> ScopeInfo will contain either inlined (array) local names or
> a hash table (names => index) containing the local names.
>
> We abstract iteration with LocalNameIterator and remove
> ContextLocalName since accessing a local name by index in
> the hash table would be expensive.
>
> This CL only implements the iterator for the array.
>
> Bug: v8:12315
> Change-Id: I2c62802652fca1cf47815ce8768a3f7487f2c39f
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3386603
> Reviewed-by: Toon Verwaest <verwaest@chromium.org>
> Commit-Queue: Victor Gomes <victorgomes@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#78623}

Bug: v8:12315
Change-Id: Ibabe231f4357a3dd02d24b89847d579b83867a1a
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3386385
Auto-Submit: Leszek Swirski <leszeks@chromium.org>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Owners-Override: Leszek Swirski <leszeks@chromium.org>
Commit-Queue: Leszek Swirski <leszeks@chromium.org>
Cr-Commit-Position: refs/heads/main@{#78625}

src/ast/scopes.cc

@@ -470,8 +470,7 @@ Scope* Scope::DeserializeScopeChain(IsolateT* isolate, Zone* zone,
       DCHECK_EQ(scope_info.ContextLocalCount(), 1);
       DCHECK_EQ(scope_info.ContextLocalMode(0), VariableMode::kVar);
       DCHECK_EQ(scope_info.ContextLocalInitFlag(0), kCreatedInitialized);
-      DCHECK(scope_info.HasInlinedLocalNames());
-      String name = scope_info.ContextInlinedLocalName(0);
+      String name = scope_info.ContextLocalName(0);
       MaybeAssignedFlag maybe_assigned =
           scope_info.ContextLocalMaybeAssignedFlag(0);
       outer_scope =

src/debug/debug-interface.cc

@@ -111,15 +111,17 @@ void CollectPrivateMethodsAndAccessorsFromContext(
     i::IsStaticFlag is_static_flag, std::vector<Local<Value>>* names_out,
     std::vector<Local<Value>>* values_out) {
   i::Handle<i::ScopeInfo> scope_info(context->scope_info(), isolate);
-  for (auto it : *scope_info) {
-    i::Handle<i::String> name(it->name(), isolate);
-    i::VariableMode mode = scope_info->ContextLocalMode(it->index());
-    i::IsStaticFlag flag = scope_info->ContextLocalIsStaticFlag(it->index());
+  int local_count = scope_info->ContextLocalCount();
+  for (int j = 0; j < local_count; ++j) {
+    i::VariableMode mode = scope_info->ContextLocalMode(j);
+    i::IsStaticFlag flag = scope_info->ContextLocalIsStaticFlag(j);
     if (!i::IsPrivateMethodOrAccessorVariableMode(mode) ||
         flag != is_static_flag) {
       continue;
     }
-    int context_index = scope_info->ContextHeaderLength() + it->index();
+
+    i::Handle<i::String> name(scope_info->ContextLocalName(j), isolate);
+    int context_index = scope_info->ContextHeaderLength() + j;
     i::Handle<i::Object> slot_value(context->get(context_index), isolate);
     DCHECK_IMPLIES(mode == i::VariableMode::kPrivateMethod,
                    slot_value->IsJSFunction());
@@ -999,9 +1001,11 @@ void GlobalLexicalScopeNames(v8::Local<v8::Context> v8_context,
         i::ScriptContextTable::GetContext(isolate, table, i);
     DCHECK(script_context->IsScriptContext());
     i::Handle<i::ScopeInfo> scope_info(script_context->scope_info(), isolate);
-    for (auto it : *scope_info) {
-      if (i::ScopeInfo::VariableIsSynthetic(it->name())) continue;
-      names->Append(Utils::ToLocal(handle(it->name(), isolate)));
+    int local_count = scope_info->ContextLocalCount();
+    for (int j = 0; j < local_count; ++j) {
+      i::String name = scope_info->ContextLocalName(j);
+      if (i::ScopeInfo::VariableIsSynthetic(name)) continue;
+      names->Append(Utils::ToLocal(handle(name, isolate)));
     }
   }
 }

src/debug/debug-scopes.cc

@@ -780,10 +780,10 @@ bool ScopeIterator::VisitContextLocals(const Visitor& visitor,
                                        Handle<Context> context,
                                        ScopeType scope_type) const {
   // Fill all context locals to the context extension.
-  for (auto it : *scope_info) {
-    Handle<String> name(it->name(), isolate_);
+  for (int i = 0; i < scope_info->ContextLocalCount(); ++i) {
+    Handle<String> name(scope_info->ContextLocalName(i), isolate_);
     if (ScopeInfo::VariableIsSynthetic(*name)) continue;
-    int context_index = scope_info->ContextHeaderLength() + it->index();
+    int context_index = scope_info->ContextHeaderLength() + i;
     Handle<Object> value(context->get(context_index), isolate_);
     if (visitor(name, value, scope_type)) return true;
   }

src/execution/execution.cc

@@ -207,9 +207,9 @@ MaybeHandle<Context> NewScriptContext(Isolate* isolate,
       native_context->script_context_table(), isolate);
 
   // Find name clashes.
-  for (auto it : *scope_info) {
-    Handle<String> name(it->name(), isolate);
-    VariableMode mode = scope_info->ContextLocalMode(it->index());
+  for (int var = 0; var < scope_info->ContextLocalCount(); var++) {
+    Handle<String> name(scope_info->ContextLocalName(var), isolate);
+    VariableMode mode = scope_info->ContextLocalMode(var);
     VariableLookupResult lookup;
     if (ScriptContextTable::Lookup(isolate, *script_context, *name, &lookup)) {
       if (IsLexicalVariableMode(mode) || IsLexicalVariableMode(lookup.mode)) {

src/execution/frames.cc

@@ -2342,12 +2342,12 @@ void JavaScriptFrame::Print(StringStream* accumulator, PrintMode mode,
   if (heap_locals_count > 0) {
     accumulator->Add("  // heap-allocated locals\n");
   }
-  for (auto it : scope_info) {
+  for (int i = 0; i < heap_locals_count; i++) {
     accumulator->Add("  var ");
-    accumulator->PrintName(it->name());
+    accumulator->PrintName(scope_info.ContextLocalName(i));
     accumulator->Add(" = ");
     if (!context.is_null()) {
-      int slot_index = Context::MIN_CONTEXT_SLOTS + it->index();
+      int slot_index = Context::MIN_CONTEXT_SLOTS + i;
       if (slot_index < context.length()) {
         accumulator->Add("%o", context.get(slot_index));
       } else {

src/objects/scope-info-inl.h

@@ -36,20 +36,6 @@ bool ScopeInfo::HasInlinedLocalNames() const {
   return ContextLocalCount() < kScopeInfoMaxInlinedLocalNamesSize;
 }
 
-String ScopeInfo::LocalNameIterator::name() const {
-  DCHECK_NOT_NULL(scope_info_);
-  DCHECK_LT(index_, scope_info_->context_local_count());
-  return scope_info_->context_local_names(index_);
-}
-
-ScopeInfo::LocalNameIterator ScopeInfo::begin() const {
-  return LocalNameIterator(this, 0);
-}
-
-ScopeInfo::LocalNameIterator ScopeInfo::end() const {
-  return LocalNameIterator(this, ContextLocalCount());
-}
-
 }  // namespace internal
 }  // namespace v8
 

src/objects/scope-info.cc

@@ -814,8 +814,7 @@ SourceTextModuleInfo ScopeInfo::ModuleDescriptorInfo() const {
   return SourceTextModuleInfo::cast(module_info());
 }
 
-String ScopeInfo::ContextInlinedLocalName(int var) const {
-  DCHECK(HasInlinedLocalNames());
+String ScopeInfo::ContextLocalName(int var) const {
   return context_local_names(var);
 }
 
@@ -924,7 +923,7 @@ std::pair<String, int> ScopeInfo::SavedClassVariable() const {
     int index = saved_class_variable_info() - Context::MIN_CONTEXT_SLOTS;
     DCHECK_GE(index, 0);
     DCHECK_LT(index, ContextLocalCount());
-    String name = ContextInlinedLocalName(index);
+    String name = ContextLocalName(index);
     return std::make_pair(name, index);
   } else {
     // The saved class variable info corresponds to the offset in the hash

src/objects/scope-info.h

@@ -143,42 +143,8 @@ class ScopeInfo : public TorqueGeneratedScopeInfo<ScopeInfo, HeapObject> {
   // Return true if the local names are inlined in the scope info object.
   inline bool HasInlinedLocalNames() const;
 
-  class LocalNameIterator {
-   public:
-    LocalNameIterator(const ScopeInfo* scope_info, int index)
-        : scope_info_(scope_info), index_(index) {}
-
-    LocalNameIterator& operator++() {
-      index_++;
-      return *this;
-    }
-
-    friend bool operator==(const LocalNameIterator& a,
-                           const LocalNameIterator& b) {
-      return a.scope_info_ == b.scope_info_ && a.index_ == b.index_;
-    }
-    friend bool operator!=(const LocalNameIterator& a,
-                           const LocalNameIterator& b) {
-      return !(a == b);
-    }
-
-    inline String name() const;
-    const LocalNameIterator* operator*() const { return this; }
-
-    int index() const { return index_; }
-
-   private:
-    const ScopeInfo* scope_info_;
-    int index_;
-  };
-
-  inline LocalNameIterator begin() const;
-
-  inline LocalNameIterator end() const;
-
-  // Return the name of a given context local.
-  // It should only be used if inlined local names.
-  String ContextInlinedLocalName(int var) const;
+  // Return the name of the given context local.
+  String ContextLocalName(int var) const;
 
   // Return the mode of the given context local.
   VariableMode ContextLocalMode(int var) const;

src/profiler/heap-snapshot-generator.cc

@@ -1043,9 +1043,11 @@ void V8HeapExplorer::ExtractContextReferences(HeapEntry* entry,
   if (!context.IsNativeContext() && context.is_declaration_context()) {
     ScopeInfo scope_info = context.scope_info();
     // Add context allocated locals.
-    for (auto it : scope_info) {
-      int idx = scope_info.ContextHeaderLength() + it->index();
-      SetContextReference(entry, it->name(), context.get(idx),
+    int context_locals = scope_info.ContextLocalCount();
+    for (int i = 0; i < context_locals; ++i) {
+      String local_name = scope_info.ContextLocalName(i);
+      int idx = scope_info.ContextHeaderLength() + i;
+      SetContextReference(entry, local_name, context.get(idx),
                           Context::OffsetOfElementAt(idx));
     }
     if (scope_info.HasContextAllocatedFunctionName()) {