Refactor of ScopeInfo::ContextSlotIndex

Refactoring ScopeInfo::ContextSlotIndex so it accepts a pointer to
LookupResult instead of references to the individual arguments.

Change-Id: I52bc7800f14e790bd4788c213ab0eff2354ab20e
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2900837
Commit-Queue: Luis Fernando Pardo Sixtos <lpardosixtos@microsoft.com>
Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Cr-Commit-Position: refs/heads/master@{#74683}

src/ast/scopes.cc

@@ -241,15 +241,11 @@ Scope::Scope(Zone* zone, ScopeType scope_type,
   if (scope_type == BLOCK_SCOPE) {
     // Set is_block_scope_for_object_literal_ based on the existince of the home
     // object variable (we don't store it explicitly).
-    VariableMode mode;
-    InitializationFlag init_flag;
-    MaybeAssignedFlag maybe_assigned_flag;
-    IsStaticFlag is_static_flag;
-
+    VariableLookupResult lookup_result;
     DCHECK_NOT_NULL(ast_value_factory);
     int home_object_index = ScopeInfo::ContextSlotIndex(
         *scope_info, *(ast_value_factory->dot_home_object_string()->string()),
-        &mode, &init_flag, &maybe_assigned_flag, &is_static_flag);
+        &lookup_result);
     DCHECK_IMPLIES(home_object_index >= 0,
                    scope_type == CLASS_SCOPE || scope_type == BLOCK_SCOPE);
     if (home_object_index >= 0) {
@@ -903,23 +899,20 @@ Variable* Scope::LookupInScopeInfo(const AstRawString* name, Scope* cache) {
 
   VariableLocation location;
   int index;
-  VariableMode mode;
-  InitializationFlag init_flag;
-  MaybeAssignedFlag maybe_assigned_flag;
-  IsStaticFlag is_static_flag;
+  VariableLookupResult lookup_result;
 
   {
     location = VariableLocation::CONTEXT;
     index =
-        ScopeInfo::ContextSlotIndex(scope_info, name_handle, &mode, &init_flag,
-                                    &maybe_assigned_flag, &is_static_flag);
+        ScopeInfo::ContextSlotIndex(scope_info, name_handle, &lookup_result);
     found = index >= 0;
   }
 
   if (!found && is_module_scope()) {
     location = VariableLocation::MODULE;
-    index = scope_info.ModuleIndex(name_handle, &mode, &init_flag,
-                                   &maybe_assigned_flag);
+    index = scope_info.ModuleIndex(name_handle, &lookup_result.mode,
+                                   &lookup_result.init_flag,
+                                   &lookup_result.maybe_assigned_flag);
     found = index != 0;
   }
 
@@ -938,7 +931,8 @@ Variable* Scope::LookupInScopeInfo(const AstRawString* name, Scope* cache) {
 
   bool was_added;
   Variable* var = cache->variables_.Declare(
-      zone(), this, name, mode, NORMAL_VARIABLE, init_flag, maybe_assigned_flag,
+      zone(), this, name, lookup_result.mode, NORMAL_VARIABLE,
+      lookup_result.init_flag, lookup_result.maybe_assigned_flag,
       IsStaticFlag::kNotStatic, &was_added);
   DCHECK(was_added);
   var->AllocateTo(location, index);
@@ -2753,25 +2747,22 @@ Variable* ClassScope::LookupPrivateNameInScopeInfo(const AstRawString* name) {
   DisallowGarbageCollection no_gc;
 
   String name_handle = *name->string();
-  VariableMode mode;
-  InitializationFlag init_flag;
-  MaybeAssignedFlag maybe_assigned_flag;
-  IsStaticFlag is_static_flag;
+  VariableLookupResult lookup_result;
   int index =
-      ScopeInfo::ContextSlotIndex(*scope_info_, name_handle, &mode, &init_flag,
-                                  &maybe_assigned_flag, &is_static_flag);
+      ScopeInfo::ContextSlotIndex(*scope_info_, name_handle, &lookup_result);
   if (index < 0) {
     return nullptr;
   }
 
-  DCHECK(IsConstVariableMode(mode));
-  DCHECK_EQ(init_flag, InitializationFlag::kNeedsInitialization);
-  DCHECK_EQ(maybe_assigned_flag, MaybeAssignedFlag::kNotAssigned);
+  DCHECK(IsConstVariableMode(lookup_result.mode));
+  DCHECK_EQ(lookup_result.init_flag, InitializationFlag::kNeedsInitialization);
+  DCHECK_EQ(lookup_result.maybe_assigned_flag, MaybeAssignedFlag::kNotAssigned);
 
   // Add the found private name to the map to speed up subsequent
   // lookups for the same name.
   bool was_added;
-  Variable* var = DeclarePrivateName(name, mode, is_static_flag, &was_added);
+  Variable* var = DeclarePrivateName(name, lookup_result.mode,
+                                     lookup_result.is_static_flag, &was_added);
   DCHECK(was_added);
   var->AllocateTo(VariableLocation::CONTEXT, index);
   return var;

src/debug/debug-frames.cc

@@ -93,13 +93,9 @@ bool FrameInspector::IsJavaScript() { return frame_->is_java_script(); }
 
 bool FrameInspector::ParameterIsShadowedByContextLocal(
     Handle<ScopeInfo> info, Handle<String> parameter_name) {
-  VariableMode mode;
-  InitializationFlag init_flag;
-  MaybeAssignedFlag maybe_assigned_flag;
-  IsStaticFlag is_static_flag;
-  return ScopeInfo::ContextSlotIndex(*info, *parameter_name, &mode, &init_flag,
-                                     &maybe_assigned_flag,
-                                     &is_static_flag) != -1;
+  VariableLookupResult lookup_result;
+  return ScopeInfo::ContextSlotIndex(*info, *parameter_name, &lookup_result) !=
+         -1;
 }
 
 RedirectActiveFunctions::RedirectActiveFunctions(SharedFunctionInfo shared,

src/debug/debug-scopes.cc

@@ -1057,14 +1057,9 @@ bool ScopeIterator::SetContextExtensionValue(Handle<String> variable_name,
 
 bool ScopeIterator::SetContextVariableValue(Handle<String> variable_name,
                                             Handle<Object> new_value) {
-  DisallowGarbageCollection no_gc;
-  VariableMode mode;
-  InitializationFlag flag;
-  MaybeAssignedFlag maybe_assigned_flag;
-  IsStaticFlag is_static_flag;
-  int slot_index =
-      ScopeInfo::ContextSlotIndex(context_->scope_info(), *variable_name, &mode,
-                                  &flag, &maybe_assigned_flag, &is_static_flag);
+  VariableLookupResult lookup_result;
+  int slot_index = ScopeInfo::ContextSlotIndex(context_->scope_info(),
+                                               *variable_name, &lookup_result);
   if (slot_index < 0) return false;
 
   context_->set(slot_index, *new_value);
@@ -1097,7 +1092,7 @@ bool ScopeIterator::SetScriptVariableValue(Handle<String> variable_name,
   Handle<ScriptContextTable> script_contexts(
       context_->global_object().native_context().script_context_table(),
       isolate_);
-  ScriptContextTable::LookupResult lookup_result;
+  VariableLookupResult lookup_result;
   if (ScriptContextTable::Lookup(isolate_, *script_contexts, *variable_name,
                                  &lookup_result)) {
     Handle<Context> script_context = ScriptContextTable::GetContext(

src/debug/debug-stack-trace-iterator.cc

@@ -100,13 +100,10 @@ v8::MaybeLocal<v8::Value> DebugStackTraceIterator::GetReceiver() const {
       return v8::MaybeLocal<v8::Value>();
     }
     DisallowGarbageCollection no_gc;
-    VariableMode mode;
-    InitializationFlag flag;
-    MaybeAssignedFlag maybe_assigned_flag;
-    IsStaticFlag is_static_flag;
+    VariableLookupResult lookup_result;
     int slot_index = ScopeInfo::ContextSlotIndex(
         context->scope_info(), ReadOnlyRoots(isolate_->heap()).this_string(),
-        &mode, &flag, &maybe_assigned_flag, &is_static_flag);
+        &lookup_result);
     if (slot_index < 0) return v8::MaybeLocal<v8::Value>();
     Handle<Object> value = handle(context->get(slot_index), isolate_);
     if (value->IsTheHole(isolate_)) return v8::MaybeLocal<v8::Value>();

src/execution/execution.cc

@@ -185,7 +185,7 @@ MaybeHandle<Context> NewScriptContext(Isolate* isolate,
   for (int var = 0; var < scope_info->ContextLocalCount(); var++) {
     Handle<String> name(scope_info->ContextLocalName(var), isolate);
     VariableMode mode = scope_info->ContextLocalMode(var);
-    ScriptContextTable::LookupResult lookup;
+    VariableLookupResult lookup;
     if (ScriptContextTable::Lookup(isolate, *script_context, *name, &lookup)) {
       if (IsLexicalVariableMode(mode) || IsLexicalVariableMode(lookup.mode)) {
         Handle<Context> context = ScriptContextTable::GetContext(

src/ic/ic.cc

@@ -501,7 +501,7 @@ MaybeHandle<Object> LoadGlobalIC::Load(Handle<Name> name,
     Handle<ScriptContextTable> script_contexts(
         global->native_context().script_context_table(), isolate());
 
-    ScriptContextTable::LookupResult lookup_result;
+    VariableLookupResult lookup_result;
     if (ScriptContextTable::Lookup(isolate(), *script_contexts, *str_name,
                                    &lookup_result)) {
       Handle<Context> script_context = ScriptContextTable::GetContext(
@@ -1562,7 +1562,7 @@ MaybeHandle<Object> StoreGlobalIC::Store(Handle<Name> name,
   Handle<ScriptContextTable> script_contexts(
       global->native_context().script_context_table(), isolate());
 
-  ScriptContextTable::LookupResult lookup_result;
+  VariableLookupResult lookup_result;
   if (ScriptContextTable::Lookup(isolate(), *script_contexts, *str_name,
                                  &lookup_result)) {
     Handle<Context> script_context = ScriptContextTable::GetContext(
@@ -2627,7 +2627,7 @@ RUNTIME_FUNCTION(Runtime_StoreGlobalIC_Slow) {
   Handle<ScriptContextTable> script_contexts(
       native_context->script_context_table(), isolate);
 
-  ScriptContextTable::LookupResult lookup_result;
+  VariableLookupResult lookup_result;
   if (ScriptContextTable::Lookup(isolate, *script_contexts, *name,
                                  &lookup_result)) {
     Handle<Context> script_context = ScriptContextTable::GetContext(

src/objects/contexts.cc

@@ -48,17 +48,14 @@ void Context::Initialize(Isolate* isolate) {
 }
 
 bool ScriptContextTable::Lookup(Isolate* isolate, ScriptContextTable table,
-                                String name, LookupResult* result) {
+                                String name, VariableLookupResult* result) {
   DisallowGarbageCollection no_gc;
   // Static variables cannot be in script contexts.
-  IsStaticFlag is_static_flag;
   for (int i = 0; i < table.synchronized_used(); i++) {
     Context context = table.get_context(i);
     DCHECK(context.IsScriptContext());
-    result->is_repl_mode = context.scope_info().IsReplModeScope();
-    int slot_index = ScopeInfo::ContextSlotIndex(
-        context.scope_info(), name, &result->mode, &result->init_flag,
-        &result->maybe_assigned_flag, &is_static_flag);
+    int slot_index =
+        ScopeInfo::ContextSlotIndex(context.scope_info(), name, result);
 
     if (slot_index >= 0) {
       result->context_index = i;
@@ -224,7 +221,7 @@ Handle<Object> Context::Lookup(Handle<Context> context, Handle<String> name,
         // Try other script contexts.
         ScriptContextTable script_contexts =
             context->global_object().native_context().script_context_table();
-        ScriptContextTable::LookupResult r;
+        VariableLookupResult r;
         if (ScriptContextTable::Lookup(isolate, script_contexts, *name, &r)) {
           Context context = script_contexts.get_context(r.context_index);
           if (FLAG_trace_contexts) {
@@ -292,13 +289,9 @@ Handle<Object> Context::Lookup(Handle<Context> context, Handle<String> name,
       // Use serialized scope information of functions and blocks to search
       // for the context index.
       ScopeInfo scope_info = context->scope_info();
-      VariableMode mode;
-      InitializationFlag flag;
-      MaybeAssignedFlag maybe_assigned_flag;
-      IsStaticFlag is_static_flag;
+      VariableLookupResult lookup_result;
       int slot_index =
-          ScopeInfo::ContextSlotIndex(scope_info, *name, &mode, &flag,
-                                      &maybe_assigned_flag, &is_static_flag);
+          ScopeInfo::ContextSlotIndex(scope_info, *name, &lookup_result);
       DCHECK(slot_index < 0 || slot_index >= MIN_CONTEXT_SLOTS);
       if (slot_index >= 0) {
         // Re-direct lookup to the ScriptContextTable in case we find a hole in
@@ -314,12 +307,12 @@ Handle<Object> Context::Lookup(Handle<Context> context, Handle<String> name,
 
         if (FLAG_trace_contexts) {
           PrintF("=> found local in context slot %d (mode = %hhu)\n",
-                 slot_index, static_cast<uint8_t>(mode));
+                 slot_index, static_cast<uint8_t>(lookup_result.mode));
         }
         *index = slot_index;
-        *variable_mode = mode;
-        *init_flag = flag;
-        *attributes = GetAttributesForMode(mode);
+        *variable_mode = lookup_result.mode;
+        *init_flag = lookup_result.init_flag;
+        *attributes = GetAttributesForMode(lookup_result.mode);
         return context;
       }
 

src/objects/contexts.h

@@ -352,21 +352,12 @@ enum ContextLookupFlags {
 //
 // The table is a fixed array, its first slot is the current used count and
 // the subsequent slots 1..used contain ScriptContexts.
+
+struct VariableLookupResult;
 class ScriptContextTable : public FixedArray {
  public:
   DECL_CAST(ScriptContextTable)
 
-  struct LookupResult {
-    int context_index;
-    int slot_index;
-    // repl_mode flag is needed to disable inlining of 'const' variables in REPL
-    // mode.
-    bool is_repl_mode;
-    VariableMode mode;
-    InitializationFlag init_flag;
-    MaybeAssignedFlag maybe_assigned_flag;
-  };
-
   inline int synchronized_used() const;
   inline void synchronized_set_used(int used);
 
@@ -382,7 +373,7 @@ class ScriptContextTable : public FixedArray {
   V8_WARN_UNUSED_RESULT
   V8_EXPORT_PRIVATE static bool Lookup(Isolate* isolate,
                                        ScriptContextTable table, String name,
-                                       LookupResult* result);
+                                       VariableLookupResult* result);
 
   V8_WARN_UNUSED_RESULT
   V8_EXPORT_PRIVATE static Handle<ScriptContextTable> Extend(

src/objects/scope-info.cc

@@ -930,15 +930,10 @@ int ScopeInfo::ModuleIndex(String name, VariableMode* mode,
 
 // static
 int ScopeInfo::ContextSlotIndex(ScopeInfo scope_info, String name,
-                                VariableMode* mode,
-                                InitializationFlag* init_flag,
-                                MaybeAssignedFlag* maybe_assigned_flag,
-                                IsStaticFlag* is_static_flag) {
+                                VariableLookupResult* lookup_result) {
   DisallowGarbageCollection no_gc;
   DCHECK(name.IsInternalizedString());
-  DCHECK_NOT_NULL(mode);
-  DCHECK_NOT_NULL(init_flag);
-  DCHECK_NOT_NULL(maybe_assigned_flag);
+  DCHECK_NOT_NULL(lookup_result);
 
   if (scope_info.IsEmpty()) return -1;
 
@@ -947,10 +942,12 @@ int ScopeInfo::ContextSlotIndex(ScopeInfo scope_info, String name,
     if (name != scope_info.context_local_names(var)) {
       continue;
     }
-    *mode = scope_info.ContextLocalMode(var);
-    *is_static_flag = scope_info.ContextLocalIsStaticFlag(var);
-    *init_flag = scope_info.ContextLocalInitFlag(var);
-    *maybe_assigned_flag = scope_info.ContextLocalMaybeAssignedFlag(var);
+    lookup_result->mode = scope_info.ContextLocalMode(var);
+    lookup_result->is_static_flag = scope_info.ContextLocalIsStaticFlag(var);
+    lookup_result->init_flag = scope_info.ContextLocalInitFlag(var);
+    lookup_result->maybe_assigned_flag =
+        scope_info.ContextLocalMaybeAssignedFlag(var);
+    lookup_result->is_repl_mode = scope_info.IsReplModeScope();
     int result = scope_info.ContextHeaderLength() + var;
 
     DCHECK_LT(result, scope_info.ContextLength());

src/objects/scope-info.h

@@ -31,6 +31,18 @@ class Scope;
 class StringSet;
 class Zone;
 
+struct VariableLookupResult {
+  int context_index;
+  int slot_index;
+  // repl_mode flag is needed to disable inlining of 'const' variables in REPL
+  // mode.
+  bool is_repl_mode;
+  IsStaticFlag is_static_flag;
+  VariableMode mode;
+  InitializationFlag init_flag;
+  MaybeAssignedFlag maybe_assigned_flag;
+};
+
 // ScopeInfo represents information about different scopes of a source
 // program  and the allocation of the scope's variables. Scope information
 // is stored in a compressed form in ScopeInfo objects and is used
@@ -154,9 +166,7 @@ class ScopeInfo : public TorqueGeneratedScopeInfo<ScopeInfo, HeapObject> {
   // If the slot is present and mode != nullptr, sets *mode to the corresponding
   // mode for that variable.
   static int ContextSlotIndex(ScopeInfo scope_info, String name,
-                              VariableMode* mode, InitializationFlag* init_flag,
-                              MaybeAssignedFlag* maybe_assigned_flag,
-                              IsStaticFlag* is_static_flag);
+                              VariableLookupResult* lookup_result);
 
   // Lookup metadata of a MODULE-allocated variable.  Return 0 if there is no
   // module variable with the given name (the index value of a MODULE variable

src/runtime/runtime-scopes.cc

@@ -52,7 +52,7 @@ Object DeclareGlobal(Isolate* isolate, Handle<JSGlobalObject> global,
                      RedeclarationType redeclaration_type) {
   Handle<ScriptContextTable> script_contexts(
       global->native_context().script_context_table(), isolate);
-  ScriptContextTable::LookupResult lookup;
+  VariableLookupResult lookup;
   if (ScriptContextTable::Lookup(isolate, *script_contexts, *name, &lookup) &&
       IsLexicalVariableMode(lookup.mode)) {
     // ES#sec-globaldeclarationinstantiation 6.a:
@@ -869,7 +869,7 @@ RUNTIME_FUNCTION(Runtime_StoreGlobalNoHoleCheckForReplLetOrConst) {
   Handle<ScriptContextTable> script_contexts(
       native_context->script_context_table(), isolate);
 
-  ScriptContextTable::LookupResult lookup_result;
+  VariableLookupResult lookup_result;
   bool found = ScriptContextTable::Lookup(isolate, *script_contexts, *name,
                                           &lookup_result);
   CHECK(found);

test/cctest/test-inobject-slack-tracking.cc

@@ -45,7 +45,7 @@ Handle<T> GetLexical(const char* name) {
   Handle<ScriptContextTable> script_contexts(
       isolate->native_context()->script_context_table(), isolate);
 
-  ScriptContextTable::LookupResult lookup_result;
+  VariableLookupResult lookup_result;
   if (ScriptContextTable::Lookup(isolate, *script_contexts, *str_name,
                                  &lookup_result)) {
     Handle<Context> script_context = ScriptContextTable::GetContext(