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.

Sign up for GitHub

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

Jump to bottom

Editorial: clarify spec values that have identity and its implications #2821

Merged
merged 1 commit into from
Sep 1, 2022
Merged

Editorial: clarify spec values that have identity and its implications #2821

merged 1 commit into from
Sep 1, 2022

Conversation

michaelficarra
Copy link
Member

@michaelficarra michaelficarra commented Jul 8, 2022 •
edited
Loading

Opened as draft since I only have the prose for now. Once we get this part agreed on and settled, I'll audit for uses of comparison, mutation, article choice, construction, etc.

New section rendering: https://ci.tc39.es/preview/tc39/ecma262/pull/2821#sec-identity

Fixes #2405.

@michaelficarra michaelficarra requested a review from a team July 8, 2022 23:59
@michaelficarra michaelficarra added editorial change editor call to be discussed in the next editor call labels Jul 8, 2022
@michaelficarra michaelficarra marked this pull request as draft July 9, 2022 00:09
bergus
bergus reviewed Jul 9, 2022
View reviewed changes
spec.html

<emu-clause id="sec-identity">
<h1>Identity</h1>
<p>In this specification, the word &ldquo;is&rdquo; is used to identify two values, as in &ldquo;If _bool_ is *true*, then&rdquo;. This identification is different when applied to values which have identity than it is when applied to those without identity.</p>
Copy link

@bergus bergus Jul 9, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

I would maybe suggest to use two different wordings: "is" or even "equals" for values (without identity), "is same as" for objects (values with identity).
Does the spec actually compare objects that often? I can only think of SameValueNonNumeric.

spec.html
</emu-clause>

<emu-clause id="sec-identity">
<h1>Identity</h1>
Copy link

Choose a reason for hiding this comment

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

I wonder if this whole section should be part of the ECMAScript Data Types and Values chapter, not the Algorithm Contentions one - it's describing semantics of objects, not just a notational convention.

@michaelficarra michaelficarra mentioned this pull request Jul 11, 2022
Merged
syg
syg reviewed Jul 13, 2022
View reviewed changes
@michaelficarra
Copy link
Member Author

Updated based on @syg's feedback. @tc39/ecma262-editors Please take another look.

syg
syg reviewed Aug 10, 2022
View reviewed changes
@michaelficarra michaelficarra removed the editor call to be discussed in the next editor call label Aug 10, 2022
@michaelficarra michaelficarra mentioned this pull request Aug 10, 2022
Merged
@michaelficarra
Copy link
Member Author

Thoughts on "characteristics" as an alternative to "qualities"?

bakkot
bakkot reviewed Aug 23, 2022
View reviewed changes
@arhadthedev

This comment was marked as resolved.

Sign in to view
@bakkot
Copy link
Contributor

bakkot commented Aug 23, 2022

Are all values with identity guaranteed to be Records?

No. The spec-internal List type and the ES language Object type both have identity, and neither are Records, for example.

@michaelficarra michaelficarra added the editor call to be discussed in the next editor call label Aug 23, 2022
bakkot
bakkot reviewed Aug 24, 2022
View reviewed changes
Copy link
Contributor

@bakkot bakkot left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Some of these uses of "any of" do not read naturally to me. I've commented on a few representative cases but there's several others.

bakkot
bakkot reviewed Aug 24, 2022
View reviewed changes
spec.html Outdated
</emu-alg>
<emu-grammar>ExportDeclaration : `export` `default` ClassDeclaration</emu-grammar>
<emu-alg>
1. Let _names_ be BoundNames of |ClassDeclaration|.
1. Let _localName_ be the sole element of _names_.
1. Return a List whose sole element is the ExportEntry Record { [[ModuleRequest]]: *null*, [[ImportName]]: *null*, [[LocalName]]: _localName_, [[ExportName]]: *"default"* }.
1. Return a List whose sole element is an ExportEntry Record { [[ModuleRequest]]: *null*, [[ImportName]]: *null*, [[LocalName]]: _localName_, [[ExportName]]: *"default"* }.
Copy link
Contributor

Choose a reason for hiding this comment

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

I am ok with this but would mildly prefer to factor the record out to an alias, so we could say Let _entry_ be a new ExportEntry Record { ... }

Copy link
Member Author

Choose a reason for hiding this comment

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

Agreed. This was kind of an intermediate step toward handling articles. Remember, this is still in draft!

syg
syg reviewed Aug 24, 2022
View reviewed changes
Copy link
Contributor

@syg syg left a comment

Choose a reason for hiding this comment

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

Shouldn't the lists of kinds of values that have/do not have identity be exhaustive?

@syg
Copy link
Contributor

syg commented Aug 24, 2022

@michaelficarra All of the editorial choices in #2821 (comment) seem to be strictly speaking unrelated to nailing down a definition of identity. It'd be easier to review some of those as separate PRs.

@michaelficarra michaelficarra removed the editor call to be discussed in the next editor call label Aug 24, 2022
@michaelficarra michaelficarra force-pushed the identity branch 2 times, most recently from 5e966e8 to d0b7483 Compare August 30, 2022 15:44
@michaelficarra michaelficarra marked this pull request as ready for review August 30, 2022 15:46
@michaelficarra
Copy link
Member Author

Dropped all the editorial changes I've been working on, cleaning up how comparisons are done. Now this just includes the identity section. I'll open up other PRs for the comparison changes. Marked as ready for review.

@michaelficarra michaelficarra mentioned this pull request Aug 30, 2022
Merged
@michaelficarra michaelficarra added the editor call to be discussed in the next editor call label Aug 31, 2022
@michaelficarra michaelficarra 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 Aug 31, 2022
syg
syg approved these changes Sep 1, 2022
View reviewed changes
Copy link
Contributor

@syg syg left a comment

Choose a reason for hiding this comment

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

This is great. lgtm!

@ljharb ljharb changed the title fixes #2405: clarify spec values that have identity and its implications Editorial: clarify spec values that have identity and its implications Sep 1, 2022
@ljharb ljharb merged commit ff191ce into main Sep 1, 2022
@ljharb ljharb deleted the identity branch September 1, 2022 17:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ljharb ljharb ljharb left review comments

@bergus bergus bergus left review comments

@bathos bathos bathos left review comments

@syg syg syg approved these changes

@bakkot bakkot bakkot approved these changes

Assignees
No one assigned
Labels
editorial change establishes editorial conventions ready to merge Editors believe this PR needs no further reviews, and is ready to land.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

clarify which spec values do and do not carry identity
7 participants
@michaelficarra @arhadthedev @bakkot @syg @ljharb @bergus @bathos