[objects] Clean-up SFI::Init/SFI::SetScript

Clean up isolate inference and non-allocating/allocating parts of
SharedFunctionInfo::InitFromFunctionLiteral, so that it can more easily
be refactored for off-thread allocation in the future. Also, make
SharedFunctionInfo::SetScript a member function, to acknowledge that it
is non-allocating.

Bug: chromium:1011762
Change-Id: I26624fceb642dfdf257ec3d96aab31ea90e48870
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1991482
Commit-Queue: Leszek Swirski <leszeks@chromium.org>
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Auto-Submit: Leszek Swirski <leszeks@chromium.org>
Cr-Commit-Position: refs/heads/master@{#65639}

src/heap/factory.cc

@@ -3375,9 +3375,10 @@ Handle<SharedFunctionInfo> Factory::NewSharedFunctionInfoForLiteral(
   FunctionKind kind = literal->kind();
   Handle<SharedFunctionInfo> shared = NewSharedFunctionInfoForBuiltin(
       literal->name(), Builtins::kCompileLazy, kind);
-  SharedFunctionInfo::InitFromFunctionLiteral(shared, literal, is_toplevel);
-  SharedFunctionInfo::SetScript(shared, script, literal->function_literal_id(),
-                                false);
+  SharedFunctionInfo::InitFromFunctionLiteral(isolate(), shared, literal,
+                                              is_toplevel);
+  shared->SetScript(ReadOnlyRoots(isolate()), *script,
+                    literal->function_literal_id(), false);
   return shared;
 }
 

src/init/bootstrapper.cc

@@ -612,8 +612,7 @@ Handle<JSFunction> Genesis::CreateEmptyFunction() {
   empty_function->shared().set_raw_scope_info(
       ReadOnlyRoots(isolate()).empty_function_scope_info());
   empty_function->shared().DontAdaptArguments();
-  SharedFunctionInfo::SetScript(handle(empty_function->shared(), isolate()),
-                                script, 1);
+  empty_function->shared().SetScript(ReadOnlyRoots(isolate()), *script, 1);
 
   return empty_function;
 }

src/objects/objects.cc

@@ -4977,59 +4977,58 @@ void SharedFunctionInfo::ScriptIterator::Reset(Isolate* isolate,
   index_ = 0;
 }
 
-void SharedFunctionInfo::SetScript(Handle<SharedFunctionInfo> shared,
-                                   Handle<HeapObject> script_object,
+void SharedFunctionInfo::SetScript(ReadOnlyRoots roots,
+                                   HeapObject script_object,
                                    int function_literal_id,
                                    bool reset_preparsed_scope_data) {
-  if (shared->script() == *script_object) return;
-  Isolate* isolate = shared->GetIsolate();
+  DisallowHeapAllocation no_gc;
 
-  if (reset_preparsed_scope_data &&
-      shared->HasUncompiledDataWithPreparseData()) {
-    shared->ClearPreparseData();
+  if (script() == script_object) return;
+
+  if (reset_preparsed_scope_data && HasUncompiledDataWithPreparseData()) {
+    ClearPreparseData();
   }
 
   // Add shared function info to new script's list. If a collection occurs,
   // the shared function info may be temporarily in two lists.
   // This is okay because the gc-time processing of these lists can tolerate
   // duplicates.
-  if (script_object->IsScript()) {
-    DCHECK(!shared->script().IsScript());
-    Handle<Script> script = Handle<Script>::cast(script_object);
-    Handle<WeakFixedArray> list =
-        handle(script->shared_function_infos(), isolate);
+  if (script_object.IsScript()) {
+    DCHECK(!script().IsScript());
+    Script script = Script::cast(script_object);
+    WeakFixedArray list = script.shared_function_infos();
 #ifdef DEBUG
-    DCHECK_LT(function_literal_id, list->length());
-    MaybeObject maybe_object = list->Get(function_literal_id);
+    DCHECK_LT(function_literal_id, list.length());
+    MaybeObject maybe_object = list.Get(function_literal_id);
     HeapObject heap_object;
     if (maybe_object->GetHeapObjectIfWeak(&heap_object)) {
-      DCHECK_EQ(heap_object, *shared);
+      DCHECK_EQ(heap_object, *this);
     }
 #endif
-    list->Set(function_literal_id, HeapObjectReference::Weak(*shared));
+    list.Set(function_literal_id, HeapObjectReference::Weak(*this));
   } else {
-    DCHECK(shared->script().IsScript());
+    DCHECK(script().IsScript());
 
     // Remove shared function info from old script's list.
-    Script old_script = Script::cast(shared->script());
+    Script old_script = Script::cast(script());
 
     // Due to liveedit, it might happen that the old_script doesn't know
     // about the SharedFunctionInfo, so we have to guard against that.
-    Handle<WeakFixedArray> infos(old_script.shared_function_infos(), isolate);
-    if (function_literal_id < infos->length()) {
+    WeakFixedArray infos = old_script.shared_function_infos();
+    if (function_literal_id < infos.length()) {
       MaybeObject raw =
           old_script.shared_function_infos().Get(function_literal_id);
       HeapObject heap_object;
-      if (raw->GetHeapObjectIfWeak(&heap_object) && heap_object == *shared) {
+      if (raw->GetHeapObjectIfWeak(&heap_object) && heap_object == *this) {
         old_script.shared_function_infos().Set(
-            function_literal_id, HeapObjectReference::Strong(
-                                     ReadOnlyRoots(isolate).undefined_value()));
+            function_literal_id,
+            HeapObjectReference::Strong(roots.undefined_value()));
       }
     }
   }
 
   // Finally set new script.
-  shared->set_script(*script_object);
+  set_script(script_object);
 }
 
 bool SharedFunctionInfo::HasBreakInfo() const {
@@ -5256,27 +5255,21 @@ void SharedFunctionInfo::DisableOptimization(BailoutReason reason) {
   }
 }
 
+// static
 void SharedFunctionInfo::InitFromFunctionLiteral(
-    Handle<SharedFunctionInfo> shared_info, FunctionLiteral* lit,
-    bool is_toplevel) {
-  Isolate* isolate = shared_info->GetIsolate();
-  bool needs_position_info = true;
+    Isolate* isolate, Handle<SharedFunctionInfo> shared_info,
+    FunctionLiteral* lit, bool is_toplevel) {
+  DCHECK(!shared_info->name_or_scope_info().IsScopeInfo());
 
   // When adding fields here, make sure DeclarationScope::AnalyzePartially is
   // updated accordingly.
   shared_info->set_internal_formal_parameter_count(lit->parameter_count());
   shared_info->SetFunctionTokenPosition(lit->function_token_position(),
                                         lit->start_position());
-  if (shared_info->scope_info().HasPositionInfo()) {
-    shared_info->scope_info().SetPositionInfo(lit->start_position(),
-                                              lit->end_position());
-    needs_position_info = false;
-  }
   shared_info->set_syntax_kind(lit->syntax_kind());
   shared_info->set_allows_lazy_compilation(lit->AllowsLazyCompilation());
   shared_info->set_language_mode(lit->language_mode());
   shared_info->set_function_literal_id(lit->function_literal_id());
-  //  shared_info->set_kind(lit->kind());
   // FunctionKind must have already been set.
   DCHECK(lit->kind() == shared_info->kind());
   shared_info->set_needs_home_object(lit->scope()->NeedsHomeObject());
@@ -5308,32 +5301,31 @@ void SharedFunctionInfo::InitFromFunctionLiteral(
     shared_info->set_is_safe_to_skip_arguments_adaptor(
         lit->SafeToSkipArgumentsAdaptor());
     DCHECK_NULL(lit->produced_preparse_data());
+
     // If we're about to eager compile, we'll have the function literal
     // available, so there's no need to wastefully allocate an uncompiled data.
-    // TODO(leszeks): This should be explicitly passed as a parameter, rather
-    // than relying on a property of the literal.
-    needs_position_info = false;
+    return;
+  }
+
+  shared_info->set_is_safe_to_skip_arguments_adaptor(false);
+  shared_info->UpdateExpectedNofPropertiesFromEstimate(lit);
+
+  Handle<UncompiledData> data;
+
+  ProducedPreparseData* scope_data = lit->produced_preparse_data();
+  if (scope_data != nullptr) {
+    Handle<PreparseData> preparse_data =
+        scope_data->Serialize(shared_info->GetIsolate());
+
+    data = isolate->factory()->NewUncompiledDataWithPreparseData(
+        lit->inferred_name(), lit->start_position(), lit->end_position(),
+        preparse_data);
   } else {
-    shared_info->set_is_safe_to_skip_arguments_adaptor(false);
-    ProducedPreparseData* scope_data = lit->produced_preparse_data();
-    if (scope_data != nullptr) {
-      Handle<PreparseData> preparse_data =
-          scope_data->Serialize(shared_info->GetIsolate());
-      Handle<UncompiledData> data =
-          isolate->factory()->NewUncompiledDataWithPreparseData(
-              lit->inferred_name(), lit->start_position(), lit->end_position(),
-              preparse_data);
-      shared_info->set_uncompiled_data(*data);
-      needs_position_info = false;
-    }
-    shared_info->UpdateExpectedNofPropertiesFromEstimate(lit);
-  }
-  if (needs_position_info) {
-    Handle<UncompiledData> data =
-        isolate->factory()->NewUncompiledDataWithoutPreparseData(
-            lit->inferred_name(), lit->start_position(), lit->end_position());
-    shared_info->set_uncompiled_data(*data);
+    data = isolate->factory()->NewUncompiledDataWithoutPreparseData(
+        lit->inferred_name(), lit->start_position(), lit->end_position());
   }
+
+  shared_info->set_uncompiled_data(*data);
 }
 
 uint16_t SharedFunctionInfo::get_property_estimate_from_literal(

src/objects/shared-function-info.h

@@ -17,6 +17,7 @@
 #include "src/objects/slots.h"
 #include "src/objects/smi.h"
 #include "src/objects/struct.h"
+#include "src/roots/roots.h"
 #include "testing/gtest/include/gtest/gtest_prod.h"
 #include "torque-generated/bit-fields-tq.h"
 #include "torque-generated/field-offsets-tq.h"
@@ -201,9 +202,10 @@ class SharedFunctionInfo : public HeapObject,
 
   // Set up the link between shared function info and the script. The shared
   // function info is added to the list on the script.
-  V8_EXPORT_PRIVATE static void SetScript(
-      Handle<SharedFunctionInfo> shared, Handle<HeapObject> script_object,
-      int function_literal_id, bool reset_preparsed_scope_data = true);
+  V8_EXPORT_PRIVATE void SetScript(ReadOnlyRoots roots,
+                                   HeapObject script_object,
+                                   int function_literal_id,
+                                   bool reset_preparsed_scope_data = true);
 
   // Layout description of the optimized code map.
   static const int kEntriesStart = 0;
@@ -548,7 +550,8 @@ class SharedFunctionInfo : public HeapObject,
   inline bool has_simple_parameters();
 
   // Initialize a SharedFunctionInfo from a parsed function literal.
-  static void InitFromFunctionLiteral(Handle<SharedFunctionInfo> shared_info,
+  static void InitFromFunctionLiteral(Isolate* isolate,
+                                      Handle<SharedFunctionInfo> shared_info,
                                       FunctionLiteral* lit, bool is_toplevel);
 
   // Updates the expected number of properties based on estimate from parser.

test/unittests/test-helpers.cc

@@ -49,7 +49,7 @@ Handle<SharedFunctionInfo> CreateSharedFunctionInfo(
   // Make sure we have an outer scope info, even though it's empty
   shared->set_raw_outer_scope_info_or_feedback_metadata(
       ScopeInfo::Empty(isolate));
-  SharedFunctionInfo::SetScript(shared, script, function_literal_id);
+  shared->SetScript(ReadOnlyRoots(isolate), *script, function_literal_id);
   return scope.CloseAndEscape(shared);
 }