Editorial: Refactor IsLooselyEqual for better symmetry

[gibson042]

...taking advantage of type mismatch after the first step

Ref #3911 (comment)

[ljharb]

this is pretty confusing imo - you're calling "to bigint or undefined" when the only possible outcome is "to bigint"

[gibson042]

No, it really is bigint or undefined here—either x is a bigint and y is a string or x is a string and y is a bigint... whichever is a bigint is preserved, but the string becomes either a bigint or undefined (the former in e.g. 0n == "0", the latter in e.g. 0n == "0.0").

[ljharb]

the if condition allows for both being strings or both being bigints, but i guess if the types were the same it'd have returned already.

i still find this confusing, and i suspect inlining the logic would be much easier to read/understand.

[gibson042]

the if condition allows for both being strings or both being bigints, but i guess if the types were the same it'd have returned already.

Yes, this is the key insight being leveraged here.

i still find this confusing, and i suspect inlining the logic would be much easier to read/understand.

I'm assuming that would entail splitting the steps, e.g.

-        1. If _x_ is either a BigInt or a String and _y_ is either a BigInt or a String, then
-          1. Return IsStrictlyEqual(ToBigIntOrUndefined(_x_), ToBigIntOrUndefined(_y_)).
+        1. If _x_ is a BigInt and _y_ is a String, return IsStrictlyEqual(_x_, ToBigIntOrUndefined(_y_)).
+        1. If _x_ is a String and _y_ is a BigInt, return IsStrictlyEqual(ToBigIntOrUndefined(_x_), _y_).

...and presumably also for the preceding Number vs. String case and its use of ToNumber?

But how are those different from the BigInt vs. Number case and its use of ℝ? The particular ToNumber calls map {number, string} → number and ℝ calls map {bigint, [finite] number} → mathematical value—is it just the {bigint, string} → {bigint, undefined} output of ToBigIntOrUndefined?

[ljharb]

i'm suggesting that "tobigint or undefined" is inherently confusing, because it's using undefined as a sentinel value.

I'd probably prefer to see these steps be something verbose that converts the string to a bigint, and then compares the two.

[ljharb]

a boolean from a predicate isn't a special sentinel value, nor is an exception a value at all, let alone a sentinel.

That's a fair point about it being fallible - what would the steps look like in that case?

[gibson042]

Fundamentally, it all comes down to ParseText output being a polymorphic union over {Parse Node, a List of errors}, which StringToBigInt (and therefore transitively also this proposed ToBigIntOrUndefined) maps into a union over {BigInt, undefined}. You can call that a sentinel value, but the types are disjoint, which I would argue is more accurately described as an option/maybe type. There's no getting around some kind of polymorphism for a fallible operation, the only question is what form it takes, and it's not uncommon for us to use undefined like this.

[ljharb]

Right, but the purpose here is "since one's a string and one's a bigint, coerce the string one to a bigint, and then compare the resulting two bigints", and it feels like unnecessary indirection to compare a string to undefined. You could replace the undefined with any non-bigint type and it'd work the same, i believe, which suggests to me that there's a cleaner way to express it.

Maybe something like:

1. If _x_ is a bigint and _y_ is a string, or _x_ is a string and _y_ is a bigint, then
  1. if _x_ is a string, then
    1. set _str_ to _x_.
    1. set _big_ to _y_.
  1. else,
    1. set _str_ to _y_.
    1. set _big_ to _x_.
  1. Set _num_ to StringToBigInt(_x_).
  1. if _num_ is *false*, return *false*.
  1. return IsLooselyEqual(_num_, _big_).

