Bug 1532946 - Tidy allocation functions by renaming overloads for object and string allocation r=sfink

jonco3 · jonco3 · commit abb2fb97f879 · 2019-03-06T11:23:08.000Z
diff --git a/js/src/builtin/TypedObject.cpp b/js/src/builtin/TypedObject.cpp
@@ -2317,7 +2317,7 @@ bool TypedObject::construct(JSContext* cx, unsigned int argc, Value* vp) {
   MOZ_ASSERT(::IsTypedObjectClass(clasp));
 
   JSObject* obj =
-      js::Allocate<JSObject>(cx, kind, /* nDynamicSlots = */ 0, heap, clasp);
+      js::AllocateObject(cx, kind, /* nDynamicSlots = */ 0, heap, clasp);
   if (!obj) {
     return cx->alreadyReportedOOM();
   }
diff --git a/js/src/gc/Allocator.cpp b/js/src/gc/Allocator.cpp
@@ -25,11 +25,9 @@
 using namespace js;
 using namespace gc;
 
-template <typename T, AllowGC allowGC /* = CanGC */>
-JSObject* js::Allocate(JSContext* cx, AllocKind kind, size_t nDynamicSlots,
-                       InitialHeap heap, const Class* clasp) {
-  static_assert(mozilla::IsConvertible<T*, JSObject*>::value,
-                "must be JSObject derived");
+template <AllowGC allowGC /* = CanGC */>
+JSObject* js::AllocateObject(JSContext* cx, AllocKind kind, size_t nDynamicSlots,
+                             InitialHeap heap, const Class* clasp) {
   MOZ_ASSERT(IsObjectAllocKind(kind));
   size_t thingSize = Arena::thingSize(kind);
 
@@ -77,16 +75,16 @@ JSObject* js::Allocate(JSContext* cx, AllocKind kind, size_t nDynamicSlots,
   return GCRuntime::tryNewTenuredObject<allowGC>(cx, kind, thingSize,
                                                  nDynamicSlots);
 }
-template JSObject* js::Allocate<JSObject, NoGC>(JSContext* cx,
-                                                gc::AllocKind kind,
-                                                size_t nDynamicSlots,
-                                                gc::InitialHeap heap,
-                                                const Class* clasp);
-template JSObject* js::Allocate<JSObject, CanGC>(JSContext* cx,
-                                                 gc::AllocKind kind,
-                                                 size_t nDynamicSlots,
-                                                 gc::InitialHeap heap,
-                                                 const Class* clasp);
+template JSObject* js::AllocateObject<NoGC>(JSContext* cx,
+                                            gc::AllocKind kind,
+                                            size_t nDynamicSlots,
+                                            gc::InitialHeap heap,
+                                            const Class* clasp);
+template JSObject* js::AllocateObject<CanGC>(JSContext* cx,
+                                               gc::AllocKind kind,
+                                               size_t nDynamicSlots,
+                                               gc::InitialHeap heap,
+                                               const Class* clasp);
 
 // Attempt to allocate a new JSObject out of the nursery. If there is not
 // enough room in the nursery or there is an OOM, this method will return
@@ -177,7 +175,7 @@ JSString* GCRuntime::tryNewNurseryString(JSContext* cx, size_t thingSize,
 }
 
 template <typename StringAllocT, AllowGC allowGC /* = CanGC */>
-StringAllocT* js::AllocateString(JSContext* cx, InitialHeap heap) {
+StringAllocT* js::AllocateStringImpl(JSContext* cx, InitialHeap heap) {
   static_assert(mozilla::IsConvertible<StringAllocT*, JSString*>::value,
                 "must be JSString derived");
 
@@ -224,10 +222,10 @@ StringAllocT* js::AllocateString(JSContext* cx, InitialHeap heap) {
 
 #define DECL_ALLOCATOR_INSTANCES(allocKind, traceKind, type, sizedType, \
                                  bgfinal, nursery, compact)             \
-  template type* js::AllocateString<type, NoGC>(JSContext * cx,         \
-                                                InitialHeap heap);      \
-  template type* js::AllocateString<type, CanGC>(JSContext * cx,        \
-                                                 InitialHeap heap);
+  template type* js::AllocateStringImpl<type, NoGC>(JSContext * cx,     \
+                                                    InitialHeap heap);  \
+  template type* js::AllocateStringImpl<type, CanGC>(JSContext * cx,    \
+                                                     InitialHeap heap);
 FOR_EACH_NURSERY_STRING_ALLOCKIND(DECL_ALLOCATOR_INSTANCES)
 #undef DECL_ALLOCATOR_INSTANCES
 
diff --git a/js/src/gc/Allocator.h b/js/src/gc/Allocator.h
@@ -17,47 +17,52 @@ namespace js {
 
 struct Class;
 
-// Allocate a new GC thing. After a successful allocation the caller must
-// fully initialize the thing before calling any function that can potentially
-// trigger GC. This will ensure that GC tracing never sees junk values stored
-// in the partially initialized thing.
-
+// Allocate a new GC thing that's not a JSObject or a string.
+//
+// After a successful allocation the caller must fully initialize the thing
+// before calling any function that can potentially trigger GC. This will ensure
+// that GC tracing never sees junk values stored in the partially initialized
+// thing.
 template <typename T, AllowGC allowGC = CanGC>
 T* Allocate(JSContext* cx);
 
-// Use for JSObject. A longer signature that includes additional information in
-// support of various optimizations. If dynamic slots are requested they will be
-// allocated and the pointer stored directly in |NativeObject::slots_|.
-template <typename, AllowGC allowGC = CanGC>
-JSObject* Allocate(JSContext* cx, gc::AllocKind kind, size_t nDynamicSlots,
-                   gc::InitialHeap heap, const Class* clasp);
+// Allocate a JSObject.
+//
+// A longer signature that includes additional information in support of various
+// optimizations. If dynamic slots are requested they will be allocated and the
+// pointer stored directly in |NativeObject::slots_|.
+template <AllowGC allowGC = CanGC>
+JSObject* AllocateObject(JSContext* cx, gc::AllocKind kind, size_t nDynamicSlots,
+                         gc::InitialHeap heap, const Class* clasp);
 
 // Internal function used for nursery-allocatable strings.
 template <typename StringAllocT, AllowGC allowGC = CanGC>
-StringAllocT* AllocateString(JSContext* cx, gc::InitialHeap heap);
+StringAllocT* AllocateStringImpl(JSContext* cx, gc::InitialHeap heap);
 
+// Allocate a string.
+//
 // Use for nursery-allocatable strings. Returns a value cast to the correct
 // type.
 template <typename StringT, AllowGC allowGC = CanGC>
-StringT* Allocate(JSContext* cx, gc::InitialHeap heap) {
-  return static_cast<StringT*>(js::AllocateString<JSString, allowGC>(cx, heap));
+StringT* AllocateString(JSContext* cx, gc::InitialHeap heap) {
+  return static_cast<StringT*>(AllocateStringImpl<JSString, allowGC>(cx, heap));
 }
 
 // Specialization for JSFatInlineString that must use a different allocation
 // type. Note that we have to explicitly specialize for both values of AllowGC
 // because partial function specialization is not allowed.
 template <>
-inline JSFatInlineString* Allocate<JSFatInlineString, CanGC>(
+inline JSFatInlineString* AllocateString<JSFatInlineString, CanGC>(
     JSContext* cx, gc::InitialHeap heap) {
   return static_cast<JSFatInlineString*>(
-      js::AllocateString<JSFatInlineString, CanGC>(cx, heap));
+      js::AllocateStringImpl<JSFatInlineString, CanGC>(cx, heap));
 }
 
 template <>
-inline JSFatInlineString* Allocate<JSFatInlineString, NoGC>(
+inline JSFatInlineString* AllocateString<JSFatInlineString, NoGC>(
     JSContext* cx, gc::InitialHeap heap) {
   return static_cast<JSFatInlineString*>(
-      js::AllocateString<JSFatInlineString, NoGC>(cx, heap));
+      js::AllocateStringImpl<JSFatInlineString, NoGC>(cx, heap));
 }
 
 }  // namespace js
diff --git a/js/src/jit/CodeGenerator.cpp b/js/src/jit/CodeGenerator.cpp
@@ -2402,12 +2402,12 @@ void CreateDependentString::generate(MacroAssembler& masm,
 
 static void* AllocateString(JSContext* cx) {
   AutoUnsafeCallWithABI unsafe;
-  return js::Allocate<JSString, NoGC>(cx, js::gc::TenuredHeap);
+  return js::AllocateString<JSString, NoGC>(cx, js::gc::TenuredHeap);
 }
 
 static void* AllocateFatInlineString(JSContext* cx) {
   AutoUnsafeCallWithABI unsafe;
-  return js::Allocate<JSFatInlineString, NoGC>(cx, js::gc::TenuredHeap);
+  return js::AllocateString<JSFatInlineString, NoGC>(cx, js::gc::TenuredHeap);
 }
 
 void CreateDependentString::generateFallback(MacroAssembler& masm) {
@@ -2443,8 +2443,8 @@ void CreateDependentString::generateFallback(MacroAssembler& masm) {
 static void* CreateMatchResultFallbackFunc(JSContext* cx, gc::AllocKind kind,
                                            size_t nDynamicSlots) {
   AutoUnsafeCallWithABI unsafe;
-  return js::Allocate<JSObject, NoGC>(cx, kind, nDynamicSlots, gc::DefaultHeap,
-                                      &ArrayObject::class_);
+  return js::AllocateObject<NoGC>(cx, kind, nDynamicSlots, gc::DefaultHeap,
+                                  &ArrayObject::class_);
 }
 
 static void CreateMatchResultFallback(MacroAssembler& masm, Register object,
diff --git a/js/src/vm/ArrayObject-inl.h b/js/src/vm/ArrayObject-inl.h
@@ -50,7 +50,7 @@ inline void ArrayObject::setLength(JSContext* cx, uint32_t length) {
   MOZ_ASSERT(shape->numFixedSlots() == 0);
 
   size_t nDynamicSlots = dynamicSlotsCount(0, shape->slotSpan(), clasp);
-  JSObject* obj = js::Allocate<JSObject>(cx, kind, nDynamicSlots, heap, clasp);
+  JSObject* obj = js::AllocateObject(cx, kind, nDynamicSlots, heap, clasp);
   if (!obj) {
     return nullptr;
   }
diff --git a/js/src/vm/Caches-inl.h b/js/src/vm/Caches-inl.h
@@ -73,8 +73,8 @@ inline NativeObject* NewObjectCache::newObjectFromHit(JSContext* cx,
   }
 
   NativeObject* obj = static_cast<NativeObject*>(
-      Allocate<JSObject, NoGC>(cx, entry->kind,
-                               /* nDynamicSlots = */ 0, heap, group->clasp()));
+      AllocateObject<NoGC>(cx, entry->kind, /* nDynamicSlots = */ 0,
+                           heap, group->clasp()));
   if (!obj) {
     return nullptr;
   }
diff --git a/js/src/vm/JSFunction-inl.h b/js/src/vm/JSFunction-inl.h
@@ -113,8 +113,7 @@ inline JSFunction* CloneFunctionObjectIfNotSingleton(
   MOZ_ASSERT(dynamicSlotsCount(shape->numFixedSlots(), shape->slotSpan(),
                                clasp) == NumDynamicSlots);
 
-  JSObject* obj =
-      js::Allocate<JSObject>(cx, kind, NumDynamicSlots, heap, clasp);
+  JSObject* obj = js::AllocateObject(cx, kind, NumDynamicSlots, heap, clasp);
   if (!obj) {
     return cx->alreadyReportedOOM();
   }
diff --git a/js/src/vm/NativeObject-inl.h b/js/src/vm/NativeObject-inl.h
@@ -488,7 +488,7 @@ inline bool NativeObject::isInWholeCellBuffer() const {
   size_t nDynamicSlots =
       dynamicSlotsCount(shape->numFixedSlots(), shape->slotSpan(), clasp);
 
-  JSObject* obj = js::Allocate<JSObject>(cx, kind, nDynamicSlots, heap, clasp);
+  JSObject* obj = js::AllocateObject(cx, kind, nDynamicSlots, heap, clasp);
   if (!obj) {
     return cx->alreadyReportedOOM();
   }
diff --git a/js/src/vm/ProxyObject.cpp b/js/src/vm/ProxyObject.cpp
@@ -187,8 +187,8 @@ void ProxyObject::nuke() {
   gc::InitialHeap heap = GetInitialHeap(newKind, group);
   debugCheckNewObject(group, shape, allocKind, heap);
 
-  JSObject* obj = js::Allocate<JSObject>(cx, allocKind, /* nDynamicSlots = */ 0,
-                                         heap, clasp);
+  JSObject* obj = js::AllocateObject(cx, allocKind, /* nDynamicSlots = */ 0,
+                                     heap, clasp);
   if (!obj) {
     return cx->alreadyReportedOOM();
   }
diff --git a/js/src/vm/StringType-inl.h b/js/src/vm/StringType-inl.h
@@ -159,7 +159,7 @@ MOZ_ALWAYS_INLINE JSRope* JSRope::new_(
   if (!validateLength(cx, length)) {
     return nullptr;
   }
-  JSRope* str = js::Allocate<JSRope, allowGC>(cx, heap);
+  JSRope* str = js::AllocateString<JSRope, allowGC>(cx, heap);
   if (!str) {
     return nullptr;
   }
@@ -220,15 +220,15 @@ MOZ_ALWAYS_INLINE JSLinearString* JSDependentString::new_(
   }
 
   JSDependentString* str =
-      js::Allocate<JSDependentString, js::NoGC>(cx, js::gc::DefaultHeap);
+      js::AllocateString<JSDependentString, js::NoGC>(cx, js::gc::DefaultHeap);
   if (str) {
     str->init(cx, baseArg, start, length);
     return str;
   }
 
   js::RootedLinearString base(cx, baseArg);
 
-  str = js::Allocate<JSDependentString>(cx, js::gc::DefaultHeap);
+  str = js::AllocateString<JSDependentString>(cx, js::gc::DefaultHeap);
   if (!str) {
     return nullptr;
   }
@@ -262,7 +262,7 @@ MOZ_ALWAYS_INLINE JSFlatString* JSFlatString::new_(JSContext* cx,
   if (cx->zone()->isAtomsZone()) {
     str = js::Allocate<js::NormalAtom, allowGC>(cx);
   } else {
-    str = js::Allocate<JSFlatString, allowGC>(cx, js::gc::DefaultHeap);
+    str = js::AllocateString<JSFlatString, allowGC>(cx, js::gc::DefaultHeap);
   }
   if (!str) {
     return nullptr;
@@ -308,7 +308,7 @@ MOZ_ALWAYS_INLINE JSThinInlineString* JSThinInlineString::new_(JSContext* cx) {
     return (JSThinInlineString*)(js::Allocate<js::NormalAtom, allowGC>(cx));
   }
 
-  return js::Allocate<JSThinInlineString, allowGC>(cx, js::gc::DefaultHeap);
+  return js::AllocateString<JSThinInlineString, allowGC>(cx, js::gc::DefaultHeap);
 }
 
 template <js::AllowGC allowGC>
@@ -317,7 +317,7 @@ MOZ_ALWAYS_INLINE JSFatInlineString* JSFatInlineString::new_(JSContext* cx) {
     return (JSFatInlineString*)(js::Allocate<js::FatInlineAtom, allowGC>(cx));
   }
 
-  return js::Allocate<JSFatInlineString, allowGC>(cx, js::gc::DefaultHeap);
+  return js::AllocateString<JSFatInlineString, allowGC>(cx, js::gc::DefaultHeap);
 }
 
 template <>
diff --git a/js/src/vm/UnboxedObject.cpp b/js/src/vm/UnboxedObject.cpp
@@ -846,7 +846,7 @@ NativeObject* UnboxedPlainObject::convertToNative(JSContext* cx,
   debugCheckNewObject(group, /* shape = */ nullptr, kind, heap);
 
   JSObject* obj =
-      js::Allocate<JSObject>(cx, kind, /* nDynamicSlots = */ 0, heap, clasp);
+      js::AllocateObject(cx, kind, /* nDynamicSlots = */ 0, heap, clasp);
   if (!obj) {
     return cx->alreadyReportedOOM();
   }

Original file line number	Diff line number	Diff line change
`@@ -2317,7 +2317,7 @@ bool TypedObject::construct(JSContext* cx, unsigned int argc, Value* vp) {`
`2317`	`2317`	`MOZ_ASSERT(::IsTypedObjectClass(clasp));`
`2318`	`2318`
`2319`	`2319`	`JSObject* obj =`
`2320`		`- js::Allocate<JSObject>(cx, kind, /* nDynamicSlots = */ 0, heap, clasp);`
	`2320`	`+ js::AllocateObject(cx, kind, /* nDynamicSlots = */ 0, heap, clasp);`
`2321`	`2321`	`if (!obj) {`
`2322`	`2322`	`return cx->alreadyReportedOOM();`
`2323`	`2323`	`}`
Original file line number	Diff line number	Diff line change
`@@ -50,7 +50,7 @@ inline void ArrayObject::setLength(JSContext* cx, uint32_t length) {`
`50`	`50`	`MOZ_ASSERT(shape->numFixedSlots() == 0);`
`51`	`51`
`52`	`52`	`size_t nDynamicSlots = dynamicSlotsCount(0, shape->slotSpan(), clasp);`
`53`		`- JSObject* obj = js::Allocate<JSObject>(cx, kind, nDynamicSlots, heap, clasp);`
	`53`	`+ JSObject* obj = js::AllocateObject(cx, kind, nDynamicSlots, heap, clasp);`
`54`	`54`	`if (!obj) {`
`55`	`55`	`return nullptr;`
`56`	`56`	`}`
Original file line number	Diff line number	Diff line change
`@@ -73,8 +73,8 @@ inline NativeObject* NewObjectCache::newObjectFromHit(JSContext* cx,`
`73`	`73`	`}`
`74`	`74`
`75`	`75`	`NativeObject* obj = static_cast<NativeObject*>(`
`76`		`- Allocate<JSObject, NoGC>(cx, entry->kind,`
`77`		`- /* nDynamicSlots = */ 0, heap, group->clasp()));`
	`76`	`+ AllocateObject<NoGC>(cx, entry->kind, /* nDynamicSlots = */ 0,`
	`77`	`+ heap, group->clasp()));`
`78`	`78`	`if (!obj) {`
`79`	`79`	`return nullptr;`
`80`	`80`	`}`
Original file line number	Diff line number	Diff line change
`@@ -113,8 +113,7 @@ inline JSFunction* CloneFunctionObjectIfNotSingleton(`
`113`	`113`	`MOZ_ASSERT(dynamicSlotsCount(shape->numFixedSlots(), shape->slotSpan(),`
`114`	`114`	`clasp) == NumDynamicSlots);`
`115`	`115`
`116`		`- JSObject* obj =`
`117`		`- js::Allocate<JSObject>(cx, kind, NumDynamicSlots, heap, clasp);`
	`116`	`+ JSObject* obj = js::AllocateObject(cx, kind, NumDynamicSlots, heap, clasp);`
`118`	`117`	`if (!obj) {`
`119`	`118`	`return cx->alreadyReportedOOM();`
`120`	`119`	`}`
Original file line number	Diff line number	Diff line change
`@@ -488,7 +488,7 @@ inline bool NativeObject::isInWholeCellBuffer() const {`
`488`	`488`	`size_t nDynamicSlots =`
`489`	`489`	`dynamicSlotsCount(shape->numFixedSlots(), shape->slotSpan(), clasp);`
`490`	`490`
`491`		`- JSObject* obj = js::Allocate<JSObject>(cx, kind, nDynamicSlots, heap, clasp);`
	`491`	`+ JSObject* obj = js::AllocateObject(cx, kind, nDynamicSlots, heap, clasp);`
`492`	`492`	`if (!obj) {`
`493`	`493`	`return cx->alreadyReportedOOM();`
`494`	`494`	`}`
Original file line number	Diff line number	Diff line change
`@@ -159,7 +159,7 @@ MOZ_ALWAYS_INLINE JSRope* JSRope::new_(`
`159`	`159`	`if (!validateLength(cx, length)) {`
`160`	`160`	`return nullptr;`
`161`	`161`	`}`
`162`		`- JSRope* str = js::Allocate<JSRope, allowGC>(cx, heap);`
	`162`	`+ JSRope* str = js::AllocateString<JSRope, allowGC>(cx, heap);`
`163`	`163`	`if (!str) {`
`164`	`164`	`return nullptr;`
`165`	`165`	`}`
`@@ -220,15 +220,15 @@ MOZ_ALWAYS_INLINE JSLinearString* JSDependentString::new_(`
`220`	`220`	`}`
`221`	`221`
`222`	`222`	`JSDependentString* str =`
`223`		`- js::Allocate<JSDependentString, js::NoGC>(cx, js::gc::DefaultHeap);`
	`223`	`+ js::AllocateString<JSDependentString, js::NoGC>(cx, js::gc::DefaultHeap);`
`224`	`224`	`if (str) {`
`225`	`225`	`str->init(cx, baseArg, start, length);`
`226`	`226`	`return str;`
`227`	`227`	`}`
`228`	`228`
`229`	`229`	`js::RootedLinearString base(cx, baseArg);`
`230`	`230`
`231`		`- str = js::Allocate<JSDependentString>(cx, js::gc::DefaultHeap);`
	`231`	`+ str = js::AllocateString<JSDependentString>(cx, js::gc::DefaultHeap);`
`232`	`232`	`if (!str) {`
`233`	`233`	`return nullptr;`
`234`	`234`	`}`
`@@ -262,7 +262,7 @@ MOZ_ALWAYS_INLINE JSFlatString* JSFlatString::new_(JSContext* cx,`
`262`	`262`	`if (cx->zone()->isAtomsZone()) {`
`263`	`263`	`str = js::Allocate<js::NormalAtom, allowGC>(cx);`
`264`	`264`	`} else {`
`265`		`- str = js::Allocate<JSFlatString, allowGC>(cx, js::gc::DefaultHeap);`
	`265`	`+ str = js::AllocateString<JSFlatString, allowGC>(cx, js::gc::DefaultHeap);`
`266`	`266`	`}`
`267`	`267`	`if (!str) {`
`268`	`268`	`return nullptr;`
`@@ -308,7 +308,7 @@ MOZ_ALWAYS_INLINE JSThinInlineString* JSThinInlineString::new_(JSContext* cx) {`
`308`	`308`	`return (JSThinInlineString*)(js::Allocate<js::NormalAtom, allowGC>(cx));`
`309`	`309`	`}`
`310`	`310`
`311`		`- return js::Allocate<JSThinInlineString, allowGC>(cx, js::gc::DefaultHeap);`
	`311`	`+ return js::AllocateString<JSThinInlineString, allowGC>(cx, js::gc::DefaultHeap);`
`312`	`312`	`}`
`313`	`313`
`314`	`314`	`template <js::AllowGC allowGC>`
`@@ -317,7 +317,7 @@ MOZ_ALWAYS_INLINE JSFatInlineString* JSFatInlineString::new_(JSContext* cx) {`
`317`	`317`	`return (JSFatInlineString*)(js::Allocate<js::FatInlineAtom, allowGC>(cx));`
`318`	`318`	`}`
`319`	`319`
`320`		`- return js::Allocate<JSFatInlineString, allowGC>(cx, js::gc::DefaultHeap);`
	`320`	`+ return js::AllocateString<JSFatInlineString, allowGC>(cx, js::gc::DefaultHeap);`
`321`	`321`	`}`
`322`	`322`
`323`	`323`	`template <>`