Bug 1635635 - Use ScriptThingsVector for FunctionCreationData. r=mgaudet

Use ScriptThingVariant to represent closed-over-bindings and inner-functions
for lazy functions. Inline the body of BaseScript::CreateLazy as well so we
can use EmitScriptThingsVector to init the gcthings. We take care to keep
setting the enclosingScript link.

Differential Revision: https://phabricator.services.mozilla.com/D74036

js/src/frontend/BytecodeSection.cpp

@@ -65,6 +65,17 @@ bool js::frontend::EmitScriptThingsVector(JSContext* cx,
     uint32_t i;
     mozilla::Span<JS::GCCellPtr>& output;
 
+    bool operator()(const ClosedOverBinding& data) {
+      JSAtom* atom = data;
+      output[i] = JS::GCCellPtr(atom);
+      return true;
+    }
+
+    bool operator()(const NullScriptThing& data) {
+      output[i] = JS::GCCellPtr(nullptr);
+      return true;
+    }
+
     bool operator()(const BigIntIndex& index) {
       BigIntCreationData& data = compilationInfo.bigIntData[index];
       BigInt* bi = data.createBigInt(cx);

js/src/frontend/Parser.cpp

@@ -1778,9 +1778,32 @@ bool PerHandlerParser<SyntaxParseHandler>::finishFunction(
 
   FunctionCreationData& fcd = funbox->functionCreationData().get();
 
-  // Initialize the data used for lazy scripts.
-  fcd.innerFunctionIndexes.emplace(std::move(pc_->innerFunctionIndexesForLazy));
-  fcd.closedOverBindings.emplace(std::move(pc_->closedOverBindingsForLazy()));
+  fcd.gcThings.emplace(cx_);
+  ScriptThingsVector& gcthings = fcd.gcThings.ref();
+
+  if (!gcthings.reserve(ngcthings.value())) {
+    return false;
+  }
+
+  // Copy inner-function and closed-over-binding info for the stencil. The order
+  // is important here. We emit functions first, followed by the bindings info.
+  // The bindings list uses nullptr as delimiter to separates the bindings per
+  // scope.
+  //
+  // See: FullParseHandler::nextLazyInnerFunction(),
+  //      FullParseHandler::nextLazyClosedOverBinding()
+  for (const FunctionIndex& index : pc_->innerFunctionIndexesForLazy) {
+    gcthings.infallibleAppend(AsVariant(index));
+  }
+  for (const ClosedOverBinding& binding : pc_->closedOverBindingsForLazy()) {
+    if (binding) {
+      gcthings.infallibleAppend(AsVariant(binding));
+    } else {
+      gcthings.infallibleAppend(AsVariant(NullScriptThing()));
+    }
+  }
+
+  MOZ_ASSERT(gcthings.length() == ngcthings.value());
 
   return true;
 }
@@ -1837,18 +1860,45 @@ bool FunctionCreationData::createLazyScript(
   immutableFlags.setFlag(ImmutableFlags::IsLikelyConstructorWrapper,
                          funbox->isLikelyConstructorWrapper());
 
-  BaseScript* lazy = BaseScript::CreateLazy(
-      cx, compilationInfo, function, sourceObject, *closedOverBindings,
-      *innerFunctionIndexes, funbox->extent, immutableFlags);
+  Rooted<BaseScript*> lazy(
+      cx,
+      BaseScript::CreateRawLazy(cx, gcThings->length(), function, sourceObject,
+                                funbox->extent, immutableFlags));
   if (!lazy) {
     return false;
   }
 
+  if (!EmitScriptThingsVector(cx, compilationInfo, *gcThings,
+                              lazy->gcthingsForInit())) {
+    return false;
+  }
+
+  // Connect inner functions to this lazy script now.
+  for (auto inner : lazy->gcthings()) {
+    if (!inner.is<JSObject>()) {
+      continue;
+    }
+    inner.as<JSObject>().as<JSFunction>().setEnclosingLazyScript(lazy);
+  }
+
   function->initScript(lazy);
 
   return true;
 }
 
+void FunctionCreationData::trace(JSTracer* trc) {
+  if (gcThings) {
+    for (ScriptThingVariant& thing : *gcThings) {
+      if (thing.is<ClosedOverBinding>()) {
+        JSAtom* atom = thing.as<ClosedOverBinding>();
+        TraceRoot(trc, &atom, "closed-over-binding");
+        MOZ_ASSERT(atom == thing.as<ClosedOverBinding>(),
+                   "Atoms should be unmovable");
+      }
+    }
+  }
+}
+
 static YieldHandling GetYieldHandling(GeneratorKind generatorKind) {
   if (generatorKind == GeneratorKind::NotGenerator) {
     return YieldIsName;

js/src/frontend/Stencil.h

@@ -326,11 +326,17 @@ class ScopeCreationData {
 
 class EmptyGlobalScopeType {};
 
+// The lazy closed-over-binding info is represented by these types that will
+// convert to a GCCellPtr(nullptr), GCCellPtr(JSAtom*).
+class NullScriptThing {};
+using ClosedOverBinding = JSAtom*;
+
 // These types all end up being baked into GC things as part of stencil
 // instantiation.
 using ScriptThingVariant =
-    mozilla::Variant<BigIntIndex, ObjLiteralCreationData, RegExpIndex,
-                     ScopeIndex, FunctionIndex, EmptyGlobalScopeType>;
+    mozilla::Variant<ClosedOverBinding, NullScriptThing, BigIntIndex,
+                     ObjLiteralCreationData, RegExpIndex, ScopeIndex,
+                     FunctionIndex, EmptyGlobalScopeType>;
 
 // A vector of things destined to be converted to GC things.
 using ScriptThingsVector = Vector<ScriptThingVariant>;
@@ -346,22 +352,17 @@ struct FunctionCreationData {
   FunctionCreationData(const FunctionCreationData&) = delete;
   FunctionCreationData(FunctionCreationData&& data) = default;
 
-  // Data used to instantiate the lazy script before script emission.
-  // -------
-  mozilla::Maybe<frontend::AtomVector> closedOverBindings = {};
-  // This is traced by the functionbox
-  mozilla::Maybe<Vector<FunctionIndex>> innerFunctionIndexes = {};
-  // -------
+  // Lazy functions have a list of GC-things that eventually becomes the
+  // PrivateScriptData structure.
+  mozilla::Maybe<ScriptThingsVector> gcThings = {};
 
   bool createLazyScript(JSContext* cx, CompilationInfo& compilationInfo,
                         HandleFunction function, FunctionBox* funbox,
                         HandleScriptSourceObject sourceObject);
 
-  bool hasLazyScriptData() const {
-    return closedOverBindings && innerFunctionIndexes;
-  }
+  bool hasLazyScriptData() const { return gcThings.isSome(); }
 
-  void trace(JSTracer* trc) {}
+  void trace(JSTracer* trc);
 };
 
 // Data used to instantiate the non-lazy script.

js/src/vm/JSScript.cpp

@@ -5329,48 +5329,6 @@ BaseScript* BaseScript::CreateRawLazy(JSContext* cx, uint32_t ngcthings,
   return lazy;
 }
 
-/* static */
-BaseScript* BaseScript::CreateLazy(
-    JSContext* cx, const frontend::CompilationInfo& compilationInfo,
-    HandleFunction fun, HandleScriptSourceObject sourceObject,
-    const frontend::AtomVector& closedOverBindings,
-    const Vector<frontend::FunctionIndex>& innerFunctionIndexes,
-    const SourceExtent& extent, uint32_t immutableFlags) {
-  uint32_t ngcthings =
-      innerFunctionIndexes.length() + closedOverBindings.length();
-
-  BaseScript* lazy = BaseScript::CreateRawLazy(cx, ngcthings, fun, sourceObject,
-                                               extent, immutableFlags);
-  if (!lazy) {
-    return nullptr;
-  }
-
-  // Fill in gcthing data with inner functions followed by binding data.
-  mozilla::Span<JS::GCCellPtr> gcThings =
-      lazy->data_ ? lazy->data_->gcthings() : mozilla::Span<JS::GCCellPtr>();
-  auto iter = gcThings.begin();
-
-  for (const frontend::FunctionIndex& index : innerFunctionIndexes) {
-    // Assumes that the associated FunctionCreationData was already published.
-    JSFunction* fun = compilationInfo.funcData[index].as<JSFunction*>();
-    *iter++ = JS::GCCellPtr(fun);
-
-    fun->setEnclosingLazyScript(lazy);
-  }
-
-  for (JSAtom* binding : closedOverBindings) {
-    if (binding) {
-      *iter++ = JS::GCCellPtr(binding);
-    } else {
-      iter++;
-    }
-  }
-
-  MOZ_ASSERT(iter == gcThings.end());
-
-  return lazy;
-}
-
 void JSScript::updateJitCodeRaw(JSRuntime* rt) {
   MOZ_ASSERT(rt);
   uint8_t* jitCodeSkipArgCheck;

js/src/vm/JSScript.h

@@ -1797,15 +1797,6 @@ class BaseScript : public gc::TenuredCell {
                                    const SourceExtent& extent,
                                    uint32_t immutableFlags);
 
-  // Create a lazy BaseScript and initialize gc-things with provided
-  // closedOverBindings and innerFunctions.
-  static BaseScript* CreateLazy(
-      JSContext* cx, const frontend::CompilationInfo& compilationInfo,
-      HandleFunction fun, HandleScriptSourceObject sourceObject,
-      const frontend::AtomVector& closedOverBindings,
-      const Vector<frontend::FunctionIndex>& innerFunctionIndexes,
-      const SourceExtent& extent, uint32_t immutableFlags);
-
   uint8_t* jitCodeRaw() const { return headerAndJitCodeRaw_.ptr(); }
   bool isUsingInterpreterTrampoline(JSRuntime* rt) const;
 
@@ -2071,6 +2062,12 @@ class BaseScript : public gc::TenuredCell {
     return data_ ? data_->gcthings() : mozilla::Span<JS::GCCellPtr>();
   }
 
+  // NOTE: This is only used to initialize a fresh script.
+  mozilla::Span<JS::GCCellPtr> gcthingsForInit() {
+    MOZ_ASSERT(!hasBytecode());
+    return data_ ? data_->gcthings() : mozilla::Span<JS::GCCellPtr>();
+  }
+
   void setFieldInitializers(FieldInitializers fieldInitializers) {
     MOZ_ASSERT(data_);
     data_->setFieldInitializers(fieldInitializers);