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 dot notation for accessing internal slots #591

Merged
merged 2 commits into from
Jun 3, 2016

Conversation

jmdyck
Copy link
Collaborator

@jmdyck jmdyck commented Jun 2, 2016

Specifically, change:

   the [[Foo]] internal slot of _O_  (132 occurrences)

and

   _O_'s [[Foo]] internal slot (265 occurrences)

to

   _O_.[[Foo]]

(Resolves issue #574)

@jmdyck
Copy link
Collaborator Author

jmdyck commented Jun 2, 2016

Some things I didn't change...

There are 18 occurrences of

    the [[DateValue]] internal slot of this Date object

which I could conceivably have changed to

    (this Date object).[[DateValue]]

but didn't.

Similarly 1 occurrence of

    the active function object's [[Realm]] internal slot

Also, there are lots of occurrences of

    _O_ has a [[Foo]] internal slot

which could be changed to something like

    _O_.[[Foo]] exists

but I left those unchanged. (Likewise the negative version.)

Finally, we could use dot notation for internal methods more. I.e., there are occurrences of

    the [[Foo]] internal method of _O_

and

    _O_'s [[Foo]] internal method

that you might want changed to

    _O_.[[Foo]]

Let me know if you want a follow-up commit for any of those changes.

@bterlson
Copy link
Member

bterlson commented Jun 2, 2016

Will take a look in a few minutes, but did you mean to include 216a965? I like the change anyway...

(this Date object).[[DateValue]]

Not a fan of this, current convention is better :)

O.[[Foo]] exists

I feel like this is a bit confusing - does it mean [[Foo]] has a value that is not undefined, or that the slot is there? I think the current convention is fine for now.

Finally, we could use dot notation for internal methods more.

I think these are good! Can you think of downsides? It removes the explicit verbage telling you the slot is a method, but from what I can see that is very obviously implied by context in all of the usages...

@bterlson
Copy link
Member

bterlson commented Jun 2, 2016

Do we need "the value of" everywhere? Seems unnecessary to me.

Other than that (and the merge conflict I created), this change seems great!

@domenic
Copy link
Member

domenic commented Jun 2, 2016

+1 for removing "the value of"

@jmdyck
Copy link
Collaborator Author

jmdyck commented Jun 2, 2016

did you mean to include 216a965?

Yup, because it allowed me to include those steps in the systematic changes of the main commit.

O.[[Foo]] exists

I feel like this is a bit confusing - does it mean [[Foo]] has a value that is not undefined, or that the slot is there?

It might be that a different verb would be less confusing. (I thought of suggesting O.[[Foo]] is present, but we already use the present/absent dichotomy in a bunch of other contexts.)

I think the current convention is fine for now.

Okay.

Finally, we could use dot notation for internal methods more.

I think these are good! Can you think of downsides? It removes the explicit verbage telling you the slot is a method, but from what I can see that is very obviously implied by context in all of the usages...

That was the only downside I could think of. I'll add a commit for those changes.

Do we need "the value of" everywhere? Seems unnecessary to me.

That occurred to me too, but I decided to leave it alone. I agree that it seems superfluous in text such as

If the value of _new_.[[ArrayBufferByteLength]] < _newLen_, ...

and

Set the value of _O_.[[ListIteratorNextIndex]] to _index_+1.

But consider

Let _V_ be the value of _O_.[[Foo]].

vs

Let _V_ be _O_.[[Foo]].

I think some people might read the latter as meaning that _V_ is now an alias for that slot, rather than a reference to the value currently there. Since the spec doesn't explicitly say what it does mean, I think "the value of" helps a little.

@domenic
Copy link
Member

domenic commented Jun 2, 2016

I disagree that "the value of" helps in those cases. I think most spec readers have enough experience with programming languages and spec-prose to understand that variable assignment is not generally pointer assignment.

@@ -6292,7 +6292,7 @@
1. If _p_ is *null*, let _done_ be *true*.
1. Else if SameValue(_p_, _O_) is *true*, return *false*.
1. Else,
1. If the [[GetPrototypeOf]] internal method of _p_ is not the ordinary object internal method defined in <emu-xref href="#sec-ordinary-object-internal-methods-and-internal-slots-getprototypeof"></emu-xref>, let _done_ be *true*.
1. If _p_.[[GetPrototypeOf]] is not the ordinary object internal method defined in <emu-xref href="#sec-ordinary-object-internal-methods-and-internal-slots-getprototypeof"></emu-xref>, let _done_ be *true*.
Copy link
Contributor

@claudepache claudepache Jun 2, 2016

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 is a good change, because at first reading it may be interpreted as "If the value of p.[[GetPrototypeOf]]"...

Copy link
Member

Choose a reason for hiding this comment

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

Isn't that first reading correct?

Copy link
Contributor

Choose a reason for hiding this comment

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

@bterlson Yes if we consider that we have an internal slot whose value is an internal method. But I like to keep the distinction between internal slots and internal methods.

@claudepache
Copy link
Contributor

For clarity, I think it is a good idea to use the notation O.[[Foo]] only when it specifically means: "the value of [[Foo]]’s internal slot of O".

@bterlson
Copy link
Member

bterlson commented Jun 2, 2016

Regarding "the value of", I think we can make it clear by tightening up the description in 6.2.1 to say:

