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: remove legacy intrinsics #2056

Merged
merged 1 commit into from
Sep 2, 2020
Merged

Conversation

ljharb
Copy link
Member

@ljharb ljharb commented Jun 19, 2020

@ljharb ljharb requested review from syg, michaelficarra, bakkot and a team June 19, 2020 04:32
Copy link
Collaborator

@jmdyck jmdyck left a comment

Choose a reason for hiding this comment

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

The Well-Known Intrinsics clause has (what might be read as) two definitions of "well-known intrinsics":

  • built-in objects that are explicitly referenced by the algorithms of this specification, and
  • the objects listed in the table "Well-Known Intrinsic Objects".

Currently, these definitions are more or less in agreement, but with this PR, they very much disagree.

E.g., is %ArrayBuffer.prototype% a well-known intrinsic? It's explicitly referenced in AllocateArrayBuffer, but it doesn't appear in the table.

What about %AsyncFunction%? It's in the table, but isn't explicitly referenced by any algorithm. (Several implicit references though.)

It's possible that the first 'definition' isn't meant to be a definition, but is just a statement of something that is true of well-known intrinsics, but is also true of other intrinsics. In which case, it's not a very helpful statement.

Either way, this PR should change some prose in the Well-Known Intrinsics clause.

spec.html Show resolved Hide resolved
spec.html Show resolved Hide resolved
spec.html Show resolved Hide resolved
spec.html Outdated
@@ -26488,7 +25947,6 @@ <h1>Boolean.prototype</h1>
<h1>Properties of the Boolean Prototype Object</h1>
<p>The Boolean prototype object:</p>
<ul>
<li>is the intrinsic object <dfn>%BooleanPrototype%</dfn>.</li>
Copy link
Collaborator

Choose a reason for hiding this comment

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

With this line removed, there's no explicit statement that the phrases "the Boolean Prototype Object" and "%Boolean.prototype%" denote the same object. While this equivalence might seem obvious given the similar wording, I don't think it should be left implicit.

Some possible solutions:

  • Just reinstate the line.
  • Leave the line out, and change:
    <p>The Boolean prototype object:</p>
    to something like:
    <p>The Boolean prototype object (i.e., %Boolean.prototype%):</p>
    or
    <p>%Boolean.prototype% (the Boolean prototype object):</p>
    (or maybe change the clause heading instead).
  • Leave the line out, and replace all occurrences of "the Boolean Prototype Object" with "%Boolean.prototype%". (This would probably be clearer as a separate PR, and might also address the equivalence of "the Foo constructor" and "%Foo%", which is not touched by this PR.)

(Ditto all of the above for the other similar removals.)

Copy link
Member Author

Choose a reason for hiding this comment

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

How is it implicit at all? It's the object on Boolean.prototype, Boolean is %Boolean%.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Sorry, when you say:

It's the object on Boolean.prototype

you mean that the phrase "the Boolean prototype object" denotes (the initial value of) Boolean.prototype, right?

I agree that that's the intent, but this PR doesn't say so. I'm not saying people won't figure it out, I'm saying there's no reason it should be something they need to figure out.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm saying that %Boolean.prototype% is already in the spec as "the original value of the prototype property on %Boolean%" - so perhaps just the sentence here needs to be updated, since it's currently a tautology?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm saying that %Boolean.prototype% is already in the spec as "the original value of the prototype property on %Boolean%"

Oh, okay. Well, I agree that that's in the spec, but I don't see how that answers my original point.

  • so perhaps just the sentence here needs to be updated, since it's currently a tautology?

I agree that's currently a tautology. How would you update it? Maybe just demote it to a Note?

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think it needs even a note; just making sure that "the Boolean Prototype object" is also noted somewhere as "the value of .prototype on %Boolean% should suffice?

Copy link
Collaborator

Choose a reason for hiding this comment

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

just making sure that "the Boolean Prototype object" is also noted somewhere as "the value of .prototype on %Boolean% should suffice?

Okay, I'm not sure how we got here, but yes, this aligns with two of the solutions I originally suggested.

And the third solution was to eliminate the phrase "the Boolean Prototype object". Does the spec benefit from introducing that phrase, rather than just saying "%Boolean.prototype%" (or Boolean.prototype, suitably qualified)?

@ljharb ljharb force-pushed the legacy-intrinsics branch 3 times, most recently from 350ca3a to 1dd2709 Compare June 19, 2020 16:48
@ljharb
Copy link
Member Author

ljharb commented Jun 19, 2020

built-in objects that are explicitly referenced by the algorithms of this specification

To me this includes every object in the entire spec, which includes, eg, %ArrayBuffer.prototype%. I agree the prose needs a bit of tweaking here, though, to avoid the ambiguity.

Copy link
Contributor

@ExE-Boss ExE-Boss left a comment

Choose a reason for hiding this comment

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

I think we should keep the <dfn>s for %*.prototype% to keep auto‑linking working.

See also:

spec.html Outdated
@@ -26008,7 +25473,6 @@ <h1>Object.values ( _O_ )</h1>
<h1>Properties of the Object Prototype Object</h1>
<p>The Object prototype object:</p>
<ul>
Copy link
Contributor

