Rename js::ObjectWeakMap js::ObjectMetadataMap

ASSIGNED
Assigned to

Status

()

Core
JavaScript Engine
ASSIGNED
2 years ago
2 years ago

People

(Reporter: jimb, Assigned: jimb)

Tracking

(Blocks: 1 bug)

unspecified
Points:
---
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

2 years ago
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.
(Assignee)

Comment 1

2 years ago
Created attachment 8723946 [details] [diff] [review]
Rename ObjectWeakMap to ObjectMetadataMap.
Assignee: nobody → jimb
Status: NEW → ASSIGNED
Attachment #8723946 - Flags: review?(terrence)
(Assignee)

Updated

2 years ago
Blocks: 1178505
(Assignee)

Comment 2

2 years ago
This patch should have no visible effect on behavior, and doesn't significantly change the logic, so it has no tests.
Flags: in-testsuite-
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.
Attachment #8723946 - Flags: review?(terrence) → review+
(Assignee)

Comment 4

2 years ago
(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?
You need to log in before you can comment on or make changes to this bug.