-
Notifications
You must be signed in to change notification settings - Fork 165
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
Conversation
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 |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
I'm not sure how ready this thing is for review. Will need to read through it tomorrow (or more likely, Thursday). |
It's probably fine to disallow using |
There was a problem hiding this 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 |
There was a problem hiding this comment.
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. |
There was a problem hiding this comment.
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|. |
There was a problem hiding this comment.
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: |
There was a problem hiding this comment.
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>}). |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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, |
There was a problem hiding this comment.
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. |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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. |
There was a problem hiding this comment.
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>. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Capitalize throw
I'll rebase when we're done with the review (I think that's screws up the review process the least). |
There was a problem hiding this 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=] |
There was a problem hiding this comment.
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|. |
There was a problem hiding this comment.
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=]. |
There was a problem hiding this comment.
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]]. |
There was a problem hiding this comment.
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.
6184b2c
to
83e0087
Compare
* 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.
b7c5579
to
dd73cf7
Compare
* 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.
out of interface objects. Closes Legacy callback interface objects are badly defined #78.
new
for named constructors #275.named constructors, interface objects, and
legacy callback interface objects.
callback interfaces with constants.
Callback interfaces do not have an interface prototype object.
Their constants sit on the legacy callback interface object itself.
Closes Constants require an interface prototype object which callback interfaces don't have #281.
Preview | Diff