Bug 1599416 - Part 14: Remove TypedObject::isAttached because it always returns true. r=mgaudet
authorAndré Bargull <andre.bargull@gmail.com>
Wed, 27 Nov 2019 14:12:23 +0000
changeset 504064 7a5a3cbc9b5be32af3b7666930e87aa33bc63981
parent 504063 8fee074100ca84c0a883b8f404c2d7628a09a584
child 504065 61813f7951e2cb7fab95e9f9caee63105943ee14
push id101711
push useraciure@mozilla.com
push dateWed, 27 Nov 2019 14:49:56 +0000
treeherderautoland@7a5a3cbc9b5b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmgaudet
bugs1599416
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 1599416 - Part 14: Remove TypedObject::isAttached because it always returns true. r=mgaudet As demonstrated in the last patch, OutlineTypedObjects always have an attached datum, which means `TypedObject::isAttached` always returns true for any TypedObject. The two new assertions in `OutlineTypedObject::obj_trace` have been added so it's easier to see that `owner_` is nullptr iff `data_` is nullptr. Differential Revision: https://phabricator.services.mozilla.com/D54718
js/src/builtin/TypedObject.cpp
js/src/builtin/TypedObject.h
js/src/builtin/TypedObject.js
js/src/vm/SelfHosting.cpp
--- a/js/src/builtin/TypedObject.cpp
+++ b/js/src/builtin/TypedObject.cpp
@@ -1489,42 +1489,32 @@ uint32_t TypedObject::offset() const {
   return PointerRangeSize(typedMemBase(), typedMem());
 }
 
 uint32_t TypedObject::length() const {
   return typeDescr().as<ArrayTypeDescr>().length();
 }
 
 uint8_t* TypedObject::typedMem() const {
-  MOZ_ASSERT(isAttached());
-
   if (is<InlineTypedObject>()) {
     return as<InlineTypedObject>().inlineTypedMem();
   }
   return as<OutlineTypedObject>().outOfLineTypedMem();
 }
 
 uint8_t* TypedObject::typedMemBase() const {
-  MOZ_ASSERT(isAttached());
   MOZ_ASSERT(is<OutlineTypedObject>());
 
   JSObject& owner = as<OutlineTypedObject>().owner();
   if (owner.is<ArrayBufferObject>()) {
     return owner.as<ArrayBufferObject>().dataPointer();
   }
   return owner.as<InlineTypedObject>().inlineTypedMem();
 }
 
