From 7291ff7e90c874aea69d33948b1dc9ebb78e488b Mon Sep 17 00:00:00 2001 From: "Mark S. Miller" Date: Mon, 29 Jan 2024 20:32:05 -0800 Subject: [PATCH] fixup: review suggestions --- packages/marshal/NEWS.md | 2 +- packages/marshal/src/rankOrder.js | 2 +- .../test/encodePassable-for-testing.js | 87 +++++++++++++++++ packages/marshal/test/test-encodePassable.js | 95 +++---------------- .../marshal/test/test-string-rank-order.js | 4 +- packages/patterns/NEWS.md | 2 +- .../patterns/test/test-string-key-order.js | 3 +- 7 files changed, 104 insertions(+), 91 deletions(-) create mode 100644 packages/marshal/test/encodePassable-for-testing.js diff --git a/packages/marshal/NEWS.md b/packages/marshal/NEWS.md index 84d8331b55..82a19cc75e 100644 --- a/packages/marshal/NEWS.md +++ b/packages/marshal/NEWS.md @@ -2,7 +2,7 @@ User-visible changes in `@endo/marshal`: # Next release -- JavaScript's relational comparison operators like `<` compare strings by lexicographic UTF16 code unit order, which is exposes an internal representational detail not relevant to the string's meaning as a Unicode string. Previously, `compareRank` and associated functions compared strings using this JavaScript-native comparison. Now `compareRank` and associated functions compare strings by lexicographic Unicode Code Point order. ***This change only affects strings containing so-called supplementary characters, i.e., those whose Unicode character code does not fit in 16 bits***. +- JavaScript's relational comparison operators like `<` compare strings by lexicographic UTF16 code unit order, which exposes an internal representational detail not relevant to the string's meaning as a Unicode string. Previously, `compareRank` and associated functions compared strings using this JavaScript-native comparison. Now `compareRank` and associated functions compare strings by lexicographic Unicode Code Point order. ***This change only affects strings containing so-called supplementary characters, i.e., those whose Unicode character code does not fit in 16 bits***. - This release does not change the `encodePassable` encoding. But now, when we say it is order preserving, we need to be careful about which order we mean. `encodePassable` is rank-order preserving when the encoded strings are compared using `compareRank`. - The key order of strings defined by the @endo/patterns module is still defined to be the same as the rank ordering of those strings. So this release changes key order among strings to also be lexicographic comparison of Unicode Code Points. To accommodate this change, you may need to adapt applications that relied on key-order being the same as JS native order. This could include the use of any patterns expressing key inequality tests, like `M.gte(string)`. - These string ordering changes brings Endo into conformance with any string ordering components of the OCapN standard. diff --git a/packages/marshal/src/rankOrder.js b/packages/marshal/src/rankOrder.js index 18f5982585..6262627b7b 100644 --- a/packages/marshal/src/rankOrder.js +++ b/packages/marshal/src/rankOrder.js @@ -8,7 +8,7 @@ import { /** * @import {Passable, PassStyle} from '@endo/pass-style' - * @import {FullCompare, RankCompare, RankCover} from './types.js' + * @import {FullCompare, RankCompare, RankCover, RankComparison} from './types.js' */ const { entries, fromEntries, setPrototypeOf, is } = Object; diff --git a/packages/marshal/test/encodePassable-for-testing.js b/packages/marshal/test/encodePassable-for-testing.js new file mode 100644 index 0000000000..032383bf95 --- /dev/null +++ b/packages/marshal/test/encodePassable-for-testing.js @@ -0,0 +1,87 @@ +/* eslint-disable no-bitwise, @endo/restrict-comparison-operands */ +import { Fail, q } from '@endo/errors'; + +import { + makeEncodePassable, + makeDecodePassable, +} from '../src/encodePassable.js'; +import { compareRank, makeComparatorKit } from '../src/rankOrder.js'; + +const buffers = { + __proto__: null, + r: [], + '?': [], + '!': [], +}; +const resetBuffers = () => { + buffers.r = []; + buffers['?'] = []; + buffers['!'] = []; +}; +const cursors = { + __proto__: null, + r: 0, + '?': 0, + '!': 0, +}; +const resetCursors = () => { + cursors.r = 0; + cursors['?'] = 0; + cursors['!'] = 0; +}; + +const encodeThing = (prefix, r) => { + buffers[prefix].push(r); + // With this encoding, all things with the same prefix have the same rank + return prefix; +}; + +const decodeThing = (prefix, e) => { + prefix === e || + Fail`expected encoding ${q(e)} to simply be the prefix ${q(prefix)}`; + (cursors[prefix] >= 0 && cursors[prefix] < buffers[prefix].length) || + Fail`while decoding ${q(e)}, expected cursors[${q(prefix)}], i.e., ${q( + cursors[prefix], + )} <= ${q(buffers[prefix].length)}`; + const thing = buffers[prefix][cursors[prefix]]; + cursors[prefix] += 1; + return thing; +}; + +const encodePassableInternal = makeEncodePassable({ + encodeRemotable: r => encodeThing('r', r), + encodePromise: p => encodeThing('?', p), + encodeError: er => encodeThing('!', er), +}); + +export const encodePassableInternal2 = makeEncodePassable({ + encodeRemotable: r => encodeThing('r', r), + encodePromise: p => encodeThing('?', p), + encodeError: er => encodeThing('!', er), + format: 'compactOrdered', +}); + +export const encodePassable = passable => { + resetBuffers(); + return encodePassableInternal(passable); +}; + +export const encodePassable2 = passable => { + resetBuffers(); + return encodePassableInternal2(passable); +}; +export const decodePassableInternal = makeDecodePassable({ + decodeRemotable: e => decodeThing('r', e), + decodePromise: e => decodeThing('?', e), + decodeError: e => decodeThing('!', e), +}); + +export const decodePassable = encoded => { + resetCursors(); + return decodePassableInternal(encoded); +}; + +const compareRemotables = (x, y) => + compareRank(encodeThing('r', x), encodeThing('r', y)); + +export const { comparator: compareFull } = makeComparatorKit(compareRemotables); diff --git a/packages/marshal/test/test-encodePassable.js b/packages/marshal/test/test-encodePassable.js index 7f964f4fcf..d539b7df08 100644 --- a/packages/marshal/test/test-encodePassable.js +++ b/packages/marshal/test/test-encodePassable.js @@ -5,91 +5,20 @@ import test from '@endo/ses-ava/prepare-endo.js'; import { fc } from '@fast-check/ava'; import { Remotable } from '@endo/pass-style'; import { arbPassable } from '@endo/pass-style/tools.js'; -import { Fail, q } from '@endo/errors'; +import { Fail } from '@endo/errors'; -import { - makePassableKit, - makeEncodePassable, - makeDecodePassable, -} from '../src/encodePassable.js'; -import { compareRank, makeComparatorKit } from '../src/rankOrder.js'; +import { makePassableKit, makeEncodePassable } from '../src/encodePassable.js'; +import { compareRank } from '../src/rankOrder.js'; import { unsortedSample } from './marshal-test-data.js'; -const buffers = { - __proto__: null, - r: [], - '?': [], - '!': [], -}; -const resetBuffers = () => { - buffers.r = []; - buffers['?'] = []; - buffers['!'] = []; -}; -const cursors = { - __proto__: null, - r: 0, - '?': 0, - '!': 0, -}; -const resetCursors = () => { - cursors.r = 0; - cursors['?'] = 0; - cursors['!'] = 0; -}; - -const encodeThing = (prefix, r) => { - buffers[prefix].push(r); - // With this encoding, all things with the same prefix have the same rank - return prefix; -}; - -const decodeThing = (prefix, e) => { - prefix === e || - Fail`expected encoding ${q(e)} to simply be the prefix ${q(prefix)}`; - (cursors[prefix] >= 0 && cursors[prefix] < buffers[prefix].length) || - Fail`while decoding ${q(e)}, expected cursors[${q(prefix)}], i.e., ${q( - cursors[prefix], - )} <= ${q(buffers[prefix].length)}`; - const thing = buffers[prefix][cursors[prefix]]; - cursors[prefix] += 1; - return thing; -}; - -const compareRemotables = (x, y) => - compareRank(encodeThing('r', x), encodeThing('r', y)); - -const encodePassableInternal = makeEncodePassable({ - encodeRemotable: r => encodeThing('r', r), - encodePromise: p => encodeThing('?', p), - encodeError: er => encodeThing('!', er), -}); -const encodePassableInternal2 = makeEncodePassable({ - encodeRemotable: r => encodeThing('r', r), - encodePromise: p => encodeThing('?', p), - encodeError: er => encodeThing('!', er), - format: 'compactOrdered', -}); - -export const encodePassable = passable => { - resetBuffers(); - return encodePassableInternal(passable); -}; -const encodePassable2 = passable => { - resetBuffers(); - return encodePassableInternal2(passable); -}; - -const decodePassableInternal = makeDecodePassable({ - decodeRemotable: e => decodeThing('r', e), - decodePromise: e => decodeThing('?', e), - decodeError: e => decodeThing('!', e), -}); - -export const decodePassable = encoded => { - resetCursors(); - return decodePassableInternal(encoded); -}; +import { + encodePassable, + encodePassable2, + encodePassableInternal2, + decodePassable, + decodePassableInternal, + compareFull, +} from './encodePassable-for-testing.js'; test('makePassableKit output shape', t => { const kit = makePassableKit(); @@ -133,8 +62,6 @@ test( (...args) => makePassableKit(...args).encodePassable, ); -const { comparator: compareFull } = makeComparatorKit(compareRemotables); - const asNumber = new Float64Array(1); const asBits = new BigUint64Array(asNumber.buffer); const getNaN = (hexEncoding = '0008000000000000') => { diff --git a/packages/marshal/test/test-string-rank-order.js b/packages/marshal/test/test-string-rank-order.js index 590b59b0f2..b8c5333d44 100644 --- a/packages/marshal/test/test-string-rank-order.js +++ b/packages/marshal/test/test-string-rank-order.js @@ -1,7 +1,7 @@ -import { test } from './prepare-test-env-ava.js'; +import test from '@endo/ses-ava/prepare-endo.js'; import { compareRank } from '../src/rankOrder.js'; -import { encodePassable } from './test-encodePassable.js'; +import { encodePassable } from './encodePassable-for-testing.js'; /** * Essentially a ponyfill for Array.prototype.toSorted, for use before diff --git a/packages/patterns/NEWS.md b/packages/patterns/NEWS.md index 4064edc5e8..dc4bd68a1e 100644 --- a/packages/patterns/NEWS.md +++ b/packages/patterns/NEWS.md @@ -2,7 +2,7 @@ User-visible changes in `@endo/patterns`: # Next release -- JavaScript's relational comparison operators like `<` compare strings by lexicographic UTF16 code unit order, which is exposes an internal representational detail not relevant to the string's meaning as a Unicode string. Previously, `compareKeys` and associated functions compared strings using this JavaScript-native comparison. Now `compareKeys` and associated functions compare strings by lexicographic Unicode Code Point order. ***This change only affects strings containing so-called supplementary characters, i.e., those whose Unicode character code does not fit in 16 bits***. +- JavaScript's relational comparison operators like `<` compare strings by lexicographic UTF16 code unit order, which exposes an internal representational detail not relevant to the string's meaning as a Unicode string. Previously, `compareKeys` and associated functions compared strings using this JavaScript-native comparison. Now `compareKeys` and associated functions compare strings by lexicographic Unicode Code Point order. ***This change only affects strings containing so-called supplementary characters, i.e., those whose Unicode character code does not fit in 16 bits***. - See the NEWS.md of @endo/marshal for more on this change. # v1.2.0 (2024-02-22) diff --git a/packages/patterns/test/test-string-key-order.js b/packages/patterns/test/test-string-key-order.js index c1a1f3d293..28c04c0c6c 100644 --- a/packages/patterns/test/test-string-key-order.js +++ b/packages/patterns/test/test-string-key-order.js @@ -1,6 +1,5 @@ // modeled on test-string-rank-order.js - -import { test } from './prepare-test-env-ava.js'; +import test from '@endo/ses-ava/prepare-endo.js'; import { compareKeys } from '../src/keys/compareKeys.js';