@ExE-Boss ExE-Boss Jun 24, 2020

Choose a reason for hiding this comment

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

Suggested change
<ul>
<ul>
<li>is the intrinsic object <dfn>%Object.prototype%</dfn>.</li>

Copy link
Contributor

Choose a reason for hiding this comment

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

When I posted #2056 (review), I thought that you’d add the <dfn>s for the remaining %*.prototype% properties without me needing to list them manually.

@Jack-Works
Copy link
Member

does the change make undeniable intrinsics deniable?

@ljharb
Copy link
Member Author

ljharb commented Jun 25, 2020

@Jack-Works this PR is purely editorial (no observable change) except for an impact on Function.prototype.toString (which makes this PR accidentally normative, which means it won't land soon). Deniability is unrelated.

@ExE-Boss
Copy link
Contributor

Well, the impact on Function.prototype.toString won’t be observable, as engines already include PropertyName when stringifing all named built‑in functions.

Also, I’m pretty sure that dotted intrinsics also count as well‑known intrinsic objects.

@ljharb
Copy link
Member Author

ljharb commented Jun 25, 2020

@ExE-Boss as is, the PR is normative in the sense that it allows engines to omit more names. Another option is to tighten the toString requirement to require all names.

"well-known" means it's in the table; I don't think dotted forms are in that category - and if they were, then the change to support the dotted notation was normative, because it then imposes the toString name requirement on every function.

@ExE-Boss
Copy link
Contributor

We could also keep the dotted function intrinsics in the Well‑Known Intrinsic Objects table until #2068 is resolved.

@ljharb
Copy link
Member Author

ljharb commented Jun 26, 2020

#1948 or similar would need to land before this is non-normative.

8/7 update: #1948 has landed; this is now editorial.

@ljharb ljharb removed the editor call to be discussed in the next editor call label Jul 8, 2020
ljharb added a commit to tc39/ecma402 that referenced this pull request Jul 9, 2020
Since tc39/ecma262#1376, the dotted form of intrinsic notation is preferred.

Additionally, tc39/ecma262#2056 intends to remove these legacy intrinsics, once it is non-normative to do so. HTML has already been updated to no longer use them.
ljharb added a commit to tc39/ecma402 that referenced this pull request Jul 11, 2020
Since tc39/ecma262#1376, the dotted form of intrinsic notation is preferred.

Additionally, tc39/ecma262#2056 intends to remove these legacy intrinsics, once it is non-normative to do so. HTML has already been updated to no longer use them.
@devsnek
Copy link
Member

devsnek commented Aug 6, 2020

Couldn't we technically just remove this whole table? Every intrinsic is already declared inline with the relevant definition via "is the %Xyz% intrinsic object"

@ljharb
Copy link
Member Author

ljharb commented Aug 7, 2020

@devsnek i think it's useful for the table to exist for all the top-level intrinsics; but either way it'd be out of scope of this PR.

@ljharb ljharb requested a review from jmdyck August 7, 2020 23:02
@jmdyck

This comment has been minimized.

@ljharb ljharb force-pushed the legacy-intrinsics branch from a77cc71 to d8b366b Compare August 10, 2020 05:25
@ljharb

This comment has been minimized.

spec.html Outdated Show resolved Hide resolved
spec.html Outdated
@@ -36709,7 +36168,6 @@ <h1>Properties of the Set Constructor</h1>

<emu-clause id="sec-set.prototype">
<h1>Set.prototype</h1>
<p>The initial value of `Set.prototype` is the intrinsic %SetPrototype% object.</p>
Copy link
Collaborator

Choose a reason for hiding this comment

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

You eliminated this line, and the corresponding one for WeakSet.prototype, which makes sense because they're tautologies now. But why just these two? There are about 20 others.

Copy link
Member Author

Choose a reason for hiding this comment

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

#2131 should address these; i'll rebase this after that lands and clean it up.

spec.html Outdated Show resolved Hide resolved
spec.html Outdated Show resolved Hide resolved
@ljharb ljharb force-pushed the legacy-intrinsics branch from d8b366b to f647a44 Compare August 11, 2020 06:12
leobalter pushed a commit to tc39/ecma402 that referenced this pull request Aug 21, 2020
Since tc39/ecma262#1376, the dotted form of intrinsic notation is preferred.

Additionally, tc39/ecma262#2056 intends to remove these legacy intrinsics, once it is non-normative to do so. HTML has already been updated to no longer use them.
ljharb added a commit to ljharb/ecma262 that referenced this pull request Aug 27, 2020
@ljharb ljharb force-pushed the legacy-intrinsics branch from f647a44 to df824ef Compare August 27, 2020 07:14
@ljharb ljharb requested a review from jmdyck August 27, 2020 07:14
Copy link
Collaborator

@jmdyck jmdyck left a comment

Choose a reason for hiding this comment

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

Looks okay to me.

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.

lgtm, long time in the making, congrats!

@ljharb ljharb self-assigned this Sep 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Editorial convention for referring to intrinsics
8 participants