- From: Domenic Denicola <notifications@github.com>
- Date: Tue, 03 May 2022 14:32:31 -0700
- To: whatwg/webidl <webidl@noreply.github.com>
- Cc: Subscribed <subscribed@noreply.github.com>
- Message-ID: <whatwg/webidl/pull/1138/review/961091398@github.com>
@domenic commented on this pull request.
I didn't do a full review of setlike but there might be some stuff to copy over.
> The type of the values is given in the angle
brackets of the setlike declaration. Values are required to be unique.
-The [=set entries=] of an object
-implementing a [=setlike=] interface is empty at
-the of the object’s creation. Prose accompanying the interface can
-describe how the [=set entries=]
-of an object change.
+Specification authors can modify the contents of the [=map entries=],
```suggestion
Specification authors can modify the contents of the [=set entries=],
```
> @@ -11422,6 +11418,32 @@ and its value is an object called a <dfn id="dfn-legacy-factory-function" export
which allows creation of objects that implement the interface.
The characteristics of a legacy factory function are described in [[#legacy-factory-functions]].
+<div algorithm>
+ Some interface operations are defined directly over ECMAScript values,
I don't think "interface operations" is right here, because both "interface" and "operation" have specific meanings in Web IDL which do not match the maplike/setlike functions.
I think an appropriate introduction would be something like
> Some ECMAScript methods defined in this section will perform an implementation check in their opening steps, to ensure they're being called on the correct kind of object and that the method is valid to call from the current context.
> @@ -4415,15 +4421,14 @@ An [=interface=] can be declared to be
</pre>
Objects implementing an interface that is declared to be maplike
-represent an ordered list of key–value pairs known as its <dfn id="dfn-map-entries" export>map entries</dfn>.
+represent an [=ordered map=] of key–value pairs, initially empty,
+known as its <dfn id="dfn-map-entries" export>map entries</dfn>.
```suggestion
known as that object's <dfn id="dfn-map-entries" export>map entries</dfn>.
```
here and below. (To make it clear _each platform object implementing a maplike interface_ owns a map entries, not the maplike interface itself.)
> |A|’s [=interface prototype object=]
with attributes { \[[Writable]]: <emu-val>true</emu-val>, \[[Enumerable]]: <emu-val>false</emu-val>, \[[Configurable]]: <emu-val>true</emu-val> }
and whose value is the [=function object=] that is the value of
-the {{@@iterator}} property.
+the <code>entries</code> property.
+
+<div algorithm>
+ To <dfn>create a map iterator</dfn>
+ from a [=/map=] |map|
+ and a |kind| ("key+value", "key", or "value"):
String markup. Also I find the parens a bit confusing so I'd say ``|kind| which is either "`key+value`", "`key`", or "`value`"``
String markup issue recurs 3 more times in this algorithm.
> |A|’s [=interface prototype object=]
with attributes { \[[Writable]]: <emu-val>true</emu-val>, \[[Enumerable]]: <emu-val>false</emu-val>, \[[Configurable]]: <emu-val>true</emu-val> }
and whose value is the [=function object=] that is the value of
-the {{@@iterator}} property.
+the <code>entries</code> property.
+
+<div algorithm>
+ To <dfn>create a map iterator</dfn>
+ from a [=/map=] |map|
+ and a |kind| ("key+value", "key", or "value"):
+
+ 1. Let |closure| be a new [=/Abstract Closure=] with no parameters
+ that captures |map| and |kind| and performs the following steps when called:
+ 1. [=map/For each=] |key|→|value| of |map|'s [=map entries=]:
```suggestion
1. [=map/For each=] |key| → |value| of |map|'s [=map entries=]:
```
> |A|’s [=interface prototype object=]
with attributes { \[[Writable]]: <emu-val>true</emu-val>, \[[Enumerable]]: <emu-val>false</emu-val>, \[[Configurable]]: <emu-val>true</emu-val> }
and whose value is the [=function object=] that is the value of
-the {{@@iterator}} property.
+the <code>entries</code> property.
+
+<div algorithm>
+ To <dfn>create a map iterator</dfn>
+ from a [=/map=] |map|
+ and a |kind| ("key+value", "key", or "value"):
+
+ 1. Let |closure| be a new [=/Abstract Closure=] with no parameters
+ that captures |map| and |kind| and performs the following steps when called:
+ 1. [=map/For each=] |key|→|value| of |map|'s [=map entries=]:
|map| is a map, which does not have map entries. I think you just want to iterate over |map|?
> |A|’s [=interface prototype object=]
with attributes { \[[Writable]]: <emu-val>true</emu-val>, \[[Enumerable]]: <emu-val>false</emu-val>, \[[Configurable]]: <emu-val>true</emu-val> }
and whose value is the [=function object=] that is the value of
-the {{@@iterator}} property.
+the <code>entries</code> property.
+
+<div algorithm>
+ To <dfn>create a map iterator</dfn>
+ from a [=/map=] |map|
+ and a |kind| ("key+value", "key", or "value"):
+
+ 1. Let |closure| be a new [=/Abstract Closure=] with no parameters
+ that captures |map| and |kind| and performs the following steps when called:
+ 1. [=map/For each=] |key|→|value| of |map|'s [=map entries=]:
+ 1. Set |key| and |value| to each [=converted to an ECMAScript value=].
+ 1. If |kind| is "key", let |result| be |key|.
+ 1. Else if |kind| is "value", let |result| be |value|.
+ 1. Else, let |result| be [$CreateArrayFromList$](« |key|, |value| »)
```suggestion
1. Else, let |result| be [$CreateArrayFromList$](« |key|, |value| »).
```
> |A|’s [=interface prototype object=]
with the following characteristics:
* The property has attributes { \[[Writable]]: <emu-val>true</emu-val>, \[[Enumerable]]: <emu-val>false</emu-val>, \[[Configurable]]: <emu-val>true</emu-val> }.
-* The value of the property is a [=built-in function object=] that [=forwards to the internal map object|forwards that name to the internal map object=].
+* The value of the property is a [=built-in function object=]
+ whose behavior when invoked is as follows:
+
+ <div algorithm="to invoke the entries method of Maps">
+ 1. Let |O| be the <emu-val>this</emu-val> value, [=implementation-checked=] against <var ignore>a</var> with identifier "<code>entries</code>" and type "<code>method</code>".
+ 1. Let |map| be |O|'s [=map entries=].
+ 1. Return the result of [=creating a map iterator=] from |map| with kind "key+value".
String markup
>
-* The property has attributes
- { \[[Writable]]: <emu-val>true</emu-val>, \[[Enumerable]]: <emu-val>false</emu-val>, \[[Configurable]]: <emu-val>true</emu-val> }.
-* <div algorithm="to invoke the get and has methods of Maps">
+ <div algorithm="to invoke the keys method of Maps">
+ 1. Let |O| be the <emu-val>this</emu-val> value, [=implementation-checked=] against <var ignore>a</var> with identifier "<code>keys</code>" and type "<code>method</code>".
+ 1. Let |map| be |O|'s [=map entries=].
+ 1. Return the result of [=creating a map iterator=] from |map| with kind "key".
String markup
> - 1. Return [=?=] <a abstract-op>Call</a>(|function|, |map|, «|key|»).
+
+<h5 id="es-map-values">values</h5>
+
+There must exist a <code class="idl">values</code> data property on
+|A|’s [=interface prototype object=]
+with the following characteristics:
+
+* The property has attributes { \[[Writable]]: <emu-val>true</emu-val>, \[[Enumerable]]: <emu-val>false</emu-val>, \[[Configurable]]: <emu-val>true</emu-val> }.
+* The value of the property is a [=built-in function object=]
+ whose behavior when invoked is as follows:
+
+ <div algorithm="to invoke the values method of Maps">
+ 1. Let |O| be the <emu-val>this</emu-val> value, [=implementation-checked=] against <var ignore>a</var> with identifier "<code>values</code>" and type "<code>method</code>".
+ 1. Let |map| be |O|'s [=map entries=].
+ 1. Return the result of [=creating a map iterator=] from |map| with kind "value".
String markup
>
* The property has attributes { \[[Writable]]: <emu-val>true</emu-val>, \[[Enumerable]]: <emu-val>false</emu-val>, \[[Configurable]]: <emu-val>true</emu-val> }.
-* The value of the property is a [=built-in function object=] that <a lt="forwards to the internal map object">forwards <code class="idl">clear</code> to the internal map object</a>.
+* The value of the property is a [=built-in function object=]
+ whose behavior when invoked is as follows:
+
+ <div algorithm="to invoke the forEach method of Maps">
+ 1. Let |O| be the <emu-val>this</emu-val> value, [=implementation-checked=] against <var ignore>a</var> with identifier "<code>forEach</code>" and type "<code>method</code>".
+ 1. Let |map| be |O|'s [=map entries=].
+ 1. Let |callbackFn| be the first argument passed to the function, or <emu-val>undefined</emu-val> if not supplied.
+ 1. If [$IsCallable$](|callbackFn|) is <emu-val>false</emu-val>, [=ECMAScript/throw=] a {{ECMAScript/TypeError}}.
+ 1. Let |thisArg| be the second argument passed to the function, or <emu-val>undefined</emu-val> if not supplied.
+ 1. [=map/For each=] |key|→|value| of |map|:
+ 1. Let |esKey| and |esValue| be |key| and |value| [=converted to an ECMAScript value=].
+ 1. Perform [=?=] [$Call$](|callbackFn|, |thisArg|, « |esValue|, |esKey|, |O| »)
```suggestion
1. Perform [=?=] [$Call$](|callbackFn|, |thisArg|, « |esValue|, |esKey|, |O| »).
```
> 1. Let |keyArg| be the first argument passed to this function, or <emu-val>undefined</emu-val> if not supplied.
- 1. Let |keyIDL| be the result of [=converted to an IDL value|converting=] |keyArg| to an IDL value of type |keyType|.
- 1. Let |key| be the result of [=converted to ECMAScript values|converting=] |keyIDL| to an ECMAScript value.
- 1. Return [=?=] <a abstract-op>Call</a>(|function|, |map|, «|key|»).
+ 1. Let |key| be |keyArg| [=converted to an IDL value=] of type |keyType|.
+ 1. If |key| is -0, set |key| to +0.
+ 1. If |map|[|key|] [=map/exists=], then return <emu-val>true</emu-val>,
```suggestion
1. If |map|[|key|] [=map/exists=], then return <emu-val>true</emu-val>;
```
> |A|’s [=interface prototype object=]
with the following characteristics:
* The property has attributes { \[[Writable]]: <emu-val>true</emu-val>, \[[Enumerable]]: <emu-val>false</emu-val>, \[[Configurable]]: <emu-val>true</emu-val> }.
-* The value of the property is a [=built-in function object=] that [=forwards to the internal map object|forwards that name to the internal map object=].
+* The value of the property is a [=built-in function object=]
+ whose behavior when invoked is as follows:
+
+ <div algorithm="to invoke the entries method of Maps">
+ 1. Let |O| be the <emu-val>this</emu-val> value, [=implementation-checked=] against <var ignore>a</var> with identifier "<code>entries</code>" and type "<code>method</code>".
+ 1. Let |map| be |O|'s [=map entries=].
+ 1. Return the result of [=creating a map iterator=] from |map| with kind "key+value".
+ </div>
The value of the [=function objects=]’ <code class="idl">length</code> properties is the Number value <emu-val>0</emu-val>.
Here and elsewhere, it should be
```suggestion
The value of the [=function object=]'s <code class="idl">length</code> property is the Number value <emu-val>0</emu-val>.
```
This unfortunately got copied into a bunch of other places instead of the other places' singular version getting copied here :)
> 1. Let |valueArg| be the second argument passed to this function, or <emu-val>undefined</emu-val> if not supplied.
- 1. Let |keyIDL| be the result of [=converted to an IDL value|converting=] |keyArg| to an IDL value of type |keyType|.
- 1. Let |valueIDL| be the result of [=converted to an IDL value|converting=] |valueArg| to an IDL value of type |valueType|.
- 1. Let |key| be the result of [=converted to ECMAScript values|converting=] |keyIDL| to an ECMAScript value.
- 1. Let |value| be the result of [=converted to ECMAScript values|converting=] |valueIDL| to an ECMAScript value.
- 1. Perform [=?=] <a abstract-op>Call</a>(|function|, |map|, «|key|, |value|»).
- 1. Return |O|.
+ 1. Let |value| be |valueArg| [=converted to an IDL value=] of type |valueType|.
+ 1. [=map/Set=] |map|[|key|] to |value|.
+ 1. Return |value| [=converted to an ECMAScript value=].
This looks like an unintentional normative change. You need to return O.
> @@ -11422,6 +11420,32 @@ and its value is an object called a <dfn id="dfn-legacy-factory-function" export
which allows creation of objects that implement the interface.
The characteristics of a legacy factory function are described in [[#legacy-factory-functions]].
+<div algorithm>
+ Some interface operations are defined directly over ECMAScript values,
+ rather than IDL or other spec-internal types.
+ These algorithms need to perform an [=implementation check=]
+ as their opening steps,
+ to ensure they're being called on the correct kind of object
+ and that the operation is valid to call from the given script.
+
+ To <dfn local-lt="implementation check|implementation-check">implementation-check an object</dfn> |esValue|
+ against the interface |interface|,
+ with the identifier |name|
+ and the type |type|:
+
+ 1. Let |object| to [=?=] [$ToObject$](|esValue|).
+ 1. If |object| is a [=platform object=],
So, |object| is an ECMAScript object right now, so it will never be a platform object. You need to instead convert it to an ECMAScript object before doing "is a platform object" and "implements" checks. That can replace the ToObject() check.
This helps most of your call sites because then you have the platform object, which has a map entries. (Right now your call sites are looking at the map entries of the ECMAScript object, which does not have any map entries.)
However it hurts call sites where you need to pass the value back to ECMAScript code, such as the return value of `set()`. For those you'd either need to convert back or preserve the original ECMAScript value.
> </div>
The value of the [=function object=]’s <code class="idl">length</code> property is the Number value <emu-val>2</emu-val>.
The value of the [=function object=]’s <code class="idl">name</code> property is the String value "<code>set</code>".
+Note: If the interface does declare a <code class=idl>set</code> method,
+it should similarly map a -0 key to +0,
+and must return the value that was set.
Nah it should return [=this=].
> @@ -72,9 +72,11 @@ urlPrefix: https://tc39.es/ecma262/; spec: ECMA-262
text: %Function.prototype%; url: sec-properties-of-the-function-prototype-object
text: %IteratorPrototype%; url: sec-%25iteratorprototype%25-object
text: %Map.prototype%; url: sec-properties-of-the-map-prototype-object
+ text: %MapIteratorPrototype%; url: sec-%mapiteratorprototype%-object
Need %25 here and below to avoid conformance errors, it looks like.
>
-For both of <code class="idl">add</code> and <code class="idl">delete</code>, if:
+If |A| does not declare a [=member=] with identifier "<code>add</code>",
+and |A| was declared with a read–write maplike declaration,
```suggestion
and |A| was declared with a read–write setlike declaration,
```
> - * the type "<code>method</code>".
- 1. If |O| does not [=implement=] <var ignore>A</var>, then [=ECMAScript/throw=] a {{ECMAScript/TypeError}}.
- 1. Let |set| be the {{ECMAScript/Set}} object that is the value of |O|’s \[[BackingSet]] [=internal slot=].
- 1. Let |type| be the value type specified in the [=setlike declaration=].
- 1. Let |function| be [=?=] <a abstract-op>Get</a>(|set|, |name|).
- 1. Let |arg| be the first argument passed to this function, or <emu-val>undefined</emu-val> if not supplied.
- 1. Let |idlValue| be the result of [=converted to an IDL value|converting=] |arg| to an IDL value of type |type|.
- 1. Let |value| be the result of [=converted to ECMAScript values|converting=] |idlValue| to an ECMAScript value.
- 1. Let |result| be [=?=] <a abstract-op>Call</a>(|function|, |set|, «|value|»).
- 1. If |name| is "delete", then return |result|.
- 1. Otherwise, return |O|.
+
+<h5 id="es-set-delete">delete</h5>
+
+If |A| does not declare a [=member=] with identifier "<code>delete</code>",
+and |A| was declared with a read–write maplike declaration,
```suggestion
and |A| was declared with a read–write setlike declaration,
```
>
<h5 id="es-set-clear">clear</h5>
-If |A| does not declare a [=member=]
-with a matching identifier, and
-|A| was declared with a read–write setlike declaration,
-then a <code class="idl">clear</code> data property with the following characteristics
-must exist on |A|’s
-[=interface prototype object=]:
+If |A| does not declare a [=member=] with identifier "<code>clear</code>",
+and |A| was declared with a read–write maplike declaration,
```suggestion
and |A| was declared with a read–write setlike declaration,
```
--
Reply to this email directly or view it on GitHub:
https://github.com/whatwg/webidl/pull/1138#pullrequestreview-961091398
You are receiving this because you are subscribed to this thread.
Message ID: <whatwg/webidl/pull/1138/review/961091398@github.com>
Received on Tuesday, 3 May 2022 21:32:44 UTC