Bug 1595745 - Part 4: Change Symbol to use ClassSpec. r=mgaudet
☠☠ backed out by ec8cad689121 ☠ ☠
authorAndré Bargull <andre.bargull@gmail.com>
Fri, 15 Nov 2019 15:02:44 +0000
changeset 502198 6c917c2ca4a7b1799ea159d5cc0f88ec85e11148
parent 502197 338ad438e066e879702224ba34317d3956bb2519
child 502199 d5f5e9091fb049e19001b408e9c784178348fd5b
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 4: Change Symbol to use ClassSpec. r=mgaudet A ClassSpec's 'FinishClassInitOp' isn't called when `InitBareBuiltinCtor` is used, which allows us to unconditionally define all well-known symbols in `SymbolClassFinish`. That means we no longer need the separate `InitBareSymbolCtor` function. Differential Revision: https://phabricator.services.mozilla.com/D52660
js/public/ProtoKey.h
js/src/builtin/Symbol.cpp
js/src/builtin/Symbol.h
js/src/vm/GlobalObject.cpp
--- a/js/public/ProtoKey.h
+++ b/js/public/ProtoKey.h
@@ -86,17 +86,17 @@
   REAL(BigInt64Array, InitViaClassSpec, TYPED_ARRAY_CLASP(BigInt64))         \
   REAL(BigUint64Array, InitViaClassSpec, TYPED_ARRAY_CLASP(BigUint64))       \
   REAL(BigInt, InitViaClassSpec, OCLASP(BigInt))                             \
   REAL(Proxy, InitProxyClass, &js::ProxyClass)                               \
   REAL(WeakMap, InitViaClassSpec, OCLASP(WeakMap))                           \
   REAL(Map, InitViaClassSpec, OCLASP(Map))                                   \
   REAL(Set, InitViaClassSpec, OCLASP(Set))                                   \
   REAL(DataView, InitViaClassSpec, OCLASP(DataView))                         \
-  REAL(Symbol, InitSymbolClass, OCLASP(Symbol))                              \
+  REAL(Symbol, InitViaClassSpec, OCLASP(Symbol))                             \
   REAL(SharedArrayBuffer, InitViaClassSpec, OCLASP(SharedArrayBuffer))       \
   REAL_IF_INTL(Intl, InitIntlClass, CLASP(Intl))                             \
   REAL_IF_INTL(Collator, InitViaClassSpec, OCLASP(Collator))                 \
   REAL_IF_INTL(DateTimeFormat, InitViaClassSpec, OCLASP(DateTimeFormat))     \
   REAL_IF_INTL(Locale, InitViaClassSpec, OCLASP(Locale))                     \
   REAL_IF_INTL(ListFormat, InitViaClassSpec, OCLASP(ListFormat))             \
   REAL_IF_INTL(NumberFormat, InitViaClassSpec, OCLASP(NumberFormat))         \
   REAL_IF_INTL(PluralRules, InitViaClassSpec, OCLASP(PluralRules))           \
--- a/js/src/builtin/Symbol.cpp
+++ b/js/src/builtin/Symbol.cpp
@@ -13,88 +13,74 @@
 
 #include "vm/JSObject-inl.h"
 #include "vm/NativeObject-inl.h"
 
 using JS::Symbol;
 using namespace js;
 
 const JSClass SymbolObject::class_ = {
-    "Symbol", JSCLASS_HAS_RESERVED_SLOTS(RESERVED_SLOTS) |
-                  JSCLASS_HAS_CACHED_PROTO(JSProto_Symbol)};
+    "Symbol",
+    JSCLASS_HAS_RESERVED_SLOTS(RESERVED_SLOTS) |
+        JSCLASS_HAS_CACHED_PROTO(JSProto_Symbol),
+    JS_NULL_CLASS_OPS, &SymbolObject::classSpec_};
+
+// This uses PlainObject::class_ because: "The Symbol prototype object is an
+// ordinary object. It is not a Symbol instance and does not have a
+// [[SymbolData]] internal slot." (ES6 rev 24, 19.4.3)
+const JSClass& SymbolObject::protoClass_ = PlainObject::class_;
 
 SymbolObject* SymbolObject::create(JSContext* cx, JS::HandleSymbol symbol) {
   SymbolObject* obj = NewBuiltinClassInstance<SymbolObject>(cx);
   if (!obj) {
     return nullptr;
   }
   obj->setPrimitiveValue(symbol);
   return obj;
 }
 
 const JSPropertySpec SymbolObject::properties[] = {
-    JS_PSG("description", descriptionGetter, 0), JS_PS_END};
+    JS_PSG("description", descriptionGetter, 0),
+    JS_STRING_SYM_PS(toStringTag, "Symbol", JSPROP_READONLY), JS_PS_END};
 
 const JSFunctionSpec SymbolObject::methods[] = {
     JS_FN(js_toString_str, toString, 0, 0),
     JS_FN(js_valueOf_str, valueOf, 0, 0),
     JS_SYM_FN(toPrimitive, toPrimitive, 1, JSPROP_READONLY), JS_FS_END};
 
 const JSFunctionSpec SymbolObject::staticMethods[] = {
     JS_FN("for", for_, 1, 0), JS_FN("keyFor", keyFor, 1, 0), JS_FS_END};
 