-bool TypedObject::isAttached() const {
-  if (is<InlineTypedObject>()) {
-    return true;
-  }
-  return as<OutlineTypedObject>().outOfLineTypedMem();
-}
-
 /******************************************************************************
  * Outline typed objects
  */
 
 /*static*/
 OutlineTypedObject* OutlineTypedObject::createUnattached(JSContext* cx,
                                                          HandleTypeDescr descr,
                                                          gc::InitialHeap heap) {
@@ -1584,30 +1574,26 @@ OutlineTypedObject* OutlineTypedObject::
     return nullptr;
   }
 
   obj->setOwnerAndData(nullptr, nullptr);
   return obj;
 }
 
 void OutlineTypedObject::attach(ArrayBufferObject& buffer, uint32_t offset) {
-  MOZ_ASSERT(!isAttached());
   MOZ_ASSERT(offset <= buffer.byteLength());
   MOZ_ASSERT(size() <= buffer.byteLength() - offset);
   MOZ_ASSERT(buffer.hasTypedObjectViews());
   MOZ_ASSERT(!buffer.isDetached());
 
   setOwnerAndData(&buffer, buffer.dataPointer() + offset);
 }
 
 void OutlineTypedObject::attach(JSContext* cx, TypedObject& typedObj,
                                 uint32_t offset) {
-  MOZ_ASSERT(!isAttached());
-  MOZ_ASSERT(typedObj.isAttached());
-
   JSObject* owner = &typedObj;
   if (typedObj.is<OutlineTypedObject>()) {
     owner = &typedObj.as<OutlineTypedObject>().owner();
     MOZ_ASSERT(typedObj.offset() <= UINT32_MAX - offset);
     offset += typedObj.offset();
   }
 
   if (owner->is<ArrayBufferObject>()) {
@@ -1699,18 +1685,20 @@ TypedObject* TypedObject::createZeroed(J
 
 /* static */
 void OutlineTypedObject::obj_trace(JSTracer* trc, JSObject* object) {
   OutlineTypedObject& typedObj = object->as<OutlineTypedObject>();
 
   TraceEdge(trc, typedObj.shapePtr(), "OutlineTypedObject_shape");
 
   if (!typedObj.owner_) {
+    MOZ_ASSERT(!typedObj.data_);
     return;
   }
+  MOZ_ASSERT(typedObj.data_);
 
   TypeDescr& descr = typedObj.typeDescr();
 
   // Mark the owner, watching in case it is moved by the tracer.
   JSObject* oldOwner = typedObj.owner_;
   TraceManuallyBarrieredEdge(trc, &typedObj.owner_, "typed object owner");
   JSObject* owner = typedObj.owner_;
 
@@ -1727,17 +1715,17 @@ void OutlineTypedObject::obj_trace(JSTra
 
     if (trc->isTenuringTracer()) {
       Nursery& nursery = trc->runtime()->gc.nursery();
       nursery.maybeSetForwardingPointer(trc, oldData, newData,
                                         /* direct = */ false);
     }
   }
 
-  if (!descr.opaque() || !typedObj.isAttached()) {
+  if (!descr.opaque()) {
     return;
   }
 
   descr.traceInstance(trc, newData);
 }
 
 bool TypeDescr::hasProperty(const JSAtomState& names, jsid id) {
   switch (kind()) {
@@ -1849,22 +1837,16 @@ bool TypedObject::obj_getProperty(JSCont
 
   switch (typedObj->typeDescr().kind()) {
     case type::Scalar:
     case type::Reference:
       break;
 
     case type::Array:
       if (JSID_IS_ATOM(id, cx->names().length)) {
-        if (!typedObj->isAttached()) {
-          JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr,
-                                    JSMSG_TYPEDOBJECT_HANDLE_UNATTACHED);
-          return false;
-        }
-
         vp.setInt32(typedObj->length());
         return true;
       }
       break;
 
     case type::Struct: {
       Rooted<StructTypeDescr*> descr(
           cx, &typedObj->typeDescr().as<StructTypeDescr>());
@@ -2006,22 +1988,16 @@ bool TypedObject::obj_setProperty(JSCont
 
   return SetPropertyOnProto(cx, obj, id, v, receiver, result);
 }
 
 bool TypedObject::obj_getOwnPropertyDescriptor(
     JSContext* cx, HandleObject obj, HandleId id,
     MutableHandle<PropertyDescriptor> desc) {
   Rooted<TypedObject*> typedObj(cx, &obj->as<TypedObject>());
-  if (!typedObj->isAttached()) {
-    JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr,
-                              JSMSG_TYPEDOBJECT_HANDLE_UNATTACHED);
-    return false;
-  }
-
   Rooted<TypeDescr*> descr(cx, &typedObj->typeDescr());
   switch (descr->kind()) {
     case type::Scalar:
     case type::Reference:
       break;
 
     case type::Array: {
       uint32_t index;
@@ -2434,23 +2410,16 @@ bool js::TypeDescrIsArrayType(JSContext*
   MOZ_ASSERT(args.length() == 1);
   MOZ_ASSERT(args[0].isObject());
   MOZ_ASSERT(args[0].toObject().is<js::TypeDescr>());
   JSObject& obj = args[0].toObject();
   args.rval().setBoolean(obj.is<js::ArrayTypeDescr>());
   return true;
 }
 
-bool js::TypedObjectIsAttached(JSContext* cx, unsigned argc, Value* vp) {
-  CallArgs args = CallArgsFromVp(argc, vp);
-  TypedObject& typedObj = args[0].toObject().as<TypedObject>();
-  args.rval().setBoolean(typedObj.isAttached());
-  return true;
-}
-
 bool js::TypedObjectTypeDescr(JSContext* cx, unsigned argc, Value* vp) {
   CallArgs args = CallArgsFromVp(argc, vp);
   TypedObject& typedObj = args[0].toObject().as<TypedObject>();
   args.rval().setObject(typedObj.typeDescr());
   return true;
 }
 
 bool js::ClampToUint8(JSContext*, unsigned argc, Value* vp) {
--- a/js/src/builtin/TypedObject.h
+++ b/js/src/builtin/TypedObject.h
@@ -602,17 +602,16 @@ class TypedObject : public JSObject {
                                                    js::gc::AllocKind kind,
                                                    js::gc::InitialHeap heap,
                                                    js::HandleShape shape,
                                                    js::HandleObjectGroup group);
 
   uint32_t offset() const;
   uint32_t length() const;
   uint8_t* typedMem(const JS::AutoRequireNoGC&) const { return typedMem(); }
-  bool isAttached() const;
 
   uint32_t size() const { return typeDescr().size(); }
 
   uint8_t* typedMem(size_t offset, const JS::AutoRequireNoGC& nogc) const {
     // It seems a bit surprising that one might request an offset
     // == size(), but it can happen when taking the "address of" a
     // 0-sized value. (In other words, we maintain the invariant
     // that `offset + size <= size()` -- this is always checked in
@@ -814,25 +813,16 @@ MOZ_MUST_USE bool ObjectIsTypedObject(JS
 // Predicates on type descriptor objects.  In all cases, 'obj' must be a type
 // descriptor.
 
 MOZ_MUST_USE bool TypeDescrIsSimpleType(JSContext*, unsigned argc, Value* vp);
 
 MOZ_MUST_USE bool TypeDescrIsArrayType(JSContext*, unsigned argc, Value* vp);
 
 /*
- * Usage: TypedObjectIsAttached(obj)
- *
- * Given a TypedObject `obj`, returns true if `obj` is
- * "attached" (i.e., its data pointer is nullptr).
- */
-MOZ_MUST_USE bool TypedObjectIsAttached(JSContext* cx, unsigned argc,
-                                        Value* vp);
-
-/*
  * Usage: TypedObjectTypeDescr(obj)
  *
  * Given a TypedObject `obj`, returns the object's type descriptor.
  */
 MOZ_MUST_USE bool TypedObjectTypeDescr(JSContext* cx, unsigned argc, Value* vp);
 
 /*
  * Usage: ClampToUint8(v)
--- a/js/src/builtin/TypedObject.js
+++ b/js/src/builtin/TypedObject.js
@@ -42,19 +42,16 @@
 // Reifies the value referenced by the pointer, meaning that it
 // returns a new object pointing at the value. If the value is
 // a scalar, it will return a JS number, but otherwise the reified
 // result will be a typedObj of the same class as the ptr's typedObj.
 function TypedObjectGet(descr, typedObj, offset) {
   assert(IsObject(descr) && ObjectIsTypeDescr(descr),
          "get() called with bad type descr");
 
-  if (!TypedObjectIsAttached(typedObj))
-    ThrowTypeError(JSMSG_TYPEDOBJECT_HANDLE_UNATTACHED);
-
   switch (DESCR_KIND(descr)) {
   case JS_TYPEREPR_SCALAR_KIND:
     return TypedObjectGetScalar(descr, typedObj, offset);
 
   case JS_TYPEREPR_REFERENCE_KIND:
     return TypedObjectGetReference(descr, typedObj, offset);
 
   case JS_TYPEREPR_ARRAY_KIND:
@@ -152,19 +149,16 @@ function TypedObjectGetReference(descr, 
 // Setting values
 //
 // The methods in this section modify the data pointed at by `this`.
 
 // Writes `fromValue` into the `typedObj` at offset `offset`, adapting
 // it to `descr` as needed. This is the most general entry point
 // and works for any type.
 function TypedObjectSet(descr, typedObj, offset, name, fromValue) {
-  if (!TypedObjectIsAttached(typedObj))
-    ThrowTypeError(JSMSG_TYPEDOBJECT_HANDLE_UNATTACHED);
-
   switch (DESCR_KIND(descr)) {
   case JS_TYPEREPR_SCALAR_KIND:
     TypedObjectSetScalar(descr, typedObj, offset, fromValue);
     return;
 
   case JS_TYPEREPR_REFERENCE_KIND:
     TypedObjectSetReference(descr, typedObj, offset, name, fromValue);
     return;
@@ -305,34 +299,28 @@ function ConvertAndCopyTo(destDescr,
                           fieldName,
                           fromValue)
 {
   assert(IsObject(destDescr) && ObjectIsTypeDescr(destDescr),
          "ConvertAndCopyTo: not type obj");
   assert(IsObject(destTypedObj) && ObjectIsTypedObject(destTypedObj),
          "ConvertAndCopyTo: not type typedObj");
 
-  if (!TypedObjectIsAttached(destTypedObj))
-    ThrowTypeError(JSMSG_TYPEDOBJECT_HANDLE_UNATTACHED);
-
   TypedObjectSet(destDescr, destTypedObj, destOffset, fieldName, fromValue);
 }
 
 // Wrapper for use from C++ code.
 function Reify(sourceDescr,
                sourceTypedObj,
                sourceOffset) {
   assert(IsObject(sourceDescr) && ObjectIsTypeDescr(sourceDescr),
          "Reify: not type obj");
   assert(IsObject(sourceTypedObj) && ObjectIsTypedObject(sourceTypedObj),
          "Reify: not type typedObj");
 
-  if (!TypedObjectIsAttached(sourceTypedObj))
-    ThrowTypeError(JSMSG_TYPEDOBJECT_HANDLE_UNATTACHED);
-
   return TypedObjectGet(sourceDescr, sourceTypedObj, sourceOffset);
 }
 
 // Warning: user exposed!
 function TypeDescrEquivalent(otherDescr) {
   if (!IsObject(this) || !ObjectIsTypeDescr(this))
     ThrowTypeError(JSMSG_TYPEDOBJECT_BAD_ARGS);
   if (!IsObject(otherDescr) || !ObjectIsTypeDescr(otherDescr))
--- a/js/src/vm/SelfHosting.cpp
+++ b/js/src/vm/SelfHosting.cpp
@@ -2371,17 +2371,16 @@ static const JSFunctionSpec intrinsic_fu
     JS_INLINABLE_FN("GuardToSetObject", intrinsic_GuardToBuiltin<SetObject>, 1,
                     0, IntrinsicGuardToSetObject),
     JS_FN("CallSetMethodIfWrapped",
           CallNonGenericSelfhostedMethod<Is<SetObject>>, 2, 0),
 
     // See builtin/TypedObject.h for descriptors of the typedobj functions.
     JS_FN("NewOpaqueTypedObject", js::NewOpaqueTypedObject, 3, 0),
     JS_FN("NewDerivedTypedObject", js::NewDerivedTypedObject, 3, 0),
-    JS_FN("TypedObjectIsAttached", js::TypedObjectIsAttached, 1, 0),
     JS_FN("TypedObjectTypeDescr", js::TypedObjectTypeDescr, 1, 0),
     JS_FN("ClampToUint8", js::ClampToUint8, 1, 0),
     JS_FN("GetTypedObjectModule", js::GetTypedObjectModule, 0, 0),
 
     JS_INLINABLE_FN("ObjectIsTypeDescr", js::ObjectIsTypeDescr, 1, 0,
                     IntrinsicObjectIsTypeDescr),
     JS_INLINABLE_FN("ObjectIsTypedObject", js::ObjectIsTypedObject, 1, 0,
                     IntrinsicObjectIsTypedObject),