Bug 1595745 - Part 12: Change GeneratorFunction to use ClassSpec. r=mgaudet
☠☠ backed out by ec8cad689121 ☠ ☠
authorAndré Bargull <andre.bargull@gmail.com>
Fri, 15 Nov 2019 15:54:45 +0000
changeset 502206 2ce96c6187c2ad834605f4ea6cfb17688c0ebadd
parent 502205 e089ebe699d2ef3729774765db5c47a58995435f
child 502207 b11677f10f9d42f360852504ebedd4a3c03a1b32
push id114172
push userdluca@mozilla.com
push dateTue, 19 Nov 2019 11:31:10 +0000
treeherdermozilla-inbound@b5c5ba07d3db [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmgaudet
bugs1595745
milestone72.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1595745 - Part 12: Change GeneratorFunction to use ClassSpec. r=mgaudet The "constructor" property of the GeneratorFunction prototype is non-writable, so we need to manually adjust the property attributes in the `FinishClassInitOp`. This change needs to happen first to ensure "constructor" is still stored in the last property, which in turn ensures the property can be modified without triggering a transition into dictionary mode. jsapi.cpp: Remove the JSProto_GeneratorFunction special cases now that we can use `ClassSpec::DontDefineConstructor`. Differential Revision: https://phabricator.services.mozilla.com/D52677
js/public/ProtoKey.h
js/src/jsapi.cpp
js/src/vm/GeneratorObject.cpp
js/src/vm/GeneratorObject.h
--- a/js/public/ProtoKey.h
+++ b/js/public/ProtoKey.h
@@ -106,17 +106,17 @@
   REAL(Reflect, InitViaClassSpec, CLASP(Reflect))                            \
   REAL(WeakSet, InitViaClassSpec, OCLASP(WeakSet))                           \
   REAL(TypedArray, InitViaClassSpec,                                         \
        &js::TypedArrayObject::sharedTypedArrayPrototypeClass)                \
   REAL(Atomics, InitViaClassSpec, OCLASP(Atomics))                           \
   REAL(SavedFrame, InitViaClassSpec, &js::SavedFrame::class_)                \
   REAL(Promise, InitViaClassSpec, OCLASP(Promise))                           \
   REAL(AsyncFunction, InitAsyncFunction, nullptr)                            \
-  REAL(GeneratorFunction, InitGeneratorFunction, nullptr)                    \
+  REAL(GeneratorFunction, InitViaClassSpec, CLASP(GeneratorFunction))        \
   REAL(AsyncGeneratorFunction, InitAsyncGeneratorFunction, nullptr)          \
   REAL(ReadableStream, InitViaClassSpec, &js::ReadableStream::class_)        \
   REAL(ReadableStreamDefaultReader, InitViaClassSpec,                        \
        &js::ReadableStreamDefaultReader::class_)                             \
   REAL(ReadableStreamDefaultController, InitViaClassSpec,                    \
        &js::ReadableStreamDefaultController::class_)                         \
   REAL(ReadableByteStreamController, InitViaClassSpec,                       \
        &js::ReadableByteStreamController::class_)                            \
--- a/js/src/jsapi.cpp
+++ b/js/src/jsapi.cpp
@@ -891,17 +891,16 @@ JS_PUBLIC_API bool JS_ResolveStandardCla
   if (stdnm && GlobalObject::skipDeselectedConstructor(cx, stdnm->key)) {
     stdnm = nullptr;
   }
 
   // If this class is anonymous, then it doesn't exist as a global
   // property, so we won't resolve anything.
   JSProtoKey key = stdnm ? stdnm->key : JSProto_Null;
   if (key != JSProto_Null && key != JSProto_AsyncFunction &&
-      key != JSProto_GeneratorFunction &&
       key != JSProto_AsyncGeneratorFunction) {
     const JSClass* clasp = ProtoKeyToClass(key);
     if (!clasp || clasp->specShouldDefineConstructor()) {
       if (!GlobalObject::ensureConstructor(cx, global, key)) {
         return false;
       }
 
       *resolved = true;
@@ -968,19 +967,18 @@ static bool EnumerateStandardClassesInTa
     if (!includeResolved && global->isStandardClassResolved(key)) {
       continue;
     }
 
     if (GlobalObject::skipDeselectedConstructor(cx, key)) {
       continue;
     }
 
-    // Async(Function|Generator) and Generator don't yet use ClassSpec.
-    if (key == JSProto_AsyncFunction || key == JSProto_GeneratorFunction ||
-        key == JSProto_AsyncGeneratorFunction) {
+    // Async(Function|Generator) don't yet use ClassSpec.
+    if (key == JSProto_AsyncFunction || key == JSProto_AsyncGeneratorFunction) {
       continue;
     }
 
     if (const JSClass* clasp = ProtoKeyToClass(key)) {
       if (!clasp->specShouldDefineConstructor()) {
         continue;
       }
     }
--- a/js/src/vm/GeneratorObject.cpp
+++ b/js/src/vm/GeneratorObject.cpp
@@ -4,16 +4,17 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "vm/GeneratorObject.h"
 
 #include "js/PropertySpec.h"
 #include "vm/AsyncFunction.h"
 #include "vm/AsyncIteration.h"
+#include "vm/GlobalObject.h"
 #include "vm/JSObject.h"
 
 #include "debugger/DebugAPI-inl.h"
 #include "vm/ArrayObject-inl.h"
 #include "vm/JSAtom-inl.h"
 #include "vm/JSScript-inl.h"
 #include "vm/NativeObject-inl.h"
 #include "vm/Stack-inl.h"
@@ -235,72 +236,96 @@ JSObject* js::NewSingletonObjectWithFunc
     return nullptr;
   }
   if (!JSObject::setDelegate(cx, obj)) {
     return nullptr;
   }
   return obj;
 }
 
-JSObject* js::InitGeneratorFunction(JSContext* cx,
-                                    Handle<GlobalObject*> global) {
+static JSObject* CreateGeneratorFunction(JSContext* cx, JSProtoKey key) {
+  RootedObject proto(
+      cx, GlobalObject::getOrCreateFunctionConstructor(cx, cx->global()));
+  if (!proto) {
+    return nullptr;
+  }
+
+  HandlePropertyName name = cx->names().GeneratorFunction;
+  return NewFunctionWithProto(cx, Generator, 1, FunctionFlags::NATIVE_CTOR,
+                              nullptr, name, proto, gc::AllocKind::FUNCTION,
+                              SingletonObject);
+}
+
+static JSObject* CreateGeneratorFunctionPrototype(JSContext* cx,
+                                                  JSProtoKey key) {
+  return NewSingletonObjectWithFunctionPrototype(cx, cx->global());
+}
+
+static bool GeneratorFunctionClassFinish(JSContext* cx,
+                                         HandleObject genFunction,
+                                         HandleObject genFunctionProto) {
+  Handle<GlobalObject*> global = cx->global();
+
+  // Change the "constructor" property to non-writable before adding any other
+  // properties, so it's still the last property and can be modified without a
+  // dictionary-mode transition.
+  MOZ_ASSERT(StringEqualsAscii(
+      JSID_TO_LINEAR_STRING(
+          genFunctionProto->as<NativeObject>().lastProperty()->propid()),
+      "constructor"));
+  MOZ_ASSERT(!genFunctionProto->as<NativeObject>().inDictionaryMode());
+
+  RootedValue genFunctionVal(cx, ObjectValue(*genFunction));
+  if (!DefineDataProperty(cx, genFunctionProto, cx->names().constructor,
+                          genFunctionVal, JSPROP_READONLY)) {
+    return false;
+  }
+  MOZ_ASSERT(!genFunctionProto->as<NativeObject>().inDictionaryMode());
+
   RootedObject iteratorProto(
       cx, GlobalObject::getOrCreateIteratorPrototype(cx, global));
   if (!iteratorProto) {
-    return nullptr;
+    return false;
   }
 
   RootedObject genObjectProto(cx, GlobalObject::createBlankPrototypeInheriting(
                                       cx, &PlainObject::class_, iteratorProto));
   if (!genObjectProto) {
-    return nullptr;
+    return false;
   }
   if (!DefinePropertiesAndFunctions(cx, genObjectProto, nullptr,
                                     generator_methods) ||
       !DefineToStringTag(cx, genObjectProto, cx->names().Generator)) {
-    return nullptr;
+    return false;
   }
 
-  RootedObject genFunctionProto(
-      cx, NewSingletonObjectWithFunctionPrototype(cx, global));
-  if (!genFunctionProto) {
-    return nullptr;
-  }
   if (!LinkConstructorAndPrototype(cx, genFunctionProto, genObjectProto,
                                    JSPROP_READONLY, JSPROP_READONLY) ||
       !DefineToStringTag(cx, genFunctionProto, cx->names().GeneratorFunction)) {
-    return nullptr;
-  }
-
-  RootedObject proto(
-      cx, GlobalObject::getOrCreateFunctionConstructor(cx, cx->global()));
-  if (!proto) {
-    return nullptr;
-  }
-  HandlePropertyName name = cx->names().GeneratorFunction;
-  RootedObject genFunction(
-      cx, NewFunctionWithProto(cx, Generator, 1, FunctionFlags::NATIVE_CTOR,
-                               nullptr, name, proto, gc::AllocKind::FUNCTION,
-                               SingletonObject));
-  if (!genFunction) {
-    return nullptr;
-  }
-  if (!LinkConstructorAndPrototype(cx, genFunction, genFunctionProto,
-                                   JSPROP_PERMANENT | JSPROP_READONLY,
-                                   JSPROP_READONLY)) {
-    return nullptr;
+    return false;
   }
 
   global->setGeneratorObjectPrototype(genObjectProto);
-  global->setConstructor(JSProto_GeneratorFunction, ObjectValue(*genFunction));
-  global->setPrototype(JSProto_GeneratorFunction,
-                       ObjectValue(*genFunctionProto));
-  return genFunction;
+
+  return true;
 }
 
+static const ClassSpec GeneratorFunctionClassSpec = {
+    CreateGeneratorFunction,
+    CreateGeneratorFunctionPrototype,
+    nullptr,
+    nullptr,
+    nullptr,
+    nullptr,
+    GeneratorFunctionClassFinish,
+    ClassSpec::DontDefineConstructor};
+
+const JSClass js::GeneratorFunctionClass = {
+    "GeneratorFunction", 0, JS_NULL_CLASS_OPS, &GeneratorFunctionClassSpec};
+
 bool AbstractGeneratorObject::isAfterYield() {
   return isAfterYieldOrAwait(JSOP_YIELD);
 }
 
 bool AbstractGeneratorObject::isAfterAwait() {
   return isAfterYieldOrAwait(JSOP_AWAIT);
 }
 
--- a/js/src/vm/GeneratorObject.h
+++ b/js/src/vm/GeneratorObject.h
@@ -11,17 +11,17 @@
 #include "vm/ArgumentsObject.h"
 #include "vm/ArrayObject.h"
 #include "vm/JSContext.h"
 #include "vm/JSObject.h"
 #include "vm/Stack.h"
 
 namespace js {
 
-class GlobalObject;
+extern const JSClass GeneratorFunctionClass;
 
 enum class GeneratorResumeKind { Next, Throw, Return };
 
 class AbstractGeneratorObject : public NativeObject {
  public:
   // Magic value stored in the resumeIndex slot when the generator is
   // running or closing. See the resumeIndex comment below.
   static const int32_t RESUME_INDEX_RUNNING = INT32_MAX;
@@ -220,17 +220,14 @@ bool GeneratorThrowOrReturn(JSContext* c
  *   only on the stack, and not the `.generator` pseudo-variable this function
  *   consults.
  */
 AbstractGeneratorObject* GetGeneratorObjectForFrame(JSContext* cx,
                                                     AbstractFramePtr frame);
 
 void SetGeneratorClosed(JSContext* cx, AbstractFramePtr frame);
 
-extern JSObject* InitGeneratorFunction(JSContext* cx,
-                                       js::Handle<GlobalObject*> global);
-
 }  // namespace js
 
 template <>
 bool JSObject::is<js::AbstractGeneratorObject>() const;
 
 #endif /* vm_GeneratorObject_h */