commit 4f2b3d65187b5dc57c4e0f25429a3385ae3ef136
parent 2680f1aa2acb32382b1e8c947f85f65e0aced478
Author: Jon Coppeard <jcoppeard@mozilla.com>
Date: Mon, 27 Oct 2025 10:54:30 +0000
Bug 1996539 - Part 1: Remove weakmap type aliases r=sfink
Currently we have a few alias for commonly used WeakMap types. We're going to
add an alloc policy template parameter that's specific to the use so remove
these.
Differential Revision: https://phabricator.services.mozilla.com/D267937
Diffstat:
10 files changed, 57 insertions(+), 58 deletions(-)
diff --git a/js/src/builtin/WeakMapObject-inl.h b/js/src/builtin/WeakMapObject-inl.h
@@ -32,11 +32,11 @@ static MOZ_ALWAYS_INLINE bool EnsureObjectHasWeakMap(
if (obj->getMap()) {
return true;
}
- auto newMap = cx->make_unique<ValueValueWeakMap>(cx, obj);
+ auto newMap = cx->make_unique<WeakCollectionObject::Map>(cx, obj);
if (!newMap) {
return false;
}
- ValueValueWeakMap* map = newMap.release();
+ WeakCollectionObject::Map* map = newMap.release();
InitReservedSlot(obj, WeakCollectionObject::DataSlot, map,
MemoryUse::WeakMapObject);
return true;
diff --git a/js/src/builtin/WeakMapObject.cpp b/js/src/builtin/WeakMapObject.cpp
@@ -37,8 +37,7 @@ using namespace js;
return true;
}
- if (ValueValueWeakMap* map =
- args.thisv().toObject().as<WeakMapObject>().getMap()) {
+ if (Map* map = args.thisv().toObject().as<WeakMapObject>().getMap()) {
Value key = args[0];
if (map->has(key)) {
args.rval().setBoolean(true);
@@ -60,7 +59,7 @@ bool WeakMapObject::has(JSContext* cx, unsigned argc, Value* vp) {
// static
bool WeakMapObject::hasObject(WeakMapObject* weakMap, JSObject* obj) {
AutoUnsafeCallWithABI unsafe;
- ValueValueWeakMap* map = weakMap->getMap();
+ Map* map = weakMap->getMap();
return map && map->has(ObjectValue(*obj));
}
@@ -73,10 +72,9 @@ bool WeakMapObject::hasObject(WeakMapObject* weakMap, JSObject* obj) {
return true;
}
- if (ValueValueWeakMap* map =
- args.thisv().toObject().as<WeakMapObject>().getMap()) {
+ if (Map* map = args.thisv().toObject().as<WeakMapObject>().getMap()) {
Value key = args[0];
- if (ValueValueWeakMap::Ptr ptr = map->lookup(key)) {
+ if (Map::Ptr ptr = map->lookup(key)) {
args.rval().set(ptr->value());
return true;
}
@@ -97,8 +95,8 @@ bool WeakMapObject::get(JSContext* cx, unsigned argc, Value* vp) {
void WeakMapObject::getObject(WeakMapObject* weakMap, JSObject* obj,
Value* result) {
AutoUnsafeCallWithABI unsafe;
- if (ValueValueWeakMap* map = weakMap->getMap()) {
- if (ValueValueWeakMap::Ptr ptr = map->lookup(ObjectValue(*obj))) {
+ if (Map* map = weakMap->getMap()) {
+ if (Map::Ptr ptr = map->lookup(ObjectValue(*obj))) {
*result = ptr->value();
return;
}
@@ -115,13 +113,12 @@ void WeakMapObject::getObject(WeakMapObject* weakMap, JSObject* obj,
return true;
}
- if (ValueValueWeakMap* map =
- args.thisv().toObject().as<WeakMapObject>().getMap()) {
+ if (Map* map = args.thisv().toObject().as<WeakMapObject>().getMap()) {
Value key = args[0];
// The lookup here is only used for the removal, so we can skip the read
// barrier. This is not very important for performance, but makes it easier
// to test nonbarriered removal from internal weakmaps (eg Debugger maps.)
- if (ValueValueWeakMap::Ptr ptr = map->lookupUnbarriered(key)) {
+ if (Map::Ptr ptr = map->lookupUnbarriered(key)) {
map->remove(ptr);
args.rval().setBoolean(true);
return true;
@@ -187,8 +184,8 @@ static bool GetOrAddWeakMapEntry(JSContext* cx, Handle<WeakMapObject*> mapObj,
return false;
}
- ValueValueWeakMap* map = mapObj->getMap();
- ValueValueWeakMap::AddPtr addPtr = map->lookupForAdd(key);
+ WeakCollectionObject::Map* map = mapObj->getMap();
+ auto addPtr = map->lookupForAdd(key);
if (!addPtr) {
if (!PreserveReflectorAndAssertValidEntry(cx, mapObj, key, value)) {
return false;
@@ -219,12 +216,12 @@ bool WeakMapObject::getOrInsert(JSContext* cx, unsigned argc, Value* vp) {
size_t WeakCollectionObject::sizeOfExcludingThis(
mozilla::MallocSizeOf aMallocSizeOf) {
- ValueValueWeakMap* map = getMap();
+ Map* map = getMap();
return map ? map->sizeOfIncludingThis(aMallocSizeOf) : 0;
}
size_t WeakCollectionObject::nondeterministicGetSize() {
- ValueValueWeakMap* map = getMap();
+ Map* map = getMap();
if (!map) {
return 0;
}
@@ -238,10 +235,10 @@ bool WeakCollectionObject::nondeterministicGetKeys(
if (!arr) {
return false;
}
- if (ValueValueWeakMap* map = obj->getMap()) {
+ if (Map* map = obj->getMap()) {
// Prevent GC from mutating the weakmap while iterating.
gc::AutoSuppressGC suppress(cx);
- for (ValueValueWeakMap::Range r = map->all(); !r.empty(); r.popFront()) {
+ for (Map::Range r = map->all(); !r.empty(); r.popFront()) {
const auto& key = r.front().key();
MOZ_ASSERT(key.isObject() || key.isSymbol());
JS::ExposeValueToActiveJS(key);
@@ -270,14 +267,16 @@ JS_PUBLIC_API bool JS_NondeterministicGetWeakMapKeys(JSContext* cx,
cx, obj.as<WeakCollectionObject>(), ret);
}
-static void WeakCollection_trace(JSTracer* trc, JSObject* obj) {
- if (ValueValueWeakMap* map = obj->as<WeakCollectionObject>().getMap()) {
+/* static */
+void WeakCollectionObject::trace(JSTracer* trc, JSObject* obj) {
+ if (Map* map = obj->as<WeakCollectionObject>().getMap()) {
map->trace(trc);
}
}
-static void WeakCollection_finalize(JS::GCContext* gcx, JSObject* obj) {
- if (ValueValueWeakMap* map = obj->as<WeakCollectionObject>().getMap()) {
+/* static */
+void WeakCollectionObject::finalize(JS::GCContext* gcx, JSObject* obj) {
+ if (Map* map = obj->as<WeakCollectionObject>().getMap()) {
gcx->delete_(obj, map, MemoryUse::WeakMapObject);
}
}
@@ -301,12 +300,12 @@ JS_PUBLIC_API bool JS::GetWeakMapEntry(JSContext* cx, HandleObject mapObj,
return true;
}
- ValueValueWeakMap* map = mapObj->as<WeakMapObject>().getMap();
+ WeakMapObject::Map* map = mapObj->as<WeakMapObject>().getMap();
if (!map) {
return true;
}
- if (ValueValueWeakMap::Ptr ptr = map->lookup(key)) {
+ if (auto ptr = map->lookup(key)) {
rval.set(ptr->value());
}
return true;
@@ -410,16 +409,16 @@ bool WeakMapObject::construct(JSContext* cx, unsigned argc, Value* vp) {
}
const JSClassOps WeakCollectionObject::classOps_ = {
- nullptr, // addProperty
- nullptr, // delProperty
- nullptr, // enumerate
- nullptr, // newEnumerate
- nullptr, // resolve
- nullptr, // mayResolve
- WeakCollection_finalize, // finalize
- nullptr, // call
- nullptr, // construct
- WeakCollection_trace, // trace
+ nullptr, // addProperty
+ nullptr, // delProperty
+ nullptr, // enumerate
+ nullptr, // newEnumerate
+ nullptr, // resolve
+ nullptr, // mayResolve
+ &finalize, // finalize
+ nullptr, // call
+ nullptr, // construct
+ &trace, // trace
};
const ClassSpec WeakMapObject::classSpec_ = {
diff --git a/js/src/builtin/WeakMapObject.h b/js/src/builtin/WeakMapObject.h
@@ -17,9 +17,8 @@ class WeakCollectionObject : public NativeObject {
public:
enum { DataSlot, SlotCount };
- ValueValueWeakMap* getMap() {
- return maybePtrFromReservedSlot<ValueValueWeakMap>(DataSlot);
- }
+ using Map = WeakMap<Value, Value>;
+ Map* getMap() { return maybePtrFromReservedSlot<Map>(DataSlot); }
size_t sizeOfExcludingThis(mozilla::MallocSizeOf aMallocSizeOf);
@@ -30,6 +29,9 @@ class WeakCollectionObject : public NativeObject {
protected:
static const JSClassOps classOps_;
+
+ static void trace(JSTracer* trc, JSObject* obj);
+ static void finalize(JS::GCContext* gcx, JSObject* obj);
};
class WeakMapObject : public WeakCollectionObject {
diff --git a/js/src/builtin/WeakSetObject.cpp b/js/src/builtin/WeakSetObject.cpp
@@ -75,10 +75,9 @@ bool WeakSetObject::add(JSContext* cx, unsigned argc, Value* vp) {
}
// Steps 5-6.
- if (ValueValueWeakMap* map =
- args.thisv().toObject().as<WeakSetObject>().getMap()) {
+ if (Map* map = args.thisv().toObject().as<WeakSetObject>().getMap()) {
Value value = args[0];
- if (ValueValueWeakMap::Ptr ptr = map->lookup(value)) {
+ if (Map::Ptr ptr = map->lookup(value)) {
map->remove(ptr);
args.rval().setBoolean(true);
return true;
@@ -111,8 +110,7 @@ bool WeakSetObject::delete_(JSContext* cx, unsigned argc, Value* vp) {
}
// Steps 4, 6.
- if (ValueValueWeakMap* map =
- args.thisv().toObject().as<WeakSetObject>().getMap()) {
+ if (Map* map = args.thisv().toObject().as<WeakSetObject>().getMap()) {
Value value = args[0];
if (map->has(value)) {
args.rval().setBoolean(true);
@@ -136,7 +134,7 @@ bool WeakSetObject::has(JSContext* cx, unsigned argc, Value* vp) {
// static
bool WeakSetObject::hasObject(WeakSetObject* weakSet, JSObject* obj) {
AutoUnsafeCallWithABI unsafe;
- ValueValueWeakMap* map = weakSet->getMap();
+ Map* map = weakSet->getMap();
return map && map->has(ObjectValue(*obj));
}
diff --git a/js/src/gc/WeakMap.h b/js/src/gc/WeakMap.h
@@ -385,12 +385,6 @@ class WeakMap : public WeakMapBase {
void traceMappings(WeakMapTracer* tracer) override;
};
-using ObjectValueWeakMap = WeakMap<JSObject*, Value>;
-using ValueValueWeakMap = WeakMap<Value, Value>;
-
-// Generic weak map for mapping objects to other objects.
-using ObjectWeakMap = WeakMap<JSObject*, JSObject*>;
-
// Get the hash from the Symbol.
HashNumber GetSymbolHash(JS::Symbol* sym);
diff --git a/js/src/jsapi-tests/testGCGrayMarking.cpp b/js/src/jsapi-tests/testGCGrayMarking.cpp
@@ -26,8 +26,9 @@ static constexpr CellColor MarkedCellColors[] = {CellColor::Gray,
namespace js {
-struct GCManagedObjectWeakMap : public ObjectWeakMap {
- using ObjectWeakMap::ObjectWeakMap;
+struct GCManagedObjectWeakMap : public WeakMap<JSObject*, JSObject*> {
+ using Base = WeakMap<JSObject*, JSObject*>;
+ using Base::Base;
};
} // namespace js
diff --git a/js/src/jsfriendapi.cpp b/js/src/jsfriendapi.cpp
@@ -741,7 +741,8 @@ JS_PUBLIC_API void js::SetAllocationMetadataBuilder(
}
JS_PUBLIC_API JSObject* js::GetAllocationMetadata(JSObject* obj) {
- ObjectWeakMap* map = ObjectRealm::get(obj).objectMetadataTable.get();
+ ObjectRealm::ObjectMetadataTable* map =
+ ObjectRealm::get(obj).objectMetadataTable.get();
if (map) {
auto ptr = map->lookup(obj);
if (ptr) {
diff --git a/js/src/vm/EnvironmentObject.h b/js/src/vm/EnvironmentObject.h
@@ -1470,7 +1470,8 @@ class DebugEnvironments {
Zone* zone_;
/* The map from (non-debug) environments to debug environments. */
- ObjectWeakMap proxiedEnvs;
+ using ProxiedEnvironmentsMap = WeakMap<JSObject*, JSObject*>;
+ ProxiedEnvironmentsMap proxiedEnvs;
/*
* The map from live frames which have optimized-away environments to the
diff --git a/js/src/vm/Realm.cpp b/js/src/vm/Realm.cpp
@@ -148,7 +148,7 @@ ObjectRealm::getOrCreateNonSyntacticLexicalEnvironment(JSContext* cx,
MOZ_ASSERT(&ObjectRealm::get(enclosing) == this);
if (!nonSyntacticLexicalEnvironments_) {
- auto map = cx->make_unique<ObjectWeakMap>(cx);
+ auto map = cx->make_unique<NonSyntacticLexialEnvironmentsMap>(cx);
if (!map) {
return nullptr;
}
@@ -419,7 +419,7 @@ void Realm::setNewObjectMetadata(JSContext* cx, HandleObject obj) {
cx->check(metadata);
if (!objects_.objectMetadataTable) {
- auto table = cx->make_unique<ObjectWeakMap>(cx);
+ auto table = cx->make_unique<ObjectRealm::ObjectMetadataTable>(cx);
if (!table) {
oomUnsafe.crash("setNewObjectMetadata");
}
diff --git a/js/src/vm/Realm.h b/js/src/vm/Realm.h
@@ -240,7 +240,9 @@ class ObjectRealm {
// All non-syntactic lexical environments in the realm. These are kept in a
// map because when loading scripts into a non-syntactic environment, we
// need to use the same lexical environment to persist lexical bindings.
- js::UniquePtr<js::ObjectWeakMap> nonSyntacticLexicalEnvironments_;
+ using NonSyntacticLexialEnvironmentsMap = WeakMap<JSObject*, JSObject*>;
+ js::UniquePtr<NonSyntacticLexialEnvironmentsMap>
+ nonSyntacticLexicalEnvironments_;
ObjectRealm(const ObjectRealm&) = delete;
void operator=(const ObjectRealm&) = delete;
@@ -251,7 +253,8 @@ class ObjectRealm {
// Keep track of the metadata objects which can be associated with each JS
// object. Both keys and values are in this realm.
- js::UniquePtr<js::ObjectWeakMap> objectMetadataTable;
+ using ObjectMetadataTable = WeakMap<JSObject*, JSObject*>;
+ js::UniquePtr<ObjectMetadataTable> objectMetadataTable;
using IteratorCache =
js::HashSet<js::PropertyIteratorObject*, js::IteratorHashPolicy,
