Skip to content

Commit 7a02799

Browse files
authored
refactor(marshal): compareRankRemotablesTied for rank-cover ops (#3265)
Refs: #2871 Refs: #2883 Refs: #3226 ## Description Splits the rank-comparison refactor out from #2871 so that the codec-admission core of that PR can land in #3226 without depending on this orthogonal change. Introduces `compareRankRemotablesTied` (and its anti-rank twin), which behaves like `compareRank` but treats all remotables as tied for the same rank rather than short-circuiting on the first remotable encountered. Makes this the default `compare` for the rank-cover operations: - `isRankSorted`, `assertRankSorted`, `sortByRank`, `getIndexCover` (commit 1) - `unionRankCovers`, `intersectRankCovers` (commit 2) `compare` becomes optional and is moved to the trailing position on each. `getIndexCover`'s argument order shifts from `(sorted, compare, rankCover)` to `(sorted, rankCover, compare?)`; the only callers are within the Endo monorepo and are updated. `unionRankCovers` and `intersectRankCovers` likewise shift from `(compare, covers)` to `(covers, compare?)`, and `@endo/patterns`'s `patternMatchers.js` (the only caller) is updated. `compareRankRemotablesTied` and `compareAntiRankRemotablesTied` are re-exported from `@endo/marshal`'s `index.js`. The first commit is salvaged from #2871 with original authorship preserved (Mark S. Miller). ### Security Considerations No change to trust boundaries, no new authorities. The comparator change is internal to marshal's rank ordering; callers that pass an explicit `compare` keep their existing behavior. ### Scaling Considerations The tied comparator visits every remotable rather than short-circuiting on the first, so rank-cover operations over inputs containing many remotables do more work per call than the prior `compareRank` default. This matches the intent of the operations (they want a stable cover, not an early exit), and the prior behavior is still available by passing `compareRank` explicitly. ### Documentation Considerations The argument-order changes on `getIndexCover`, `unionRankCovers`, and `intersectRankCovers` are caller-visible. No internal callers remain on the old signatures. External callers, if any, would need to swap argument order; this is captured in the changeset. ### Testing Considerations `packages/marshal/test/rankOrder.test.js` is extended to: - Exercise the new optional/default-arg behavior of all six rank-cover operations. - Resolve two pre-existing TODOs that asked for the `totally orders ranks` and `is transitive` property tests to run against both `compareRank` and `compareRankRemotablesTied`. Both are total preorders, so both properties hold for both, and we now parameterize the tests to exercise each. ### Compatibility Considerations Argument-order shifts on three exported functions (`getIndexCover`, `unionRankCovers`, `intersectRankCovers`) and one new default comparator on six. Captured in a changeset. The `compareRankRemotablesTied` and `compareAntiRankRemotablesTied` exports are additions; nothing is removed. ### Upgrade Considerations No live-system upgrade concerns. The changeset covers the package-level migration for external consumers.
2 parents 0ec70c6 + 337d16a commit 7a02799

5 files changed

Lines changed: 267 additions & 93 deletions

File tree

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
---
2+
'@endo/marshal': minor
3+
'@endo/patterns': patch
4+
---
5+
6+
Add `compareRankRemotablesTied` and `compareAntiRankRemotablesTied`,
7+
which behave like `compareRank` and `compareAntiRank` except that they
8+
treat all remotables as tied for the same rank instead of short-circuiting
9+
the comparison on encountering a remotable. Make the `compare` parameter
10+
optional on `isRankSorted`, `assertRankSorted`, `sortByRank`,
11+
`getIndexCover`, `unionRankCovers`, and `intersectRankCovers`,
12+
defaulting to `compareRankRemotablesTied`.
13+
14+
The following parameter orders change so the optional comparator lands
15+
at the end:
16+
17+
- `getIndexCover(sorted, compare, rankCover)`
18+
`getIndexCover(sorted, rankCover, compare?)`
19+
- `unionRankCovers(compare, covers)`
20+
`unionRankCovers(covers, compare?)`
21+
- `intersectRankCovers(compare, covers)`
22+
`intersectRankCovers(covers, compare?)`
23+
24+
These are breaking changes to helpers that have no known external
25+
callers within the Endo monorepo. Within the monorepo, the only
26+
callers of `unionRankCovers` and `intersectRankCovers` are in
27+
`@endo/patterns`'s `patternMatchers.js`, which has been updated to
28+
match the new parameter order.
29+
30+
Salvaged from https://github.com/endojs/endo/pull/2871 so that the
31+
codec-admission core of that PR (now in #3226) can land independently
32+
of this rank-comparison refactor.

packages/marshal/index.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,11 @@ export {
1919
compareByCodePoints,
2020
assertRankSorted,
2121
compareRank,
22+
compareRankRemotablesTied,
2223
isRankSorted,
2324
sortByRank,
2425
compareAntiRank,
26+
compareAntiRankRemotablesTied,
2527
makeFullOrderComparatorKit,
2628
getPassStyleCover,
2729
intersectRankCovers,

packages/marshal/src/rankOrder.js

Lines changed: 51 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -359,12 +359,28 @@ harden(comparatorMirrorImage);
359359
export const { comparator: compareRank, antiComparator: compareAntiRank } =
360360
makeComparatorKit();
361361

362+
/**
363+
* Like `compareRank` and `compareAntiRank` and unlike `fullCompare`,
364+
* `compareRankRemotablesTied` and `compareAntiRankRemotablesTied`
365+
* considers all remotables tied for the same rank.
366+
* Unlike `compareRank` and `compareAntiRank`,
367+
* `compareRankRemotablesTied` and `compareAntiRankRemotablesTied`
368+
* do not short circuit on encounting remotables.
369+
*/
370+
export const {
371+
comparator: compareRankRemotablesTied,
372+
antiComparator: compareAntiRankRemotablesTied,
373+
} = makeComparatorKit((_x, _y) => 0);
374+
362375
/**
363376
* @param {Passable[]} passables
364-
* @param {RankCompare} compare
377+
* @param {RankCompare} [compare]
365378
* @returns {boolean}
366379
*/
367-
export const isRankSorted = (passables, compare) => {
380+
export const isRankSorted = (
381+
passables,
382+
compare = compareRankRemotablesTied,
383+
) => {
368384
const subMemoOfSorted = memoOfSorted.get(compare);
369385
assert(subMemoOfSorted !== undefined);
370386
if (subMemoOfSorted.has(passables)) {
@@ -383,9 +399,9 @@ harden(isRankSorted);
383399

384400
/**
385401
* @param {Passable[]} sorted
386-
* @param {RankCompare} compare
402+
* @param {RankCompare} [compare]
387403
*/
388-
export const assertRankSorted = (sorted, compare) =>
404+
export const assertRankSorted = (sorted, compare = compareRankRemotablesTied) =>
389405
isRankSorted(sorted, compare) ||
390406
// TODO assert on bug could lead to infinite recursion. Fix.
391407
// eslint-disable-next-line no-use-before-define
@@ -395,10 +411,10 @@ harden(assertRankSorted);
395411
/**
396412
* @template {Passable} T
397413
* @param {Iterable<T>} passables
398-
* @param {RankCompare} compare
414+
* @param {RankCompare} [compare]
399415
* @returns {T[]}
400416
*/
401-
export const sortByRank = (passables, compare) => {
417+
export const sortByRank = (passables, compare = compareRankRemotablesTied) => {
402418
/** @type {T[]} mutable for in-place sorting, but with hardened elements */
403419
let unsorted;
404420
if (Array.isArray(passables)) {
@@ -439,12 +455,17 @@ harden(sortByRank);
439455
* https://en.wikipedia.org/wiki/Binary_search_algorithm#Procedure_for_finding_the_leftmost_element
440456
*
441457
* @param {Passable[]} sorted
442-
* @param {RankCompare} compare
443458
* @param {Passable} key
444-
* @param {("leftMost" | "rightMost")=} bias
459+
* @param {RankCompare} [compare]
460+
* @param {"leftMost" | "rightMost"} [bias]
445461
* @returns {number}
446462
*/
447-
const rankSearch = (sorted, compare, key, bias = 'leftMost') => {
463+
const rankSearch = (
464+
sorted,
465+
key,
466+
compare = compareRankRemotablesTied,
467+
bias = 'leftMost',
468+
) => {
448469
assertRankSorted(sorted, compare);
449470
let left = 0;
450471
let right = sorted.length;
@@ -463,14 +484,19 @@ const rankSearch = (sorted, compare, key, bias = 'leftMost') => {
463484

464485
/**
465486
* @param {Passable[]} sorted
466-
* @param {RankCompare} compare
467487
* @param {RankCover} rankCover
488+
* @param {RankCompare} [compare] which rank comparison function to use.
489+
* Default to `compareRankRemotablesTied`.
468490
* @returns {IndexCover}
469491
*/
470-
export const getIndexCover = (sorted, compare, [leftKey, rightKey]) => {
492+
export const getIndexCover = (
493+
sorted,
494+
[leftKey, rightKey],
495+
compare = compareRankRemotablesTied,
496+
) => {
471497
assertRankSorted(sorted, compare);
472-
const leftIndex = rankSearch(sorted, compare, leftKey, 'leftMost');
473-
const rightIndex = rankSearch(sorted, compare, rightKey, 'rightMost');
498+
const leftIndex = rankSearch(sorted, leftKey, compare, 'leftMost');
499+
const rightIndex = rankSearch(sorted, rightKey, compare, 'rightMost');
474500
return [leftIndex, rightIndex];
475501
};
476502
harden(getIndexCover);
@@ -524,11 +550,15 @@ const maxRank = (compare, a, b) => (compare(a, b) >= 0 ? a : b);
524550
const minRank = (compare, a, b) => (compare(a, b) <= 0 ? a : b);
525551

526552
/**
527-
* @param {RankCompare} compare
528553
* @param {RankCover[]} covers
554+
* @param {RankCompare} [compare] which rank comparison function to use.
555+
* Default to `compareRankRemotablesTied`.
529556
* @returns {RankCover}
530557
*/
531-
export const unionRankCovers = (compare, covers) => {
558+
export const unionRankCovers = (
559+
covers,
560+
compare = compareRankRemotablesTied,
561+
) => {
532562
/**
533563
* @param {RankCover} a
534564
* @param {RankCover} b
@@ -543,11 +573,15 @@ export const unionRankCovers = (compare, covers) => {
543573
harden(unionRankCovers);
544574

545575
/**
546-
* @param {RankCompare} compare
547576
* @param {RankCover[]} covers
577+
* @param {RankCompare} [compare] which rank comparison function to use.
578+
* Default to `compareRankRemotablesTied`.
548579
* @returns {RankCover}
549580
*/
550-
export const intersectRankCovers = (compare, covers) => {
581+
export const intersectRankCovers = (
582+
covers,
583+
compare = compareRankRemotablesTied,
584+
) => {
551585
/**
552586
* @param {RankCover} a
553587
* @param {RankCover} b

0 commit comments

Comments
 (0)