feat(pass-style,marshal): ByteArray, a new binary Passable type#1538
feat(pass-style,marshal): ByteArray, a new binary Passable type#1538
Conversation
mhofman
left a comment
There was a problem hiding this comment.
I think we should try to align copyBytes with the prior art of the Web platform's Blob.
80cc7f2 to
fca467e
Compare
15cbf4f to
a6a6ca0
Compare
4bf3d8c to
ce5f3bf
Compare
ce5f3bf to
947e166
Compare
e24eced to
ac041b2
Compare
b25c593 to
fe561d2
Compare
b32ab45 to
544a213
Compare
1281d0c to
42075f3
Compare
d0f1cd5 to
bc99d9e
Compare
bc99d9e to
0509bcc
Compare
42075f3 to
134bd68
Compare
0509bcc to
07017b0
Compare
4502570 to
fa09e05
Compare
gibson042
left a comment
There was a problem hiding this comment.
This looks mostly good, but I have reservations about treating ByteArray as a Container [of Passables].
|
Wow, @gibson042 , those suggestions were extremely helpful, thanks! PTAL |
gibson042
left a comment
There was a problem hiding this comment.
Thanks! LGTM modulo updating packages/patterns/README.md.
gibson042
left a comment
There was a problem hiding this comment.
Oh, I think this PR should also update arb-passable.js, although accommodations will be necessary to exclude ByteArrays in encodePassable.test.js until they have a defined encoding, e.g.:
-/** @param {typeof import('@fast-check/ava').fc} fc */
-export const makeArbitraries = fc => {
+/**
+ * @param {typeof import('@fast-check/ava').fc} fc
+ * @param {Array<'byteArray'>} [exclusions]
+ */
+export const makeArbitraries = (fc, exclusions = []) => {
const arbString = fc.oneof(fc.string(), fc.fullUnicodeString());
const keyableLeaves = [
fc.constantFrom(null, undefined, false, true),
arbString,
arbString.map(s => Symbol.for(s)),
// primordial symbols and registered lookalikes
fc.constantFrom(
...Object.getOwnPropertyNames(Symbol).flatMap(k => {
const v = Symbol[k];
if (typeof v !== 'symbol') return [];
return [v, Symbol.for(k), Symbol.for(`@@${k}`)];
}),
),
fc.bigInt(),
fc.integer(),
+ ...[fc.uint8Array().map(arr => arr.buffer.transferToImmutable())].filter(
+ () => !exclusions.includes('byteArray'),
+ ),
fc.constantFrom(-0, NaN, Infinity, -Infinity),
fc.record({}),
fc.constantFrom(exampleAlice, exampleBob, exampleCarol),
];|
And to capture my thoughts on the encoding itself: given that we want both shortlex ordering and (to abuse vocabulary somewhat) isomorphic rank-order comparisons of
|
Done |
e2fd1ff to
75ea396
Compare
I just read patterns/README.md carefully, and did not spot the flaw you have in mind. Is this indeed the document you have in mind? What needs updating? |
gibson042
left a comment
There was a problem hiding this comment.
I just read patterns/README.md carefully, and did not spot the flaw you have in mind. Is this indeed the document you have in mind? What needs updating?
Ah, no change is needed now that we acknowledge ByteArray as primitive.
Closes: #1331
Refs: ocapn/ocapn#5 (comment) #2414
Description
Recognize Immutable ArrayBuffers, when also frozen, as the new pass-style
byteArray. Add a branch for each of the dispatches on pass-style that must now understand byte-arrays as well. As of this PR, many of those additional branches are stubbed out with "not yet implemented" errors as placeholders.Security Considerations
The underlying shim implementation, if we're still using that, uses a normal mutable ArrayBuffer internally, but safely encapsulates it so nothing outside the shim can mutate it.
Scaling Considerations
The underlying shim implementation helps us handle bulk binary data in a largely no-copy or minimal-copy manner.
Documentation Considerations
Will add a new pass-style, so everywhere we enumerate all the pass-styles, we'll need to add byte-array.
Testing Considerations
test.failingtest.Compatibility Considerations
Old code that dispatched only on the pass-styles it knew about will not correctly handle passables that contain byte-arrays.
The shim cannot reasonably support TypedArrays of DataViews backed by Immutable ArrayBuffers.
Upgrade Considerations