Re: [heycam/webidl] Specify open dictionaries. (#180)

domenic commented on this pull request.



> @@ -7529,6 +7570,82 @@ iterable |iterable| and an iterator getter
 </div>
 
 
+<h4 id="es-open-dictionary">Open dictionaries — dictionary&lt;[|K|,] |V|&gt;</h4>
+
+IDL {{dictionary}}&lt;|K|, |V|&gt; values are represented by
+ECMAScript <emu-val>Object</emu-val> values.
+
+<div algorithm="convert object to open dictionary" id="es-to-open-dictionary">
+    An ECMAScript value |O| is [=converted to an IDL value|converted=] to an IDL <code>{{dictionary}}<|K|, |V|></code> value as follows:
+
+    1.  If [=Type=](|O|) is not <emu-val>Object</emu-val>,
+        <a lt="es throw">throw a <emu-val>TypeError</emu-val></a>.
+    1.  Let |result| be a new empty instance of <code>{{dictionary}}<|K|, |V|></code>.
+    1.  Let |entries| be [=EnumerableOwnProperties=](|O|, "key+value").
+    1.  [=ReturnIfAbrupt=](|entries|).

Use ? notation instead of ReturnIfAbrupt.

> @@ -7529,6 +7564,82 @@ iterable |iterable| and an iterator getter
 </div>
 
 
+<h4 id="es-open-dictionary">Open dictionaries — dictionary&lt;[|K|,] |V|&gt;</h4>
+
+IDL {{dictionary}}&lt;|K|, |V|&gt; values are represented by
+ECMAScript <emu-val>Object</emu-val> values.
+
+<div algorithm="convert object to open dictionary" id="es-to-open-dictionary">
+    An ECMAScript value |O| is [=converted to an IDL value|converted=] to an IDL <code>{{dictionary}}<|K|, |V|></code> value as follows:
+
+    1.  If [=Type=](|O|) is not <emu-val>Object</emu-val>,
+        <a lt="es throw">throw a <emu-val>TypeError</emu-val></a>.
+    1.  Let |result| be a new empty instance of <code>{{dictionary}}<|K|, |V|></code>.
+    1.  Let |entries| be [=EnumerableOwnProperties=](|O|, "key+value").

You should not use EnumerableOwnProperties here, because EnumerableOwnProperties creates an array. This mixes levels between the abstract (list/records/ES spec devices) and the concrete (Arrays), and also makes it unclear whether in subsequent steps when you do [key, value] what the exact semantics are (e.g. is it impacted by changes to Array.prototype or the iteration protocol).

> +    1.  Let |entries| be [=EnumerableOwnProperties=](|O|, "key+value").
+    1.  [=ReturnIfAbrupt=](|entries|).
+    1.  Repeat, for each element [|key|, |value|] of |entries| in [=List=] order:
+        1.  Let |typedKey| be |key| [=converted to an IDL value=] of type |K|.
+        1.  Let |typedValue| be |value| [=converted to an IDL value=] of type |V|.
+        1.  Assert: |typedKey| is not yet a key in |result|.
+        1.  Append to |result| a mapping from |typedKey| to |typedValue|.
+    1.  Return |result|.
+</div>
+
+<div algorithm="convert open dictionary to object" id="open-dictionary-to-es">
+    An IDL <code>{{dictionary}}<…></code> value |D| is [=converted to an
+    ECMAScript value|converted=] to an ECMAScript value as follows:
+
+    1.  Let |result| be a new <emu-val>Object</emu-val> instance
+        created as if by the expression <code>{}</code>.

Web IDL may use this phrase elsewhere, but from now on we should be saying "Let result be ! ObjectCreate(%ObjectPrototype%)".

> +    <table class="data">
+        <thead><th>Key</th><th>Value</th></thead>
+        <tr><td><code>"b"</code></td><td><code>3</code></td></tr>
+        <tr><td><code>"a"</code></td><td><code>4</code></td></tr>
+    </table>
+
+    Open dictionaries only consider [=own property|own=] [=enumerable=] properties, so given an IDL operation
+    <code>dictionary&lt;long> identity(dictionary&lt;long> arg)</code> which
+    returns its argument, the following code passes its assertions:
+
+    <pre highlight="js">
+        let proto = {a: 3, b: 4};
+        let obj = {__proto__: proto, d: 5, c: 6}
+        Object.defineProperty(obj, "e", {value: 7, enumerable: false});
+        let result = identity(obj);
+        assert(result.a === undefined);

Nit: I like using `console.assert` in these examples, since it is an actual function that exists in browsers.

> +        let obj = {__proto__: proto, d: 5, c: 6}
+        Object.defineProperty(obj, "e", {value: 7, enumerable: false});
+        let result = identity(obj);
+        assert(result.a === undefined);
+        assert(result.b === undefined);
+        assert(result.e === undefined);
+        let entries = Object.entries(result);
+        assert(entries[0][0] === "d");
+        assert(entries[0][1] === "5");
+        assert(entries[1][0] === "c");
+        assert(entries[1][1] === "6");
+    </pre>
+
+    Dictionary keys and values can be constrained, although keys can only be
+    constrained among the three string types. The following conversions all
+    throw {{TypeError}}s:

Is this true? I thought the conversion behavior was generally to do things like replacing surrogate pairs or returning 0. In particular I am almost certain that converting an object to a long does not throw a TypeError.

> +        assert(result.a === undefined);
+        assert(result.b === undefined);
+        assert(result.e === undefined);
+        let entries = Object.entries(result);
+        assert(entries[0][0] === "d");
+        assert(entries[0][1] === "5");
+        assert(entries[1][0] === "c");
+        assert(entries[1][1] === "6");
+    </pre>
+
+    Dictionary keys and values can be constrained, although keys can only be
+    constrained among the three string types. The following conversions all
+    throw {{TypeError}}s:
+    <table class="data">
+        <thead><th>Value</th><th>Passed to type</th></thead>
+        <tr><td><code>{"😞": 1}</code></td><td><code>{{dictionary}}&lt;ByteString, long></code></td></tr>

Consider never using `long` in examples, per https://w3ctag.github.io/design-principles/#numeric-types. (`long long` would be more appropriate in this case.)

-- 
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/180#pullrequestreview-2397740