Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Editorial: Recast comparison algorithms as proper abstract operations #2378

Merged
merged 4 commits into from
Jun 12, 2021

Conversation

jmdyck
Copy link
Collaborator

@jmdyck jmdyck commented Apr 9, 2021

Specifically:

  • Abstract Relational Comparison
  • Abstract Equality Comparison
  • Strict Equality Comparison

See #545 (comment) (third bullet).

Downstream: HTML refers to Abstract Equality Comparison and Strict Equality Comparison. (But the link will still work.)

@jmdyck jmdyck changed the title Editiorial: Recast comparison algorithms as proper abstract operations Editorial: Recast comparison algorithms as proper abstract operations Apr 9, 2021
spec.html Outdated
<p>The comparison _x_ &lt; _y_, where _x_ and _y_ are values, produces *true*, *false*, or *undefined* (which indicates that at least one operand is *NaN*). In addition to _x_ and _y_ the algorithm takes a Boolean flag named _LeftFirst_ as a parameter. The flag is used to control the order in which operations with potentially visible side-effects are performed upon _x_ and _y_. It is necessary because ECMAScript specifies left to right evaluation of expressions. The default value of _LeftFirst_ is *true* and indicates that the _x_ parameter corresponds to an expression that occurs to the left of the _y_ parameter's corresponding expression. If _LeftFirst_ is *false*, the reverse is the case and operations must be performed upon _y_ before _x_. Such a comparison is performed as follows:</p>
<emu-clause id="sec-abstract-relational-comparison" aoid="AbstractRelationalComparison">
<h1>AbstractRelationalComparison ( _x_, _y_ [ , _LeftFirst_ ] )</h1>
<p>The abstract operation AbstractRelationalComparison takes arguments _x_ (an ECMAScript language value) and _y_ (an ECMAScript language value), and optional argument _LeftFirst_ (a Boolean). It provides the semantics for the comparison _x_ &lt; _y_, returning *true*, *false*, or *undefined* (which indicates that at least one operand is *NaN*). The _LeftFirst_ flag is used to control the order in which operations with potentially visible side-effects are performed upon _x_ and _y_. It is necessary because ECMAScript specifies left to right evaluation of expressions. The default value of _LeftFirst_ is *true* and indicates that the _x_ parameter corresponds to an expression that occurs to the left of the _y_ parameter's corresponding expression. If _LeftFirst_ is *false*, the reverse is the case and operations must be performed upon _y_ before _x_. It performs the following steps when called:</p>
<emu-alg>
1. If the _LeftFirst_ flag is *true*, then
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't realize we ever defaulted parameters by prose in the preamble. That seems not great (although unrelated to this change).

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm guessing you mean: state a default in the preamble without 'implementing' that default via a presence-check in the algorithm. If so: yes, not great.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We do that in a number of places, yes (see #2379, for example)

Copy link
Collaborator Author

@jmdyck jmdyck Apr 9, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did a quick check though non-functions, and the only other case I found was that OrdinaryCreateFromConstructor's algorithm doesn't provide a default for optional _internalSlotsList_. (i.e., the case addressed by #2379).

spec.html Outdated
<h1>Abstract Relational Comparison</h1>
<p>The comparison _x_ &lt; _y_, where _x_ and _y_ are values, produces *true*, *false*, or *undefined* (which indicates that at least one operand is *NaN*). In addition to _x_ and _y_ the algorithm takes a Boolean flag named _LeftFirst_ as a parameter. The flag is used to control the order in which operations with potentially visible side-effects are performed upon _x_ and _y_. It is necessary because ECMAScript specifies left to right evaluation of expressions. The default value of _LeftFirst_ is *true* and indicates that the _x_ parameter corresponds to an expression that occurs to the left of the _y_ parameter's corresponding expression. If _LeftFirst_ is *false*, the reverse is the case and operations must be performed upon _y_ before _x_. Such a comparison is performed as follows:</p>
<emu-clause id="sec-abstract-relational-comparison" aoid="AbstractRelationalComparison">
<h1>AbstractRelationalComparison ( _x_, _y_ [ , _LeftFirst_ ] )</h1>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thoughts on calling this AbstractLessThanComparison, just so it's clear what the order is? (Or some other such name.)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When this came up in IRC, there was some sentiment for keeping the names the same so as not to invalidate books and such. But if that's off the table, then yes, I think we can come up with better names.

Thoughts on calling this AbstractLessThanComparison, just so it's clear what the order is?

That would be an improvement. I think "LessThan" implies "Comparison", so maybe just AbstractLessThan. Further, Abstract doesn't seem right: abstract as opposed to what? So maybe just LessThan. Or maybe GeneralLessThan if you want to convey that it accepts any two ECMAScript language values.

For {Abstract,Strict}EqualityComparison: again, the "Comparison" seems redundant. And again, "Abstract" doesn't seem right; to contrast with "Strict", I think "Loose" would be better. So maybe {Loose,Strict}Equality or {Loose,Strict}Equals.

Copy link
Contributor

@bakkot bakkot Apr 9, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do think there's some value in keeping the existing names. It's just that for this one, in particular, the fact that the calling convention will no longer mention < will make it ambiguous in a way that "StrictEqualityComparison" is not.

Still, I'm open to renaming the others as well, I guess.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we do rename them, I vote for isStrictlyEqual / isLooselyEqual or isAbstractlyEqual, or similar (ie, predicate naming)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IsLessThan, IsStrictlyEqual, and IsRoughlyEqual would be my preference, but I don't have strong feelings here.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like @ljharb's suggestion of "loosely" better than "roughly", I think, because "roughly" makes me think about floating point epsilons rather than "not strict".

Anyway, those sound ok to me.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My preference is IsLessThan, IsStrictlyEqual, IsLooselyEqual.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is "loose" a common term used to refer to == comparisons?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, it's what MDN uses, as well as this popular book. And this book also says "the == operator, generally referred to as the 'loose equality' operator". So, pretty common, I think.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like "loose" is about as common as "strict": googling "javascript loose equality" gets me about 762,000 results, compared to about 790,000 for "javascript strict equality".

spec.html Outdated Show resolved Hide resolved
Copy link
Member

@michaelficarra michaelficarra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. I'm okay with whatever names are chosen.

spec.html Outdated
<h1>Abstract Relational Comparison</h1>
<p>The comparison _x_ &lt; _y_, where _x_ and _y_ are values, produces *true*, *false*, or *undefined* (which indicates that at least one operand is *NaN*). In addition to _x_ and _y_ the algorithm takes a Boolean flag named _LeftFirst_ as a parameter. The flag is used to control the order in which operations with potentially visible side-effects are performed upon _x_ and _y_. It is necessary because ECMAScript specifies left to right evaluation of expressions. The default value of _LeftFirst_ is *true* and indicates that the _x_ parameter corresponds to an expression that occurs to the left of the _y_ parameter's corresponding expression. If _LeftFirst_ is *false*, the reverse is the case and operations must be performed upon _y_ before _x_. Such a comparison is performed as follows:</p>
<emu-clause id="sec-abstract-relational-comparison" aoid="AbstractRelationalComparison">
<h1>AbstractRelationalComparison ( _x_, _y_ [ , _LeftFirst_ ] )</h1>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My preference is IsLessThan, IsStrictlyEqual, IsLooselyEqual.

@jmdyck jmdyck force-pushed the comparison_as_ao branch from 050b1d7 to a035e47 Compare June 10, 2021 04:13
@jmdyck
Copy link
Collaborator Author

jmdyck commented Jun 10, 2021

force-pushed to:

  • rebase to master
  • fold in the "StrictEqualityComparison fixup" commit
  • rename:
    • Abstract Relational Comparison -> IsLessThan
    • Abstract Equality Comparison -> IsLooselyEqual
    • Strict Equality Comparison -> IsStrictlyEqual

Also, I added a commit to make the LeftFirst parameter of IsLessThan non-optional, to address bakkot's comment above.

@bakkot bakkot added ready to merge Editors believe this PR needs no further reviews, and is ready to land. and removed editor call to be discussed in the next editor call labels Jun 11, 2021
@ljharb ljharb force-pushed the comparison_as_ao branch from a035e47 to c7d208f Compare June 12, 2021 04:47
@ljharb ljharb merged commit c7d208f into tc39:master Jun 12, 2021
@jmdyck jmdyck deleted the comparison_as_ao branch June 12, 2021 18:09
mathiasbynens pushed a commit to mathiasbynens/ecma262 that referenced this pull request Oct 18, 2021
... as a proper abstract operation, named IsLessThan.
mathiasbynens pushed a commit to mathiasbynens/ecma262 that referenced this pull request Oct 18, 2021
... as a proper abstract operation, named IsLooselyEqual.
mathiasbynens pushed a commit to mathiasbynens/ecma262 that referenced this pull request Oct 18, 2021
... as a proper abstract operation, named IsStrictlyEqual.
mathiasbynens pushed a commit to mathiasbynens/ecma262 that referenced this pull request Oct 18, 2021
jmdyck added a commit to jmdyck/html that referenced this pull request Oct 19, 2022
tc39/ecma262#2378 did some renaming:

The Abstract Equality Comparison algorithm
became the IsLooselyEqual abstract operation.

The Strict Equality Comparison algorithm
became the IsStrictlyEqual abstract operation.
domenic pushed a commit to whatwg/html that referenced this pull request Oct 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
editorial change ready to merge Editors believe this PR needs no further reviews, and is ready to land.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants