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

Revamp interface bindings #283

Closed
wants to merge 10 commits into from
Closed

Conversation

tobie
Copy link
Collaborator

@tobie tobie commented Jan 24, 2017


Preview | Diff

that is the value of the aforementioned property.

The identifier used for the named constructor must not
The [=NamedConstructor identifier|identifier=] used for the named constructor must not
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 not sure this is really useful.

Copy link
Member

Choose a reason for hiding this comment

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

I could go either way. We don't have separate #x-identifier concepts for everything else. On the other hand, the "after the = sign" thing down below does sure seem out of place, so moving it up here is not a bad idea.

@tobie
Copy link
Collaborator Author

tobie commented Jan 25, 2017

I'm not sure how ready this thing is for review. Will need to read through it tomorrow (or more likely, Thursday).

@foolip
Copy link
Member

foolip commented Jan 25, 2017

It's probably fine to disallow using [NoInterfaceObject] on callback interfaces period. At least I wouldn't expect to see any new callback interfaces on the platform, and the existing ones don't have have [NoInterfaceObject].

Copy link
Member

@domenic domenic 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 really, really great modernization and bug-fixing work. Found some nits, but overall quite happy.

that is the value of the aforementioned property.

The identifier used for the named constructor must not
The [=NamedConstructor identifier|identifier=] used for the named constructor must not
Copy link
Member

Choose a reason for hiding this comment

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

I could go either way. We don't have separate #x-identifier concepts for everything else. On the other hand, the "after the = sign" thing down below does sure seem out of place, so moving it up here is not a bad idea.

index.bs Outdated
then [=ECMAScript/throw=] a <emu-val>TypeError</emu-val>.
1. If [=NewTarget=] is <emu-val>undefined</emu-val>, then
[=ECMAScript/throw=] a <emu-val>TypeError</emu-val>.
1. Let |arg|<sub>0..|n|−1</sub> be arguments.
Copy link
Member

Choose a reason for hiding this comment

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

"the passed arguments" (here and below)

index.bs Outdated
1. Return |O|.
1. Let |proto| be the [=%FunctionPrototype%=] of |realm|.
1. If |I| inherits from some other interface |P|,
then set |proto| to the [=interface object=] of |P|.
Copy link
Member

Choose a reason for hiding this comment

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

"interface object of P in realm".

index.bs Outdated
Otherwise, the value is determined as follows:
The [=interface object=]
for a given non-callback [=interface=] |I| with [=identifier=] |id|
and [=Realm=] |realm| is created as follows:
Copy link
Member

Choose a reason for hiding this comment

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

"and in Realm realm"

index.bs Outdated
PropertyDescriptor{\[[Value]]: |length|, \[[Writable]]: <emu-val>false</emu-val>, \[[Enumerable]]: <emu-val>false</emu-val>, \[[Configurable]]: <emu-val>true</emu-val>}).
1. Let |obj| be the [=interface prototype object=] of [=interface=] |I|.
1. Perform [=!=] [=DefinePropertyOrThrow=](|F|, "prototype",
PropertyDescriptor{\[[Value]]: |obj|, \[[Writable]]: <emu-val>false</emu-val>, \[[Enumerable]]: <emu-val>false</emu-val>, \[[Configurable]]: <emu-val>false</emu-val>}).
Copy link
Member

Choose a reason for hiding this comment

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

s/obj/proto; surprised the var checker didn't catch this

Copy link
Collaborator Author

@tobie tobie Feb 1, 2017

Choose a reason for hiding this comment

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

That's correct, actually. |obj| is the [=interface prototype object=]. prototype. |proto| is the [[Prototype]] slot of the [=interface object=].

Agree that the variables are ill-named. Renaming to |proto| and |constructorProto| respectively.

index.bs Outdated
This object also must be
associated with the ECMAScript global environment associated
with the [=named constructor=].
If the internal \[[Call]] method of the [=named constructor=] returns normally,
Copy link
Member

Choose a reason for hiding this comment

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

Prexisting problem, but: this shouldn't be talking about the internal [[Call]] method. It should be talking about the steps that spec authors use to describe the named constructor ("the actions listed in the description of constructor" below).

@@ -10141,87 +10093,67 @@ which allows construction of objects that
implement the interface on which the
[{{NamedConstructor}}] extended attributes appear.
Copy link
Member

Choose a reason for hiding this comment

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

I would remove the "must have a [[Call]] property" sentence; that is implicit and restating it is confusing (how could it not be true)?

index.bs Outdated
A named constructor must have a property named “length” with attributes
{ \[[Writable]]: <emu-val>false</emu-val>, \[[Enumerable]]: <emu-val>false</emu-val>, \[[Configurable]]: <emu-val>true</emu-val> }
whose value is a <emu-val>Number</emu-val> determined as follows:
Assuming |id| is the [=NamedConstructor identifier|identifier=] of the constructor
Copy link
Member

Choose a reason for hiding this comment

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

I'd restate this intro as you did for interface objects. E.g.

The named constructor for a given non-callback interface I with identifier id in Realm realm is created as follows:

index.bs Outdated
then the interface prototype object must also
have a property named “constructor” with attributes
{ \[[Writable]]: <emu-val>true</emu-val>, \[[Enumerable]]: <emu-val>false</emu-val>, \[[Configurable]]: <emu-val>true</emu-val> }
whose value is a reference to the interface object for the interface.
Copy link
Member

Choose a reason for hiding this comment

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

Link interface object while you're here?

index.bs Outdated
and [=Realm=] |realm| is created as follows:

1. Let |steps| be the following steps:
1. [=ECMAScript/throw=] a <emu-val>TypeError</emu-val>.
Copy link
Member

Choose a reason for hiding this comment

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

Capitalize throw

@tobie
Copy link
Collaborator Author

tobie commented Feb 1, 2017

I'll rebase when we're done with the review (I think that's screws up the review process the least).

Copy link
Member

@domenic domenic left a comment

Choose a reason for hiding this comment

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

A few more nits, but feel free to merge after fixing them.

index.bs Outdated

<h5 id="es-constructible-interfaces" oldids="es-interface-call">Constructible Interfaces</h5>
The [=interface object=] for a given [=interface=] is a [=function object=]
Copy link
Member

Choose a reason for hiding this comment

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

Missing period

index.bs Outdated
associated with the ECMAScript global environment associated
with the [=named constructor=].
If the actions listed in the description of the constructor return normally,
then the [=named constructor=] must return an object that implements interface |I|.
Copy link
Member

Choose a reason for hiding this comment

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

"then those steps must return".

index.bs Outdated
If the actions listed in the description of the constructor return normally,
then the [=named constructor=] must return an object that implements interface |I|.
This object also must be associated with the ECMAScript global environment
associated with the [=named constructor=].
Copy link
Member

Choose a reason for hiding this comment

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

"This object's relevant Realm must be the same as that of the named constructor"

index.bs Outdated
and its value is an object called the
<dfn id="dfn-legacy-callback-interface-object" export>legacy callback interface object</dfn>.
The property has the attributes { \[[Writable]]: <emu-val>true</emu-val>, \[[Enumerable]]: <emu-val>false</emu-val>, \[[Configurable]]: <emu-val>true</emu-val> }.
The characteristics of a legacy callback interface object are described in [[#legacy-callback-interface-object]].
Copy link
Member

Choose a reason for hiding this comment

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

This last sentence is just a link to the section that the sentence is already in, which seems bad.

* Extract legacy callback interface objects
  out of interface objects. Closes whatwg#78.
* Clarify that legacy callback interfaces are function objects.
* Give them a length property. Closes whatwg#279. Closes whatwg#83.
* Require new for Named Constructors. Closes whatwg#275.
* Rely on ES abstract operations for defining,
  named constructors, interface objects, and
  legacy callback interface objects.
* Disallow using the [NoInterfaceObject] on
  callback interfaces with constants.
Callback interfaces do not have an interface prototype object.
Their constants sit on the legacy callback interface object itself.

Closes whatwg#281.
@tobie tobie closed this in #313 Feb 20, 2017
tobie added a commit that referenced this pull request Feb 20, 2017
* Extract legacy callback interface objects
  out of interface objects. Closes #78.
* Clarify that legacy callback interfaces are function objects.
* Give them a length property. Closes #279. Closes #83.
* Require new for Named Constructors. Closes #275.
* Rely on ES abstract operations for defining
  named constructors, interface objects, and
  legacy callback interface objects.
* Disallow using [NoInterfaceObject] on
  callback interfaces with constants.
* Don't require an interface prototype object for constants.
  Callback interfaces do not have an interface prototype object.
  Their constants sit on the legacy callback interface object itself.
  Closes #281.

Closes #283.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants