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

@syg approved this pull request.

lgtm modulo the [[Realm]] question for ABs.

>  
-    To <dfn id="dfn-detach" for="ArrayBuffer" export>detach</dfn> an {{ArrayBuffer}}, these steps must be followed:
+    1.  Assert: if the type is not {{DataView}}, then |bytes|'s [=byte sequence/length=] [=modulo=]
+        the [=element size=] of that type is 0.
+    1.  Let |arrayBuffer| be the result of [=ArrayBuffer/creating=] an {{ArrayBuffer}} from |bytes|
+        in |realm|.
+    1.  Let |constructor| be the appropriate constructor from |realm|.\[[Intrinsics]] for the type
+        of {{ArrayBufferView}} being created.
+    1.  Return [=!=] [$TypedArrayCreate$](|constructor|, « |arrayBuffer| »).

I had not realized that ArrayBufferView also parameterizes over DataView. TypedArrayCreate doesn't work for DataView. I guess you need a branch here that calls the DataView constructor.

>  
-    1.  Let |O| be the ECMAScript object that is the {{ArrayBuffer}}.
-    1.  Perform [=!=] <a abstract-op>DetachArrayBuffer</a>(|O|).
+    1.  Assert: |bytes|'s [=byte sequence/length=] ≤ |arrayBuffer|.\[[ArrayBufferByteLength]]
+        &minus; |startingOffset|.
+    1.  For |i| in [=the range=] |startingOffset| to |startingOffset| + |bytes|'s [=byte
+        sequence/length=] &minus; 1, inclusive, perform [=!=] [$SetValueInBuffer$](|arrayBuffer|,
+        |i|, Uint8, |bytes|[|i| - |startingOffset|], false, Unordered).

Same as above, I'd pass true instead of false here.

> +    To <dfn id="dfn-get-buffer-source-copy" export lt="get a copy of the buffer source">get a copy of the bytes held by the buffer source</dfn>
+    given a {{BufferSource}} |bufferSource|:
+
+    1.  Let |arrayBuffer| be |bufferSource|.
+    1.  Let |offset| be 0.
+    1.  Let |length| be 0.
+    1.  If |bufferSource| has a \[[ViewedArrayBuffer]] [=internal slot=], then:
+        1.  Set |arrayBuffer| to |bufferSource|.\[[ViewedArrayBuffer]].
+        1.  Set |offset| to |bufferSource|.\[[ByteOffset]].
+        1.  Set |length| to |bufferSource|.\[[ByteLength]].
+    1.  Otherwise, set |length| to |bufferSource|.\[[ArrayBufferByteLength]].
+    1.  If [=!=] [$IsDetachedBuffer$](|arrayBuffer|) is true, then return the empty
+        [=byte sequence=].
+    1.  Let |bytes| be a new [=byte sequence=] of [=byte sequence/length=] equal to |length|.
+    1.  For |i| in [=the range=] |offset| to |offset| + |length| &minus; 1, inclusive, set
+        |bytes|[|i| &minus; |offset|] to [=!=] [$GetValueFromBuffer$](|arrayBuffer|, |i|, Uint8,

On second read, I'd pass true instead of false here, which basically means "aligned"; see this note for an explanation. For byte-wise writes I don't think it's actually observable, but it's editorially clearer IMO.

> +        <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
+cannot overlap with other modifications, and then give the new {{ArrayBuffer}} instance to author
+code as necessary.
+
+<div algorithm>
+    To <dfn id="dfn-detach" for="ArrayBuffer" export>detach</dfn> an {{ArrayBuffer}} |arrayBuffer|:
+
+    1.  Asssert: [=!=] [$IsSharedArrayBuffer$](|arrayBuffer|) is false.

```suggestion
    1.  Assert: [=!=] [$IsSharedArrayBuffer$](|arrayBuffer|) is false.
```

> +<div algorithm>
+    To <dfn id="dfn-detach" for="ArrayBuffer" export>detach</dfn> an {{ArrayBuffer}} |arrayBuffer|:
+
+    1.  Asssert: [=!=] [$IsSharedArrayBuffer$](|arrayBuffer|) is false.
+    1.  Perform [=?=] [$DetachArrayBuffer$](|arrayBuffer|).
+
+    <p class="note">This will throw an exception if |arrayBuffer| has an \[[ArrayBufferDetachKey]]
+    that is not undefined, such as a {{Memory|WebAssembly.Memory}}'s {{Memory/buffer}}.
+    [[WASM-JS-API-1]]
+</div>
+
+<div algorithm>
+    To <dfn for="ArrayBuffer" export>transfer</dfn> an {{ArrayBuffer}} |arrayBuffer|, optionally
+    given a [=Realm=] |realm|:
+
+    1.  Asssert: [=!=] [$IsSharedArrayBuffer$](|arrayBuffer|) is false.

```suggestion
    1.  Assert: [=!=] [$IsSharedArrayBuffer$](|arrayBuffer|) is false.
```

> +
+    <p class="note">This will throw an exception if |arrayBuffer| has an \[[ArrayBufferDetachKey]]
+    that is not undefined, such as a {{Memory|WebAssembly.Memory}}'s {{Memory/buffer}}.
+    [[WASM-JS-API-1]]
+</div>
+
+<div algorithm>
+    To <dfn for="ArrayBuffer" export>transfer</dfn> an {{ArrayBuffer}} |arrayBuffer|, optionally
+    given a [=Realm=] |realm|:
+
+    1.  Asssert: [=!=] [$IsSharedArrayBuffer$](|arrayBuffer|) is false.
+    1.  Assert: [=!=] [$IsDetachedBuffer$](|arrayBuffer|) is false.
+    1.  Let |arrayBufferData| be |arrayBuffer|.\[[ArrayBufferData]].
+    1.  Let |arrayBufferByteLength| be |arrayBuffer|.\[[ArrayBufferByteLength]].
+    1.  Perform [=?=] [$DetachArrayBuffer$](|arrayBuffer|).
+    1.  If |realm| is not given, let |realm| be |arrayBuffer|.\[[Realm]].

I don't think ABs have a [[Realm]], only functions do. 

-- 
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-675673921

Received on Thursday, 3 June 2021 22:44:30 UTC