then it's very explicit and doesn't need a new sentinel-returning AO (obv with some kind of spec ternary it'd be much cleaner)

[gibson042]

Those steps accomplish the necessary behavior, but IMO would actually reduce comprehensibility of IsLooselyEqual, which has many branches for [order-independent] special type pairs.

• SameType → IsStrictlyEqual
• (undefined, null) → true
• [normative-optional] (document.all, undefined | null) → true
• (false | true, *) → IsLooselyEqual(ToNumber(false | true), *)
• (Object, String | Number | BigInt | Symbol) → IsLooselyEqual(ToPrimitive(Object), String | Number | BigInt | Symbol)
• (String, Number) → IsStrictlyEqual(ToNumber(String), Number)
• (String, BigInt) → IsStrictlyEqual(StringToBigInt(String), BigInt)
• (BigInt, Number) → ℝ(BigInt) = ℝ(Number)

I think there's even another possible approach that replaces recursion with binding reassignments:

1. If _x_ is either *undefined* or *null*, or _y_ is either *undefined* or *null*, then
  1. [normative-optional] If the host is a web browser or otherwise supports The [[IsHTMLDDA]] Internal Slot, then
    1. If _x_ is an Object and _x_ has an [[IsHTMLDDA]] internal slot, set _x_ to *undefined*.
    1. If _y_ is an Object and _y_ has an [[IsHTMLDDA]] internal slot, set _y_ to *undefined*.
  1. If _x_ is either *undefined* or *null* and _y_ is either *undefined* or *null*, return *true*.
  1. Return *false*.
1. If _x_ is an Object and _y_ is not an Object, set _x_ to ? ToPrimitive(_x_).
1. If _y_ is an Object and _x_ is not an Object, set _y_ to ? ToPrimitive(_y_).
1. If SameType(_x_, _y_) is *true*, return IsStrictlyEqual(_x_, _y_).
1. If _x_ is a Boolean, set _x_ to ! ToNumber(_x_).
1. If _y_ is a Boolean, set _y_ to ! ToNumber(_y_).
1. NOTE: Either _x_ and _y_ are both Numbers, or their types are different.
1. If _x_ is either a Number or a String and _y_ is either a Number or a String, then
  1. Return IsStrictlyEqual(! ToNumber(_x_), ! ToNumber(_y_)).
1. If _x_ is either a BigInt or a String and _y_ is either a BigInt or a String, then
  1. Return IsStrictlyEqual(ToBigIntOrUndefined(_x_), ToBigIntOrUndefined(_y_)).
1. If _x_ is either a BigInt or a finite Number and _y_ is either a BigInt or a finite Number, then
  1. If ℝ(_x_) = ℝ(_y_), return *true*.
1. Return *false*.

[ljharb]

that seems like the winner so far

[ljharb]

i normally love more small AOs but in this case i think inlining the table directly into ToBigInt would be clearer.

[gibson042]

I had the same thought.

[gibson042]

This case is poorly covered by Editorial Conventions: Comparisons. We could avoid it by splitting into two steps, but even if so it'd be worth establishing a convention.

[jmdyck]

Well, it says (modulo some elision):

when comparing for equality against multiple options,
consolidate is using one of for more than two options (e.g., _x_ is one of _option1_, _option2_, or _option3_)

so that would seem to cover this step, and agree with what you've got.

On the other hand, there's a bullet under Phrasing Conventions that begins "to create an untagged union of types". Under those rules, you'd say:

Suggested change

	1. If _primitive_ is one of *undefined*, *null*, a Number, or a Symbol, return *undefined*.
	1. If _primitive_ is either a Number, a Symbol, *undefined*, or *null*, return *undefined*.

The question is whether this context is "creating an untagged union of types". I'm not sure what was intended when it was added to Editorial Conventions, but it's a boiled-down version of #2972 (comment) and following, which tried to be fairly general in its application, explicitly including forms such as if _X_ is <disjunction>.

[gibson042]

I decided that this is "an untagged union of types" given current conventions, and opened #3937 to drive more clarity.

[github-actions]

The rendered spec preview for this PR is available as a single page at https://tc39.es/ecma262/pr/3918 and as multiple pages at https://tc39.es/ecma262/pr/3918/multipage .

[jmdyck]

Instead of this assertion, maybe _primitive_ should be declared as an ECMAScript language value, but not an Object, or just a primitive value.

[gibson042]

We don't have any current uses of that pattern AFAICT, but I like it. As of the latest push here, "primitive value" is now a <dfn>ed term.

[jmdyck]

This goes against the 'Terms and Definitions' convention (which I'm guessing was inherited from Ecma) that each term & its defining text should have the same part of speech. In this case, they should both be noun phrases, but you've changed it to a sentence.

Besides that, #1278 is going to happen eventually, so it's probably best not to add to T&D. Instead, just leave this as is, and instead add the <dfn> to the corresponding sentence in 4.3.

[gibson042]

This goes against the 'Terms and Definitions' convention (which I'm guessing was inherited from Ecma) that each term & its defining text should have the same part of speech. In this case, they should both be noun phrases, but you've changed it to a sentence.

Just like "implementation-approximated" and "implementation-defined" a few terms up.

Besides that, #1278 is going to happen eventually, so it's probably best not to add to T&D. Instead, just leave this as is, and instead add the <dfn> to the corresponding sentence in 4.3.

4.3 explicitly states "This overview is not part of the standard proper", which makes it seem like a bad home for such a <dfn>. I guess it eventually belongs in ECMAScript Language Types.

[jmdyck]

Just like "implementation-approximated" and "implementation-defined" a few terms up.

Yeah, those (and "host-defined") were added in PR #1951. Looks like I didn't comment on the wording anomaly then.

4.3 explicitly states "This overview is not part of the standard proper", which makes it seem like a bad home for such a .

Hm, didn't notice that. And yeah, there are no <dfn>s currently in 4.3. So agreed, bad suggestion. Should we explicitly mark 4.3 as "informative"?

I guess it eventually belongs in ECMAScript Language Types.

Yup.

[gibson042]

I just moved it now.