Reland "[runtime] Adds LocalNameIterator"

This is a reland of f605d77822

Adds a GC safe (using handles) and unsafe versions of the iterator.

V8HeapExplorer needs an unsafe one, since it does not allow the
creation of handles.

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: I6288a08b9c342cd3a9cabcb621c40bb44c08c9c4
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3394706
Reviewed-by: Camillo Bruni <cbruni@chromium.org>
Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Commit-Queue: Victor Gomes <victorgomes@chromium.org>
Cr-Commit-Position: refs/heads/main@{#78653}

src/ast/scopes.cc

@@ -470,7 +470,8 @@ 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);
-      String name = scope_info.ContextLocalName(0);
+      DCHECK(scope_info.HasInlinedLocalNames());
+      String name = scope_info.ContextInlinedLocalName(0);
       MaybeAssignedFlag maybe_assigned =
           scope_info.ContextLocalMaybeAssignedFlag(0);
       outer_scope =

src/debug/debug-interface.cc

@@ -111,17 +111,15 @@ 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);
-  int local_count = scope_info->ContextLocalCount();
+  for (auto it : i::ScopeInfo::IterateLocalNames(scope_info)) {
-  for (int j = 0; j < local_count; ++j) {
+    i::Handle<i::String> name(it->name(), isolate);
-    i::VariableMode mode = scope_info->ContextLocalMode(j);
+    i::VariableMode mode = scope_info->ContextLocalMode(it->index());
-    i::IsStaticFlag flag = scope_info->ContextLocalIsStaticFlag(j);
+    i::IsStaticFlag flag = scope_info->ContextLocalIsStaticFlag(it->index());
     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());
@@ -1001,11 +999,9 @@ 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);
-    int local_count = scope_info->ContextLocalCount();
+    for (auto it : i::ScopeInfo::IterateLocalNames(scope_info)) {
-    for (int j = 0; j < local_count; ++j) {
+      if (i::ScopeInfo::VariableIsSynthetic(it->name())) continue;
-      i::String name = scope_info->ContextLocalName(j);
+      names->Append(Utils::ToLocal(handle(it->name(), isolate)));
-      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 (int i = 0; i < scope_info->ContextLocalCount(); ++i) {
+  for (auto it : ScopeInfo::IterateLocalNames(scope_info)) {
-    Handle<String> name(scope_info->ContextLocalName(i), isolate_);
+    Handle<String> name(it->name(), isolate_);
     if (ScopeInfo::VariableIsSynthetic(*name)) continue;
-    int context_index = scope_info->ContextHeaderLength() + i;
+    int context_index = scope_info->ContextHeaderLength() + it->index();
     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 (int var = 0; var < scope_info->ContextLocalCount(); var++) {
+  for (auto it : ScopeInfo::IterateLocalNames(scope_info)) {
-    Handle<String> name(scope_info->ContextLocalName(var), isolate);
+    Handle<String> name(it->name(), isolate);
-    VariableMode mode = scope_info->ContextLocalMode(var);
+    VariableMode mode = scope_info->ContextLocalMode(it->index());
     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 (int i = 0; i < heap_locals_count; i++) {
+  for (auto it : ScopeInfo::IterateLocalNames(&scope_info, no_gc)) {
     accumulator->Add("  var ");
-    accumulator->PrintName(scope_info.ContextLocalName(i));
+    accumulator->PrintName(it->name());
     accumulator->Add(" = ");
     if (!context.is_null()) {
-      int slot_index = Context::MIN_CONTEXT_SLOTS + i;
+      int slot_index = Context::MIN_CONTEXT_SLOTS + it->index();
       if (slot_index < context.length()) {
         accumulator->Add("%o", context.get(slot_index));
       } else {

src/objects/scope-info-inl.h

@@ -36,6 +36,66 @@ bool ScopeInfo::HasInlinedLocalNames() const {
   return ContextLocalCount() < kScopeInfoMaxInlinedLocalNamesSize;
 }
 
+template <typename ScopeInfoPtr>
+class ScopeInfo::LocalNamesIterator {
+ public:
+  class Iterator {
+   public:
+    Iterator(ScopeInfoPtr scope_info, int index)
+        : scope_info_(scope_info), index_(index) {}
+
+    Iterator& operator++() {
+      index_++;
+      return *this;
+    }
+
+    friend bool operator==(const Iterator& a, const Iterator& b) {
+      return *a.scope_info_ == *b.scope_info_ && a.index_ == b.index_;
+    }
+    friend bool operator!=(const Iterator& a, const Iterator& b) {
+      return !(a == b);
+    }
+
+    String name() const {
+      DCHECK_LT(index_, scope_info_->context_local_count());
+      return scope_info_->context_local_names(index_);
+    }
+
+    const Iterator* operator*() const { return this; }
+
+    int index() const { return index_; }
+
+   private:
+    ScopeInfoPtr scope_info_;
+    int index_;
+  };
+
+  explicit LocalNamesIterator(ScopeInfoPtr scope_info)
+      : scope_info_(scope_info) {}
+
+  inline Iterator begin() const { return Iterator(scope_info_, 0); }
+
+  inline Iterator end() const {
+    return Iterator(scope_info_, scope_info_->ContextLocalCount());
+  }
+
+ private:
+  ScopeInfoPtr scope_info_;
+};
+
+// static
+ScopeInfo::LocalNamesIterator<Handle<ScopeInfo>> ScopeInfo::IterateLocalNames(
+    Handle<ScopeInfo> scope_info) {
+  return LocalNamesIterator<Handle<ScopeInfo>>(scope_info);
+}
+
+// static
+ScopeInfo::LocalNamesIterator<ScopeInfo*> ScopeInfo::IterateLocalNames(
+    ScopeInfo* scope_info, const DisallowGarbageCollection& no_gc) {
+  USE(no_gc);
+  return LocalNamesIterator<ScopeInfo*>(scope_info);
+}
+
 }  // namespace internal
 }  // namespace v8
 

src/objects/scope-info.cc

@@ -814,7 +814,8 @@ SourceTextModuleInfo ScopeInfo::ModuleDescriptorInfo() const {
   return SourceTextModuleInfo::cast(module_info());
 }
 
-String ScopeInfo::ContextLocalName(int var) const {
+String ScopeInfo::ContextInlinedLocalName(int var) const {
+  DCHECK(HasInlinedLocalNames());
   return context_local_names(var);
 }
 
@@ -923,7 +924,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 = ContextLocalName(index);
+    String name = ContextInlinedLocalName(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,8 +143,18 @@ class ScopeInfo : public TorqueGeneratedScopeInfo<ScopeInfo, HeapObject> {
   // Return true if the local names are inlined in the scope info object.
   inline bool HasInlinedLocalNames() const;
 
-  // Return the name of the given context local.
+  template <typename ScopeInfoPtr>
-  String ContextLocalName(int var) const;
+  class LocalNamesIterator;
+
+  static inline LocalNamesIterator<Handle<ScopeInfo>> IterateLocalNames(
+      Handle<ScopeInfo> scope_info);
+
+  static inline LocalNamesIterator<ScopeInfo*> IterateLocalNames(
+      ScopeInfo* scope_info, const DisallowGarbageCollection& no_gc);
+
+  // Return the name of a given context local.
+  // It should only be used if inlined local names.
+  String ContextInlinedLocalName(int var) const;
 
   // Return the mode of the given context local.
   VariableMode ContextLocalMode(int var) const;

src/profiler/heap-snapshot-generator.cc

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