Editorial: refer to numeric methods as we do other AOs

[michaelficarra]

This leaves the :: in the names for now to make the diff a little easier. We can rename the AOs in a follow-up if we think it's worth it.

[michaelficarra]

I inlined and removed the ::unit constants, since they were described as the multiplicative identity, but only ever used in additive positions. Plus, it just makes sense to use 1 literally in the increment/decrement algorithms.

[michaelficarra]

@bakkot Comments addressed.

[bakkot]

I don't love having these duplicated - it would maybe be better to have rowspan=2?

[michaelficarra]

since the lightweight table doesn't have cell borders, that would render confusingly

[bakkot]

LGTM other than comments.

I think having these automatically link is well worth the slight overhead in amount of prose.

[michaelficarra]

@bakkot Comments addressed.

[jmdyck]

I think having these automatically link is well worth the slight overhead in amount of prose.

Is that the rationale for this PR? The initial comment didn't really say.

[bakkot]

The original motivation for the PR was to more precisely capture the return types for these operations - to talk sensibly about the return type of e.g. T::unsignedRightShift you have to say it can return either a normal completion of a Number or a throw completion of a TypeError, which obscures what's actually happening - namely, that Number::unsignedRightShift always returns a Number and never throws, and BigInt::unsignedRightShift always throws.

This matters more with #2547, which is where the issue came up.

However, separately, I think having these automatically link is enough motivation on its own.

[jmdyck]

The original motivation for the PR was to more precisely capture the return types for these operations - to talk sensibly about the return type of e.g. T::unsignedRightShift you have to say it can return either a normal completion of a Number or a throw completion of a TypeError, which obscures what's actually happening - namely, that Number::unsignedRightShift always returns a Number and never throws, and BigInt::unsignedRightShift always throws.

Okay, but Number::uRS and BigInt::uRS are defined separately, so (with tc39/ecmarkup#363) we can precisely convey each one's return type. Wouldn't that be enough to address the original motivation?

This matters more with #2547, which is where the issue came up.

That one says "not ready for review", so I'm trying not to look at it.

However, separately, I think having these automatically link is enough motivation on its own.

Yup, could be.

[bakkot]

Okay, but Number::uRS and BigInt::uRS are defined separately, so (with tc39/ecmarkup#363) we can precisely convey each one's return type.

Yes, but you have to know which you're calling for that to be useful at the call site. Otherwise you end up with exactly the same lack of precision - you don't know if your value is a completion record or not. Hence this PR.

[jmdyck]

Okay, but Number::uRS and BigInt::uRS are defined separately, so (with tc39/ecmarkup#363) we can precisely convey each one's return type.

Yes, but you have to know which you're calling for that to be useful at the call site. Otherwise you end up with exactly the same lack of precision - you don't know if your value is a completion record or not.

Right! But if you don't know which you're calling in the status quo, how do you think this PR improves that?

E.g., if the status quo has Type(x)::uRS(x,y) and this PR splits that into (roughly)
if x is a Number then Number::uRS(x,y) else if x is a BigInt then BigInt::uRS(x,y)
then sure, you know that the type of Number::uRS(x,y) is Number and the type of BigInt::uRS(x,y) is throw(TypeError), but the type of the whole expression is still exactly the same as for Type(x)::uRS(x,y).

[ljharb]

Sure, but then you don't have a mixed return type at an individual callsite.

[michaelficarra]

In particular, mixed between a completion record and a non completion record.

[jmdyck]

Sure, but then you don't have a mixed return type at an individual callsite.

Well, you don't have a mixed return type at the callsites of Number::uRS and BigInt::uRS, but you do still have a mixed return type at the callsites of ApplyStringOrNumericBinaryOperator.

Looking at this example more realistically, this PR doesn't actually unfold/inline any invocation of ::unsignedRightShift. The only invocation is in ApplyStringOrNumericBinaryOperator, where all the numeric operations are invoked via ? _operation_(_lnum_, _rnum_). This PR doesn't avoid a mixed return type at that callsite.

In particular, mixed between a completion record and a non completion record.

This PR unfolds calls to:

• _T_::equal
• _T_::sameValue
• _T_::sameValueZero

where the return type is Boolean regardless of _T_, and

• _T_::unaryMinus
• _T_::bitwiseNOT
• _T_::add
• _T_::subtract
• _T_::lessThan

where the result type does depend on _T_, but never involves an abrupt completion.

So I don't think this PR changes any callsite where the return type mixes a completion record and a non completion record.

[bakkot]

@jmdyck Oh, right. The plan is for #2547 to modify ApplyStringOrNumericBinaryOperator to distinguish between the throwing and non-throwing AOs, like so, but I agree it doesn't affect any callsites in that way right now.

[jmdyck]

The plan is for #2547 to modify ApplyStringOrNumericBinaryOperator to distinguish between the throwing and non-throwing AOs, like so, but I agree it doesn't affect any callsites in that way right now.

Hm, interesting.

Still, that doesn't explain why this PR unfolds _T_::foo() when the return type isn't 'mixed', and in some cases isn't even dependent on _T_.

[bakkot]

If we're not going to use _T_::foo() consistently, it's simpler to just not have such a mechanism at all, so that readers don't need to learn what that syntax means.

[jmdyck]

And we're not going to use it consistently because in some cases it has a 'mixed' return type?

[bakkot]

Yup.

[jmdyck]

Okay. I'm not sure I agree it's an improvement, but at least the rationale is clearer now.