- From: Domenic Denicola <notifications@github.com>
- Date: Sat, 10 Feb 2018 01:58:23 +0000 (UTC)
- To: heycam/webidl <webidl@noreply.github.com>
- Cc: Subscribed <subscribed@noreply.github.com>
- Message-ID: <heycam/webidl/pull/494/review/95606373@github.com>
domenic commented on this pull request.
This is really really good. A few minor nits, and I think we need a bit better signposting about the remaining declarative bits so people are clear what the status is. I think they are:
- interface prototype objects and globals get properties declaratively from:
- stringifiers
- common iterator behavior
- iterable declarations
- maplike declarations
- setlike declarations
- interface prototype objects and globals get internal methods declaratively from:
- platform objects implementing interfaces
- legacy platform objects
- instances of interface types get properties declaratively from:
- unforgeable operations
- unforgeable attributes
I think that's everything?
> 1. Perform [=!=] <a abstract-op>DefinePropertyOrThrow</a>(|F|, "<code>prototype</code>",
PropertyDescriptor{\[[Value]]: |proto|, \[[Writable]]: <emu-val>false</emu-val>, \[[Enumerable]]: <emu-val>false</emu-val>, \[[Configurable]]: <emu-val>false</emu-val>}).
+ 1. [=Define the constants=] of [=interface=] |I| on |F| given |realm|.
I might name these "install", but define works too.
> - [=interface prototype object=]
- for the inherited interface.
- 1. Otherwise, if |A| is declared with the [{{LegacyArrayClass}}]
- extended attribute, then return {{%ArrayPrototype%}}.
- 1. Otherwise, return {{%ObjectPrototype%}}.
+ The [=interface prototype object=] for a given [=interface=] |interface| and [=Realm=] |realm|
+ is <dfn lt="create an interface prototype object">created</dfn> as follows:
+
+ 1. Let |proto| be null.
+ 1. If |interface| is declared with the [{{Global}}] [=extended attribute=],
+ and |interface| [=support named properties|supports named properties=],
+ then set |proto| to the [=named properties object=] in |realm| of |interface|.
+ 1. Otherwise, if |interface| is declared to inherit from another interface,
+ then set |proto| to the [=interface prototype object=] in |realm|
+ of that [=inherited interface=].
+ 1. Otherwise, if |interface| is the {{DOMException}} [=interface=],
Nice that we get to move this into here, instead of having it be an exception in the DOMException definition. We should remove the "must" from the custom bindings section, and instead make a statement of fact that refers back to this algorithm.
> + given |op|, |interface|, and |realm|.
+ 1. Let |modifiable| be <emu-val>false</emu-val> if |op| is [=unforgeable=]
+ and <emu-val>true</emu-val> otherwise.
+ 1. Let |desc| be the PropertyDescriptor{\[[Value]]: |method|,
+ \[[Writable]]: |modifiable|, \[[Enumerable]]: <emu-val>true</emu-val>,
+ \[[Configurable]]: |modifiable|}.
+ 1. Let |id| be |op|'s [=identifier=].
+ 1. Perform [=!=] <a abstract-op>DefinePropertyOrThrow</a>(|interfaceProtoObj|, |id|, |desc|).
+ 1. [=Expose the constants=] of |interface| on |interfaceProtoObj|.
+ 1. If the [{{NoInterfaceObject}}] [=extended attribute=] was not specified on |interface|, then:
+ 1. Let |constructor| be the [=interface object=] of |interface|.
+ 1. Let |desc| be the PropertyDescriptor{\[[Writable]]: <emu-val>true</emu-val>,
+ \[[Enumerable]]: <emu-val>false</emu-val>, \[[Configurable]]: <emu-val>true</emu-val>,
+ \[[Value]]: |constructor|}.
+ 1. Perform [=!=] <a abstract-op>DefinePropertyOrThrow</a>(|interfaceProtoObj|, "<code>constructor</code>", |desc|).
+ 1. Return |interfaceProtoObj|.
It might be good to add a warning on what's missing, i.e. what features of the interfaceProtoObj are still installed declaratively, with pointers to the sections that do that. I definitely agree it's good to start with this though and not bite it all off at once.
> + and |interface| [=support named properties|supports named properties=],
+ then set |proto| to the [=named properties object=] in |realm| of |interface|.
+ 1. Otherwise, if |interface| is declared to inherit from another interface,
+ then set |proto| to the [=interface prototype object=] in |realm|
+ of that [=inherited interface=].
+ 1. Otherwise, if |interface| is the {{DOMException}} [=interface=],
+ then set |proto| to |realm|.\[[Intrinsics]].[[{{%ErrorPrototype%}}]].
+ 1. Otherwise, if |interface| is declared with the [{{LegacyArrayClass}}] [=extended attribute=],
+ then set |proto| to |realm|.\[[Intrinsics]].[[{{%ArrayPrototype%}}]].
+ 1. Otherwise, set |proto| to |realm|.\[[Intrinsics]].[[{{%ObjectPrototype%}}]].
+ 1. Assert: <a abstract-op>Type</a>(|proto|) is Object.
+ 1. Let |interfaceProtoObj| be [=!=] <a abstract-op>ObjectCreate</a>(|proto|).
+ 1. If |interface| has any [=member=] declared with the [{{Unscopable}}] [=extended attribute=],
+ then:
+ 1. Let |unscopableObject| be the result of performing [=!=]
+ <a abstract-op>ObjectCreate</a>(<emu-val>null</emu-val>).
The use of null here is a normative change that updates us to match the ES spec. Does it close an open bug? It'll be worth adding tests for :).
> + <emu-val>true</emu-val>).
+ 1. Let |desc| be the PropertyDescriptor{\[[Value]]: |unscopableObject|,
+ \[[Writable]]: <emu-val>false</emu-val>, \[[Enumerable]]: <emu-val>false</emu-val>,
+ \[[Configurable]]: <emu-val>true</emu-val>}.
+ 1. Perform [=!=] <a abstract-op>DefinePropertyOrThrow</a>(|interfaceProtoObj|, {{@@unscopables}}, |desc|).
+ 1. If |interface| is not declared with the [{{Global}}] [=extended attribute=], then:
+ 1. [=Define the regular attributes=] of |interface| on |interfaceProtoObj| given |realm|.
+ 1. [=Define the regular operations=] of |interface| on |interfaceProtoObj| given |realm|.
+ 1. [=Define the constants=] of |interface| on |interfaceProtoObj| given |realm|.
+ 1. If the [{{NoInterfaceObject}}] [=extended attribute=] was not specified on |interface|, then:
+ 1. Let |constructor| be the [=interface object=] of |interface| in |realm|.
+ 1. Let |desc| be the PropertyDescriptor{\[[Writable]]: <emu-val>true</emu-val>,
+ \[[Enumerable]]: <emu-val>false</emu-val>, \[[Configurable]]: <emu-val>true</emu-val>,
+ \[[Value]]: |constructor|}.
+ 1. Perform [=!=] <a abstract-op>DefinePropertyOrThrow</a>(|interfaceProtoObj|, "<code>constructor</code>", |desc|).
+ 1. Return |interfaceProtoObj|.
</div>
We can imperative-ize the immutable prototype exotic objectness using the technique of this example: https://tc39.github.io/ecma262/#sec-boundfunctioncreate step 5. Actually, I see you used that same technique later on.
>
- The [=interface object=]
- for a given non-callback [=interface=] |I| with [=identifier=] |id|
- and in [=Realm=] |realm| is created as follows:
+ The [=interface object=] for a given non-callback [=interface=] |I|
+ with [=identifier=] |id| and in [=Realm=] |realm|
+ is <dfn lt="create an interface object">created</dfn> as follows:
Most of these algorithms don't return anything. Although I don't think they have callers yet, probably they should return the thing they create.
> - \[[Prototype]] [=internal slot=] whose value is returned from
- the following steps:
-
- 1. If |A| is declared to inherit from another interface, then return the
- [=interface prototype object=]
- for the inherited interface.
- 1. Otherwise, if |A| is declared with the [{{LegacyArrayClass}}]
- extended attribute, then return {{%ArrayPrototype%}}.
- 1. Otherwise, return {{%ObjectPrototype%}}.
+<div algorithm>
+ The [=named properties object=] for a given [=interface=] |interface| and [=Realm=] |realm|,
+ is <dfn lt="create a named properties object">created</dfn> as follows:
+
+ 1. Let |proto| be null.
+ 1. If |interface| is declared to inherit from another interface,
+ then set |proto| to the [=interface prototype object=] in |realm| for the inherited interface.
Link inherited interface?
> - 1. Otherwise, if |A| is declared with the [{{LegacyArrayClass}}]
- extended attribute, then return {{%ArrayPrototype%}}.
- 1. Otherwise, return {{%ObjectPrototype%}}.
+<div algorithm>
+ The [=named properties object=] for a given [=interface=] |interface| and [=Realm=] |realm|,
+ is <dfn lt="create a named properties object">created</dfn> as follows:
+
+ 1. Let |proto| be null.
+ 1. If |interface| is declared to inherit from another interface,
+ then set |proto| to the [=interface prototype object=] in |realm| for the inherited interface.
+ 1. Otherwise, if |interface| is declared with the [{{LegacyArrayClass}}] [=extended attribute=],
+ then set |proto| to |realm|.\[[Intrinsics]].[[{{%ArrayPrototype%}}]].
+ 1. Otherwise, set |proto| to |realm|.\[[Intrinsics]].[[{{%ObjectPrototype%}}]].
+ 1. Let |obj| be a newly created object.
+ 1. Set |obj|'s internal methods to the definitions specified in
+ [=ECMA-262 §9.1|ECMA-262 §9.1 Ordinary object internal methods and internal slots=], unless they are specified below.
Maybe s/below/the rest of this section, or even the rest of `[[#named-properties-object]]`?
> <h4 id="es-constants">Constants</h4>
-For each [=exposed=] [=constant=] defined on an [=interface=] |A|,
-there must be a corresponding property.
-The property has the following characteristics:
+[=Constants=] are exposed on [=interface objects=],
+[=legacy callback interface objects=],
+[=interface prototype objects=], and
+on the single object that implements the interface,
+when an interface is declared with the [{{Global}}] [=extended attribute=].
Might be worth a follow-up bug to disallow constants on [Global]s, as part of the general deprecation-ish of constants.
>
-* The name of the property is the [=identifier=] of the [=constant=].
-* The location of the property is determined as follows:
- * If the interface was declared with the [{{Global}}] [=extended attribute=],
- then the property exists on the single object that implements the interface.
- * Otherwise, if the interface has an [=interface prototype object=],
- then the property exists on it.
-* The value of the property is that which is obtained by [=converted to an ECMAScript value|converting=] the [=constant=]’s IDL value to an ECMAScript value.
-* The property has attributes { \[[Writable]]: <emu-val>false</emu-val>, \[[Enumerable]]: <emu-val>true</emu-val>, \[[Configurable]]: <emu-val>false</emu-val> }.
+<div algorithm>
+ To <dfn>define the constants</dfn> of [=interface=] or [=namespace=] |definition| on |target|,
Namespaces can't have constants, so no need for that.
> -* The location of the property is determined as follows:
- * If the attribute is a [=static attribute=],
- then there is a single corresponding property and it exists on the interface’s [=interface object=].
- * Otherwise, if the attribute is [=unforgeable=] on
- the interface or if the interface was declared with the [{{Global}}] [=extended attribute=],
- then the property exists on every object that implements the interface.
- * Otherwise, the property exists solely on the interface’s [=interface prototype object=].
-* The property has attributes { \[[Get]]: |G|, \[[Set]]: |S|, \[[Enumerable]]: <emu-val>true</emu-val>, \[[Configurable]]: |configurable| },
- where:
- * |configurable| is <emu-val>false</emu-val> if the attribute was
- declared with the [{{Unforgeable}}] extended attribute and <emu-val>true</emu-val> otherwise;
- * |G| is the [=attribute getter=] created given the attribute, the interface,
- and the [=relevant Realm=] of the object that is the location of the property; and
- * |S| is the [=attribute setter=] created given the attribute, the interface,
- and the [=relevant Realm=] of the object that is the location of the property.
+[=Static attributes=] are exposed on the [=interface object=].
I think here it's worth calling out that "define the unforgeable regular attributes" doesn't yet have any callers. Perhaps with a normative statement like "The implementation must perform define the unforgeable regular attributes on every instance created of every interface type. NOTE: we hope to turn this declarative into imperative later." (My phrasing is bad; end of long day; please phrase better :))
> -* The location of the property is determined as follows:
- * If the attribute is a [=static attribute=],
- then there is a single corresponding property and it exists on the interface’s [=interface object=].
- * Otherwise, if the attribute is [=unforgeable=] on
- the interface or if the interface was declared with the [{{Global}}] [=extended attribute=],
- then the property exists on every object that implements the interface.
- * Otherwise, the property exists solely on the interface’s [=interface prototype object=].
-* The property has attributes { \[[Get]]: |G|, \[[Set]]: |S|, \[[Enumerable]]: <emu-val>true</emu-val>, \[[Configurable]]: |configurable| },
- where:
- * |configurable| is <emu-val>false</emu-val> if the attribute was
- declared with the [{{Unforgeable}}] extended attribute and <emu-val>true</emu-val> otherwise;
- * |G| is the [=attribute getter=] created given the attribute, the interface,
- and the [=relevant Realm=] of the object that is the location of the property; and
- * |S| is the [=attribute setter=] created given the attribute, the interface,
- and the [=relevant Realm=] of the object that is the location of the property.
+[=Static attributes=] are exposed on the [=interface object=].
I guess also another "must" for what to do with globals.
> -* The location of the property is determined as follows:
- * If the operation is [=static operations|static=], then
- the property exists on the [=interface object=].
- * Otherwise, if the operation is [=unforgeable=] on
- the interface or if the interface was declared with the [{{Global}}] [=extended attribute=],
- then the property exists on every object that implements the interface.
- * Otherwise, the property exists solely on the interface’s [=interface prototype object=].
-* The property has attributes
- { \[[Writable]]: |B|, \[[Enumerable]]: <emu-val>true</emu-val>, \[[Configurable]]: |B| },
- where |B| is <emu-val>false</emu-val> if the operation is
- [=unforgeable=] on the interface,
- and <emu-val>true</emu-val> otherwise.
-* The value of the property is the result of [=creating an operation function=] given the operation,
- the interface, and the [=relevant Realm=] of the object that is the location of the property.
+For each unique [=identifier=] of an [=exposed=] [=operation=] defined on the [=interface=],
+there must exist a corresponding property.
Here you kept the must, although eventually it's unnecessary (and you've been deleting it from previous sections, making these intros just statements of fact---which is good). Probably best to delete it from here too, although you'll need to---like the above---add some temporary musts for unforgeable and globals.
> +
+ 1. Let |operations| be the [=list=] of [=unforgeable=] [=regular operations=] that are [=members=] of |definition|.
+ 1. [=Define the operations=] |operations| of |definition| on |target| given |realm|.
+</div>
+
+<div algorithm>
+ To <dfn>define the operations</dfn> |operations| of [=interface=] or [=namespace=] |definition| on |target|,
+ given [=Realm=] |realm|, run the following steps:
+
+ 1. [=list/For each=] [=operation=] |op| of |operations|:
+ 1. If |op| is not [=exposed=] in |realm|, then [=iteration/continue=].
+ 1. Let |method| be the result of [=creating an operation function=]
+ given |op|, |definition|, and |realm|.
+ 1. Let |modifiable| be <emu-val>false</emu-val> if |op| is [=unforgeable=]
+ and <emu-val>true</emu-val> otherwise.
+ 1. Let |id| be |op|'s [=identifier=].
Tiny nit: the order of this step and the next is switched here vs. for attributes. Might as well make them consistent.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/heycam/webidl/pull/494#pullrequestreview-95606373
Received on Saturday, 10 February 2018 03:05:03 UTC