Re: [heycam/webidl] Add and improve operations on BufferSources (#987)

@TimothyGu approved this pull request.

> On the others, it occured to me that we have two directions here: super-polymorphism, where everything is defined on BufferSources, or something more true to the model where ArrayBuffers are special. I was curious for your thoughts.

Here's an alternative: super-polymorphism for read-like operations (byte length, is detached, get a copy), ArrayBuffers are special for write-like operations (create, write, detach, transfer). The name "BufferSource" is fairly explicit about being the _source_, so I don't think it's worthwhile to make write operations work for any BufferSource.

Come to think of it, this layout is what you have already!

> +    1.  Let |esArrayBuffer| be be the result of [=converted to an ECMAScript value|converting=]
+        |arrayBuffer| to an ECMAScript value.
+    1.  Assert: |bytes|'s [=byte sequence/length=] ≤ |esArrayBuffer|.\[[ArrayBufferByteLength]]
+        − |startingOffset|.
+    1.  For |i| in [=the range=] |startingOffset| to |startingOffset| + |bytes|'s [=byte
+        sequence/length=] − 1, inclusive, perform [=!=] [$SetValueInBuffer$](|esArrayBuffer|,
+        |i|, Uint8, |bytes|[|i| - |startingOffset|], true, Unordered).
+</div>
+
+<div algorithm="ArrayBufferView/write">
+    To <dfn export for="ArrayBufferView">write</dfn> a [=byte sequence=] |bytes| into an
+    {{ArrayBufferView}} |view|, optionally given a
+    <dfn export for="ArrayBufferView/write">|startingOffset|</dfn> (default 0):
+
+    1.  Let |esView| be be the result of [=converted to an ECMAScript value|converting=]
+        |arrayBuffer| to an ECMAScript value.

```suggestion
        |view| to an ECMAScript value.
```

> @@ -76,6 +76,11 @@ urlPrefix: https://w3c.github.io/mediacapture-main/; spec: MEDIACAPTURE-STREAMS
 urlPrefix: http://www.unicode.org/glossary/; spec: UNICODE
     type: dfn
         text: Unicode scalar value; url: unicode_scalar_value
+urlPrefix: https://webassembly.github.io/spec/js-api/; spec: WASM-JS-API-1
+    type: interface
+        text: Memory; url: #memory

The reference is unfortunately quite generic… Is it possible to specify `text: WebAssembly.Memory` instead? Or add a `for: WebAssembly` to line 79.

> +    To <dfn id="dfn-detach" for="ArrayBuffer" export>detach</dfn> an {{ArrayBuffer}} |arrayBuffer|:
+
+    1.  Let |esArrayBuffer| be be the result of [=converted to an ECMAScript value|converting=]
+        |arrayBuffer| to an ECMAScript value.
+    1.  Assert: [=!=] [$IsSharedArrayBuffer$](|esArrayBuffer|) is false.
+    1.  Perform [=?=] [$DetachArrayBuffer$](|esArrayBuffer|).
+
+    <p class="note">This will throw an exception if |esArrayBuffer| has an \[[ArrayBufferDetachKey]]
+    that is not undefined, such as is the case with the value of {{Memory|WebAssembly.Memory}}'s
+    {{Memory/buffer}} attribute. [[WASM-JS-API-1]]
+
+    <p class="note">Detaching a buffer that is already detached is a no-op.
+</div>
+
+<div algorithm>
+    A {{BufferSource}} |bufferSource| <dfn for="BufferSource" export>is detached</dfn> if the

```suggestion
    A {{BufferSource}} |bufferSource| is <dfn for="BufferSource" export>detached</dfn> if the
```

>  </div>
 
+<div algorithm>
+    To <dfn id="dfn-detach" for="ArrayBuffer" export>detach</dfn> an {{ArrayBuffer}} |arrayBuffer|:
+
+    1.  Let |esArrayBuffer| be be the result of [=converted to an ECMAScript value|converting=]
+        |arrayBuffer| to an ECMAScript value.
+    1.  Assert: [=!=] [$IsSharedArrayBuffer$](|esArrayBuffer|) is false.
+    1.  Perform [=?=] [$DetachArrayBuffer$](|esArrayBuffer|).
+
+    <p class="note">This will throw an exception if |esArrayBuffer| has an \[[ArrayBufferDetachKey]]
+    that is not undefined, such as is the case with the value of {{Memory|WebAssembly.Memory}}'s
+    {{Memory/buffer}} attribute. [[WASM-JS-API-1]]
+
+    <p class="note">Detaching a buffer that is already detached is a no-op.

```suggestion
    <p class="note">Detaching a buffer that is already [=BufferSource/detached=] is a no-op.
```

> +    <dfn export for="ArrayBufferView/write">|startingOffset|</dfn> (default 0):
+
+    1.  Assert: |bytes|'s [=byte sequence/length=] ≤ |view|.\[[ByteLength]] &minus;
+        |startingOffset|.
+    1.  Assert: if |view| is not a {{DataView}}, then |bytes|'s [=byte sequence/length=] [=modulo=]
+        the [=element size=] of |view|'s type is 0.
+    1.  [=ArrayBuffer/Write=] |bytes| into |view|.\[[ViewedArrayBuffer]] with
+        <i>[=ArrayBuffer/write/startingOffset=]</i> set to |view|.\[[ByteOffset]] +
+        |startingOffset|.
+</div>
+
+<p class="advisement">Extreme care must be taken when writing specification text that
+[=ArrayBuffer/writes=] into an {{ArrayBuffer}} or {{ArrayBufferView}}, as the underlying data can
+easily be changed by the script author or other APIs at unpredictable times. This is especially true
+if the [{{AllowShared}}] [=extended attribute=] is involved. For the non-shared cases, it is
+recommended to [=ArrayBuffer/transfer=] the {{ArrayBuffer}} first if possible, to ensure the writes

Looks good!

-- 
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/987#pullrequestreview-692328488