For example, if R is the record shown in the previous paragraph then R.[[Field2]] is shorthand for “the value of the field of R named [[Field2]]”.`

@jmdyck
Copy link
Collaborator Author

jmdyck commented Jun 2, 2016

(except that Objects aren't Records, they just use the same notation for internal slots/methods)

@bterlson
Copy link
Member

bterlson commented Jun 2, 2016

Yes, that should be clarified as well :)

@jmdyck
Copy link
Collaborator Author

jmdyck commented Jun 2, 2016

@claudepache:

For clarity, I think it is a good idea to use the notation O.[[Foo]] only when it specifically means: "the value of [[Foo]]’s internal slot of O".

What do you intend to be excluded by emphasizing the words "the value of"? (Perhaps you mean that we shouldn't change _O_ has a [[Foo]] internal slot to _O_.[[Foo]] exists, because that isn't a reference to the value of the slot, merely to the existence of the slot. But @bterlson has already discouraged such a change, and this PR doesn't make it.)

And by saying "slot", do you mean to exclude internal methods?

@claudepache
Copy link
Contributor

@claudepache:

For clarity, I think it is a good idea to use the notation O.[[Foo]] only when it specifically means: "the value of [[Foo]]’s internal slot of O".

What do you intend to be excluded by emphasizing the words "the value of"?

For example, "O.[[Foo]] exists" when it means "O has a [[Foo]] internal slot".

And by saying "slot", do you mean to exclude internal methods?

Probably, yes. For internal methods, use only the notation "O.[Foo]", and use it only when it means: "the value returned by calling the [[Foo]] internal method of O with argument bar.

... in the 4 places where it is preceded by:
    Let _args_ be the arguments object.

(Prep for subsequent refactor.)
Specifically, change:
    the [[Foo]] internal slot of _O_   ->   _O_.[[Foo]]   (132 occ)

    _O_'s [[Foo]] internal slot        ->   _O_.[[Foo]]   (265 occ)

(Resolves issue tc39#574)
@bterlson
Copy link
Member

bterlson commented Jun 3, 2016

I am personally fine with the conceptual model where internal methods are internal slots with function-like values, but in the interest of moving this along without creating massive rebasing headaches, I think we can take everything but 000be90 and get a huge win. We can discuss using the dot-convention for methods in a separate issue. Thoughts @jmdyck?

@jmdyck
Copy link
Collaborator Author

jmdyck commented Jun 3, 2016

Fine with me.

(Though I'm not sure how it works git-wise: do I resubmit 000be90 as a new PR, or do you create it, or do you just want the topic discussed first?)

@bterlson
Copy link
Member

bterlson commented Jun 3, 2016

I'm sure there's a million better ways to do it, but the way I would do it is to something like

  1. git checkout editorial
  2. git checkout -b some-branch-name
  3. git checkout editorial
  4. git reset --hard HEAD~1
  5. git push -f /editorial (this will update this PR)
  6. git push /some-branch-name (and send a new PR for this)

@claudepache
Copy link
Contributor

claudepache commented Jun 3, 2016

I am personally fine with the conceptual model where internal methods are internal slots with function-like values

Note that [[RegExpMatcher]] contains a procedure, but is (rightfully) an internal slot. My guess is that we are spoiled by the JS object model where the distinction between methods and properties is blurred. :-P

@jmdyck
Copy link
Collaborator Author

jmdyck commented Jun 3, 2016

Okay, done, I think.

@bterlson
Copy link
Member

bterlson commented Jun 3, 2016

Looks like you've done it, thanks!

@jmdyck are you interested in doing a follow-up pr for removal of "the value of"? I can put one up too if you'd like, no worries.

@bterlson bterlson merged commit 0deb41f into tc39:master Jun 3, 2016
@jmdyck
Copy link
Collaborator Author

jmdyck commented Jun 3, 2016

I'll think about it, but I've got another PR to do first.

@domenic
Copy link
Member

domenic commented Jun 3, 2016

<3 yaaaay this makes specs so much more readable. Going to go change streams now.

domenic added a commit to whatwg/streams that referenced this pull request Jun 3, 2016
@ was originally introduced as a shorthand, since the ECMAScript spec always used the longhand "the value of the [[X]] internal slot of Y". As of tc39/ecma262#591, ECMAScript has now moved to Y.[[X]] as notation, so we can follow their lead.

See also the original discussion where @ was settled on in #178.
@jmdyck jmdyck deleted the editorial branch June 3, 2016 20:46
jmdyck added a commit to jmdyck/ecma262 that referenced this pull request Jun 4, 2016
Introduce variable _f_ for the active function object,
which then allows the use of dot notation for _f_.[[Realm]].
(Should have done this in PR tc39#591.)

(This will also allow a later refactoring.)
bterlson pushed a commit that referenced this pull request Jun 6, 2016
Introduce variable _f_ for the active function object,
which then allows the use of dot notation for _f_.[[Realm]].
(Should have done this in PR #591.)

(This will also allow a later refactoring.)
ljharb added a commit to tc39/proposal-promise-finally that referenced this pull request Jul 29, 2016
jyasskin added a commit to jyasskin/web-bluetooth-1 that referenced this pull request Aug 9, 2016
jyasskin added a commit to jyasskin/web-bluetooth-1 that referenced this pull request Aug 10, 2016
jyasskin added a commit to WebBluetoothCG/web-bluetooth that referenced this pull request Aug 11, 2016
domenic added a commit to domenic/browser-payment-api that referenced this pull request Dec 13, 2016
Fixes w3c#336. See the history here in whatwg/streams#178tc39/ecma262#574tc39/ecma262#591whatwg/streams@7791c58. My bad for leading people down this path.
domenic added a commit to domenic/browser-payment-api that referenced this pull request Dec 13, 2016
Fixes w3c#336. See the history here in whatwg/streams#178tc39/ecma262#574tc39/ecma262#591whatwg/streams@7791c58. My bad for leading people down this path.
marcoscaceres pushed a commit to w3c/payment-request that referenced this pull request Dec 13, 2016
Fixes #336. See the history here in whatwg/streams#178tc39/ecma262#574tc39/ecma262#591whatwg/streams@7791c58. My bad for leading people down this path.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants