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: use <ul> to list characteristics of intrinsics #1125

Merged
merged 9 commits into from
May 17, 2018

Conversation

jmdyck
Copy link
Collaborator

@jmdyck jmdyck commented Mar 2, 2018

[Note that, currently, the first 8 commits listed for this PR actually belong to PR #1122. If/when that's merged, I think they'll disappear from here.]

For many intrinsics (the constructors, the prototype objects, and a few others), there are blocks of text that specify (what I'm calling here) their 'characteristics'. (Elsewhere I'd say 'properties', but in ES that word is already taken.) This PR converts each of those blocks from paragraphs to a bulleted list.

Mainly, I think the result is more readable (both by humans and software). Implementers and testers can (more easily) treat it as a checklist of items to implement and test.

The commit could be merged as is. However, there are variations/extensions that you might like to consider/discuss/request.

(1)
Grammatically speaking, each <li> is a predicate, where the corresponding subject is factored out into the line preceding the <ul>. Instead, you might prefer that each <li> be a complete sentence. E.g.,

    The Array constructor has the following characteristics:
      - It is the intrinsic object <dfn>%Array%</dfn>.
      - It is the initial value of the `Array` property of the global object.
      - etc

One advantage of this approach is that it lets you use more 'natural' wording. E.g., instead of
- has a [[Prototype]] internal slot whose value is the intrinsic object %FunctionPrototype%.
you can say:
- The value of its [[Prototype]] internal slot is the intrinsic object %FunctionPrototype%.
(which is more like how the spec currently structures that statement).

(2)
For intrinsic objects that are constructors, their characteristics are split over 2 clauses (typically "The Foo Constructor" and "Properties of the Foo Constructor"). You might like to combine these 2 blocks so that all the characteristics appear in one place. (The clauses would still be separate.)

(3)
The characteristics of an intrinsic are mostly given in (a subset of) the following order:

  • If it's a well-known intrinsic, what is its Table 7 name?
  • If it has a global name, what's that?
  • Is it the value of some property of some other intrinsic?
  • What kind of object is it?
  • Can it be used as a constructor, and if so, what does that do?
  • Can it be called as a function, and if so, what does that do?
  • If it's a constructor, is it subclassable?
  • What is the value of its [[Prototype]]?
  • What is the value of its [[Extensible]]?
  • What are its data properties?

However, there are exceptions to this order. You might like the order to be standardized (either to the above, or something else).

(4)
You might like to introduce special markup/markdown for these blocks (e.g., <emu-intrinsic>), making them more machine-readable, which could be useful to implementers and testers, and would allow for specialized rendering in the HTML version of the spec.

(5)
As a follow-up to the discussion re PR #1122, you might like to use these blocks to make the per-Realm-ness of intrinsics more explicit. E.g., starting each block with:
Within each Realm, the Foo constructor:
or
Every Realm has a Foo constructor, with the following characteristics:

@ljharb
Copy link
Member

ljharb commented Mar 2, 2018

(@jmdyck ftr, after #1122 is merged, you'll have to rebase and force push this PR to have them disappear from here)

Copy link
Member

@ljharb ljharb left a comment

Choose a reason for hiding this comment

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

Do you have an example rendering of this change? It's hard to tell if it's an improvement or not just from the text.

This is also a very very long diff; I wonder if it would be easier to review if each section was changed in a separate commit?

spec.html Outdated
<p>In addition to the properties defined in this specification the global object may have additional host defined properties. This may include a property whose value is the global object itself; for example, in the HTML document object model the `window` property of the global object is the global object itself.</p>
<p>The global object:</p>
<ul>
<li>is created before control enters any execution context.</li>
Copy link
Member

Choose a reason for hiding this comment

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

This removes the word "unique", is that intended?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, intended. To say "unique" for this object but not the others suggests a distinction that I don't think is there, e.g. that there's a single global object shared by all realms.

<li>is the initial value of the `Math` property of the global object.</li>
<li>is an ordinary object.</li>
<li>has a [[Prototype]] internal slot whose value is the intrinsic object %ObjectPrototype%.</li>
<li>is not a function object.</li>
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if it would be helpful to establish a definition for objects like Math and JSON that are "namespace objects", so the boilerplate on both could be reduced?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Note that the spec already has something called a "module namespace object", so introducing the (unrelated) term "namespace object" might be confusing.

We could reduce some boilerplate if we simply acknowledged that "is not a function object" implies "does not have [[Call]] or [[Construct]]". (If we wanted a term for auto-linking, "non-function object" would do.)

Also, assuming it's true that every non-function object necessarily "does not have an [[ECMAScriptCode]] internal slot or any other of the internal slots listed in table 27", then we could reduce some more boilerplate by saying that in one place.

<li>returns a String representing the current time (UTC) when called as a function rather than as a constructor.</li>
<li>is a single function whose behaviour is overloaded based upon the number and types of its arguments.</li>
<li>is designed to be subclassable. It may be used as the value of an `extends` clause of a class definition. Subclass constructors that intend to inherit the specified `Date` behaviour must include a `super` call to the `Date` constructor to create and initialize the subclass instance with a [[DateValue]] internal slot.</li>
<li>has a `length` property whose value is 7.</li>
Copy link
Member

Choose a reason for hiding this comment

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

should 7 have backticks?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The spec isn't consistent on whether it should have backticks, or asterisks, or neither. And this PR isn't attempting to resolve that inconsistency.

ljharb
ljharb previously requested changes Mar 2, 2018
Copy link
Member

@ljharb ljharb left a comment

Choose a reason for hiding this comment

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

(blocking solely to ensure this isn't merged prior to #1122 going in and this being rebased)

@jmdyck
Copy link
Collaborator Author

jmdyck commented Mar 3, 2018

Do you have an example rendering of this change?

Nope. I'll see if I can come up with one.

This is also a very very long diff; I wonder if it would be easier to review if each section was changed in a separate commit?

Well, that would be 88 commits, which seems high. Unless, by "section", you meant "top-level clause", in which case that'd be only 9 commits (for clauses 18 through 26 inclusive).

@ljharb
Copy link
Member

ljharb commented Mar 4, 2018

The 9 commit variant seems fine, if that's something you're willing to do :-)

@jmdyck
Copy link
Collaborator Author

jmdyck commented Mar 4, 2018

The 9 commit variant seems fine, if that's something you're willing to do :-)

I'm willing, but I think I might wait until #1122 is merged (or otherwise resolved), so I only have to force-push once?

@bterlson
Copy link
Member

I merged #1122. If anyone gets time to rebase this I'll pull it.

@ljharb ljharb dismissed their stale review March 20, 2018 11:10

I've done the rebase.

Copy link
Member

@ljharb ljharb left a comment

Choose a reason for hiding this comment

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

Still lots of changes, and I'd prefer the 9-commit approach, but this LGTM if @bterlson wants to pull now.

<li>accepts any arguments and returns *undefined* when invoked.</li>
<li>does not have a [[Construct]] internal method; it cannot be used as a constructor with the `new` operator.</li>
<li>has a [[Prototype]] internal slot whose value is the intrinsic object %ObjectPrototype%.</li>
<li>has an [[Extensible]] internal slot whose initial value is *true*.</li>
Copy link
Member

Choose a reason for hiding this comment

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

Not related to this PR; just dropping a note to myself for later: does this need a specific call-out, or is the default [[Extensible]] true?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Prose-wise, Clause 17 says Unless specified otherwise, the [[Extensible]] internal slot of a built-in object initially has the value *true*. And algorithm-wise, CreateIntrinsics creates %FunctionPrototype% via CreateBuiltinFunction, which explicitly sets [[Extensible]] to *true*.

So yeah, that specific point is redundant.

(And similarly for the [[Extensible]] bullet of 13 other intrinsics. Offhand, it looks like the only intrinsic with [[Extensible]] = *false* is %ThrowTypeError%, although interestingly, the prose that says so doesn't agree with the setting in CreateBuiltinFunction. CreateIntrinsics should maybe Set _thrower_.[[Extensible]] to *false* explicitly.)

Copy link
Member

Choose a reason for hiding this comment

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

Is it worth removing the redundant points in this PR?

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 don't know, some people might like them. Anyhow, the question is orthogonal to this PR. (But I'd be fine with submitting a separate commit if there's consensus to remove them.)

<ul>
<li>is the intrinsic object <dfn>%StringPrototype%</dfn>.</li>
<li>is a String exotic object and has the internal methods specified for such objects.</li>
<li>has a [[StringData]] internal slot whose value is *""*.</li>
Copy link
Member

Choose a reason for hiding this comment

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

Note for later: should this use *""* or the empty String?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The spec is far more inclined to say the empty String (or sometimes the empty string), maybe because (depending on the font) it might be unclear whether "" has a space between the quotes or not.

@jmdyck
Copy link
Collaborator Author

jmdyck commented Mar 20, 2018

I'm still willing to do the 9-commit split, and also a rendered version, I've just been busy with other stuff. Plus I was hoping for more discussion on the possibilities raised in the initial comment. But if @bterlson wants to pull as-is, that's fine with me too.

@jmdyck
Copy link
Collaborator Author

jmdyck commented Mar 29, 2018

Do you have an example rendering of this change? It's hard to tell if it's an improvement or not just from the text.

http://jmdyck.github.io/ecma262/characteristics_of_intrinsics.html

@jmdyck
Copy link
Collaborator Author

jmdyck commented Apr 4, 2018

forced-pushed the 9-commit split, @ljharb.

Copy link
Member

@ljharb ljharb left a comment

Choose a reason for hiding this comment

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

Thanks, splitting it up made it MUCH easier to review.

I think none of my comments are blockers and could be done in a follow up.

<ul>
<li>is the intrinsic object <dfn>%StringPrototype%</dfn>.</li>
<li>is a String exotic object and has the internal methods specified for such objects.</li>
<li>has a [[StringData]] internal slot whose value is *""*.</li>
Copy link
Member

Choose a reason for hiding this comment

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

“is the empty String”?

<li>also creates and initializes a new Array object when called as a function rather than as a constructor. Thus the function call `Array(&hellip;)` is equivalent to the object creation expression `new Array(&hellip;)` with the same arguments.</li>
<li>is a single function whose behaviour is overloaded based upon the number and types of its arguments.</li>
<li>is designed to be subclassable. It may be used as the value of an `extends` clause of a class definition. Subclass constructors that intend to inherit the exotic `Array` behaviour must include a `super` call to the `Array` constructor to initialize subclass instances that are Array exotic objects. However, most of the `Array.prototype` methods are generic methods that are not dependent upon their `this` value being an Array exotic object.</li>
<li>has a `length` property whose value is 1.</li>
Copy link
Member

Choose a reason for hiding this comment

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

asterisks or back ticks around the 1?

<p>The GeneratorFunction prototype object:</p>
<ul>
<li>is an ordinary object.</li>
<li>is not a function object and does not have an [[ECMAScriptCode]] internal slot or any other of the internal slots listed in <emu-xref href="#table-27"></emu-xref> or <emu-xref href="#table-56"></emu-xref>.</li>
Copy link
Member

Choose a reason for hiding this comment

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

This seems like it might be a more concise way to express all the other “doesn’t have call, isn’t a function, doesn’t have Construct” instances in the spec

@jmdyck
Copy link
Collaborator Author

jmdyck commented Apr 23, 2018

(Force-pushed updates to resolve conflict created by PR #1153.)

@bterlson bterlson merged commit 43ba390 into tc39:master May 17, 2018
@bterlson
Copy link
Member

I can't wait for the rendering to update. This is a big improvement, thanks a ton!

@jmdyck jmdyck deleted the chofin branch May 22, 2018 17:19
jmdyck added a commit to jmdyck/ecma262 that referenced this pull request May 22, 2018
This follows up on a comment from @ljharb re PR 1125:
tc39#1125 (comment)
bterlson pushed a commit that referenced this pull request Jun 4, 2018
* Markup: insert missing </p>

* Editorial: add underscores around metavariable-name

* Editorial: change *""* to 'the empty String'

This follows up on a comment from @ljharb re PR 1125:
#1125 (comment)
ljharb pushed a commit to ljharb/ecma262 that referenced this pull request Jun 5, 2018
* bulletized intrinsics: clause 18 The Global Object
* bulletized intrinsics: clause 19 Fundamental Objects
* bulletized intrinsics: clause 20 Numbers and Dates
* bulletized intrinsics: clause 21 Text Processing
* bulletized intrinsics: clause 22 Indexed Collections
* bulletized intrinsics: clause 23 Keyed Collection
* bulletized intrinsics: clause 24 Structured Data
* bulletized intrinsics: clause 25 Control Abstraction Objects
* bulletized intrinsics: clause 26 Reflection
ljharb pushed a commit to ljharb/ecma262 that referenced this pull request Jun 5, 2018
* Markup: insert missing </p>

* Editorial: add underscores around metavariable-name

* Editorial: change *""* to 'the empty String'

This follows up on a comment from @ljharb re PR 1125:
tc39#1125 (comment)
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.

3 participants