1251528 - Rename js::ObjectWeakMap js::ObjectMetadataMap

Status: ASSIGNED

(Core :: JavaScript Engine, defect)

Description

[Jim Blandy :jimb]

The js::ObjectWeakMap type is generally used to store metadata about objects, and the present name is easily confused with js::WeakMapObject, which is something entirely different. The name js::ObjectMetadataMap will also fit nicely with the upcoming js::StringMetadataMap type.

Comment 1

[Jim Blandy :jimb]

Attachment: Rename ObjectWeakMap to ObjectMetadataMap.

Comment 2

[Jim Blandy :jimb]

This patch should have no visible effect on behavior, and doesn't significantly change the logic, so it has no tests.

Comment 3

[Terrence Cole [:terrence]]

Comment on attachment 8723946 [details] [diff] [review]
Rename ObjectWeakMap to ObjectMetadataMap.

Review of attachment 8723946 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/jsweakmap.h
@@ +387,5 @@
>  
> +// A simple map for storing metadata about JSObjects, where the metadata itself
> +// is a JSObject. This is appropriate for metadata that is usually absent. This
> +// is just a thin wrapper around ObjectValueMap, with a more focused interface.
> +class ObjectMetadataMap

This is orthogonal to WeakMap and is only concerned with our metadata implementation; I think it should move to jscompartment.{cpp,h} with the usages.

Comment 4

[Jim Blandy :jimb]

(In reply to Terrence Cole [:terrence] from comment #3)
> This is orthogonal to WeakMap and is only concerned with our metadata
> implementation; I think it should move to jscompartment.{cpp,h} with the
> usages.

Did you know that ObjectWeapMap is also used in vm/ScopeObject.h and builtin/TypedObject.cpp? In that light,
1) Do you still approve of renaming it ObjectMetadataMap?
2) Do you still think it should be moved to jscompartment.cpp?