tc39 / ecma262

Status, process, and documents for ECMA-262
https://tc39.es/ecma262/
Other
14.85k stars 1.27k forks source link

Editorial: Change some `X is Y` to `SameValue(X, Y) is *true*` #3340

Closed jmdyck closed 6 days ago

jmdyck commented 1 month ago

(apply editorial convention to recently-merged PR #3337)

gibson042 commented 1 month ago

@jmdyck There's another instance in sec-weakref-execution:

           1. For each FinalizationRegistry _fg_ such that _fg_.[[Cells]] contains a Record _cell_ such that _cell_.[[WeakRefTarget]] is _value_, do
             1. Set _cell_.[[WeakRefTarget]] to ~empty~.
             1. Optionally, perform HostEnqueueFinalizationRegistryCleanupJob(_fg_).
-          1. For each WeakMap _map_ such that _map_.[[WeakMapData]] contains a Record _r_ such that _r_.[[Key]] is _value_, do
+          1. For each WeakMap _map_ such that _map_.[[WeakMapData]] contains a Record _r_ such that _r_.[[Key]] is not ~empty~ and SameValue(_r_.[[Key]], _value_) is *true*, do
             1. Set _r_.[[Key]] to ~empty~.
             1. Set _r_.[[Value]] to ~empty~.
           1. For each WeakSet _set_ such that _set_.[[WeakSetData]] contains _value_, do
jmdyck commented 1 month ago

@jmdyck There's another instance in sec-weakref-execution:

In more detail (for those who are wondering):

I.e., replace

_r_.[[Key]] is _value_

with

_r_.[[Key]] is not ~empty~ and SameValue(_r_.[[Key]], _value_)

as you suggest.

(I think the same reasoning applies to the is comparisons on steps 1.a and 1.b of that algorithm.)

While this is a valid replacement, I'm not sure it's an improvement. I'll put it in if the editors want it.

syg commented 3 weeks ago

While this is a valid replacement, I'm not sure it's an improvement. I'll put it in if the editors want it.

I'm leaning towards leaving the WeakRef execution stuff as is since explictly using is is more evocative (to me) of comparing identity, which is fitting for WeakRefs.