-JSObject* SymbolObject::initClass(JSContext* cx, Handle<GlobalObject*> global,
-                                  bool defineMembers) {
-  // This uses &JSObject::class_ because: "The Symbol prototype object is an
-  // ordinary object. It is not a Symbol instance and does not have a
-  // [[SymbolData]] internal slot." (ES6 rev 24, 19.4.3)
-  RootedObject proto(
-      cx, GlobalObject::createBlankPrototype<PlainObject>(cx, global));
-  if (!proto) {
-    return nullptr;
-  }
+static bool SymbolClassFinish(JSContext* cx, HandleObject ctor,
+                              HandleObject proto) {
+  HandleNativeObject nativeCtor = ctor.as<NativeObject>();
 
-  RootedFunction ctor(cx, GlobalObject::createConstructor(
-                              cx, construct, ClassName(JSProto_Symbol, cx), 0));
-  if (!ctor) {
-    return nullptr;
-  }
-
-  if (defineMembers) {
-    // Define the well-known symbol properties, such as Symbol.iterator.
-    ImmutablePropertyNamePtr* names = cx->names().wellKnownSymbolNames();
-    RootedValue value(cx);
-    unsigned attrs = JSPROP_READONLY | JSPROP_PERMANENT;
-    WellKnownSymbols* wks = cx->runtime()->wellKnownSymbols;
-    for (size_t i = 0; i < JS::WellKnownSymbolLimit; i++) {
-      value.setSymbol(wks->get(i));
-      if (!NativeDefineDataProperty(cx, ctor, names[i], value, attrs)) {
-        return nullptr;
-      }
+  // Define the well-known symbol properties, such as Symbol.iterator.
+  ImmutablePropertyNamePtr* names = cx->names().wellKnownSymbolNames();
+  RootedValue value(cx);
+  unsigned attrs = JSPROP_READONLY | JSPROP_PERMANENT;
+  WellKnownSymbols* wks = cx->runtime()->wellKnownSymbols;
+  for (size_t i = 0; i < JS::WellKnownSymbolLimit; i++) {
+    value.setSymbol(wks->get(i));
+    if (!NativeDefineDataProperty(cx, nativeCtor, names[i], value, attrs)) {
+      return false;
     }
   }
-
-  if (!LinkConstructorAndPrototype(cx, ctor, proto)) {
-    return nullptr;
-  }
+  return true;
+}
 
-  if (defineMembers) {
-    if (!DefinePropertiesAndFunctions(cx, proto, properties, methods) ||
-        !DefineToStringTag(cx, proto, cx->names().Symbol) ||
-        !DefinePropertiesAndFunctions(cx, ctor, nullptr, staticMethods)) {
-      return nullptr;
-    }
-  }
-
-  if (!GlobalObject::initBuiltinConstructor(cx, global, JSProto_Symbol, ctor,
-                                            proto)) {
-    return nullptr;
-  }
-  return proto;
-}
+const ClassSpec SymbolObject::classSpec_ = {
+    GenericCreateConstructor<SymbolObject::construct, 0,
+                             gc::AllocKind::FUNCTION>,
+    GenericCreatePrototype<SymbolObject>,
+    staticMethods,
+    nullptr,
+    methods,
+    properties,
+    SymbolClassFinish};
 
 // ES6 rev 24 (2014 Apr 27) 19.4.1.1 and 19.4.1.2
 bool SymbolObject::construct(JSContext* cx, unsigned argc, Value* vp) {
   // According to a note in the draft standard, "Symbol has ordinary
   // [[Construct]] behaviour but the definition of its @@create method causes
   // `new Symbol` to throw a TypeError exception." We do not support @@create
   // yet, so just throw a TypeError.
   CallArgs args = CallArgsFromVp(argc, vp);
@@ -235,16 +221,8 @@ bool SymbolObject::descriptionGetter_imp
   }
   return true;
 }
 
 bool SymbolObject::descriptionGetter(JSContext* cx, unsigned argc, Value* vp) {
   CallArgs args = CallArgsFromVp(argc, vp);
   return CallNonGenericMethod<IsSymbol, descriptionGetter_impl>(cx, args);
 }
-
-JSObject* js::InitSymbolClass(JSContext* cx, Handle<GlobalObject*> global) {
-  return SymbolObject::initClass(cx, global, true);
-}
-
-JSObject* js::InitBareSymbolCtor(JSContext* cx, Handle<GlobalObject*> global) {
-  return SymbolObject::initClass(cx, global, false);
-}
--- a/js/src/builtin/Symbol.h
+++ b/js/src/builtin/Symbol.h
@@ -7,29 +7,25 @@
 #ifndef builtin_Symbol_h
 #define builtin_Symbol_h
 
 #include "vm/NativeObject.h"
 #include "vm/SymbolType.h"
 
 namespace js {
 
-class GlobalObject;
-
 class SymbolObject : public NativeObject {
   /* Stores this Symbol object's [[PrimitiveValue]]. */
   static const unsigned PRIMITIVE_VALUE_SLOT = 0;
 
  public:
   static const unsigned RESERVED_SLOTS = 1;
 
   static const JSClass class_;
-
-  static JSObject* initClass(JSContext* cx, Handle<GlobalObject*> global,
-                             bool defineMembers);
+  static const JSClass& protoClass_;
 
   /*
    * Creates a new Symbol object boxing the given primitive Symbol.  The
    * object's [[Prototype]] is determined from context.
    */
   static SymbolObject* create(JSContext* cx, JS::HandleSymbol symbol);
 
   JS::Symbol* unbox() const {
@@ -58,18 +54,14 @@ class SymbolObject : public NativeObject
   static MOZ_MUST_USE bool descriptionGetter_impl(JSContext* cx,
                                                   const CallArgs& args);
   static MOZ_MUST_USE bool descriptionGetter(JSContext* cx, unsigned argc,
                                              Value* vp);
 
   static const JSPropertySpec properties[];
   static const JSFunctionSpec methods[];
   static const JSFunctionSpec staticMethods[];
+  static const ClassSpec classSpec_;
 };
 
-extern JSObject* InitSymbolClass(JSContext* cx, Handle<GlobalObject*> global);
-
-extern JSObject* InitBareSymbolCtor(JSContext* cx,
-                                    Handle<GlobalObject*> global);
-
 } /* namespace js */
 
 #endif /* builtin_Symbol_h */
--- a/js/src/vm/GlobalObject.cpp
+++ b/js/src/vm/GlobalObject.cpp
@@ -752,17 +752,17 @@ bool GlobalObject::initSelfHostingBuilti
       return false;
     }
   }
 
   return InitBareBuiltinCtor(cx, global, JSProto_Array) &&
          InitBareBuiltinCtor(cx, global, JSProto_TypedArray) &&
          InitBareBuiltinCtor(cx, global, JSProto_Uint8Array) &&
          InitBareBuiltinCtor(cx, global, JSProto_Int32Array) &&
-         InitBareSymbolCtor(cx, global) &&
+         InitBareBuiltinCtor(cx, global, JSProto_Symbol) &&
          DefineFunctions(cx, global, builtins, AsIntrinsic);
 }
 
 /* static */
 bool GlobalObject::isRuntimeCodeGenEnabled(JSContext* cx, HandleValue code,
                                            Handle<GlobalObject*> global) {
   HeapSlot& v = global->getSlotRef(RUNTIME_CODEGEN_ENABLED);
   if (v.isUndefined()) {