Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: implement Set.isSubsetOf() for Node <22, which defaults to builtin implementation whenever possible #1009

Merged
merged 8 commits into from
Nov 7, 2024
23 changes: 23 additions & 0 deletions .github/workflows/tact.yml
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,29 @@ jobs:
- name: Checkout repository
uses: actions/checkout@v3

- name: Setup Node.js 18 for backwards compat tests
uses: actions/setup-node@v3
with:
node-version: 18
# without caching

- name: Backwards compat tests
run: |
# Temporarily ignore engines
yarn config set ignore-engines true
# Install dependencies, gen and build the compiler
yarn install
yarn clean
yarn gen
yarn build
# Test some specific things for backwards compatibility.
# It's important to restrain from using too much of Node.js 22+ features
# until it goes into maintenance LTS state and majority of users catches up
yarn jest -t 'isSubsetOf'
verytactical marked this conversation as resolved.
Show resolved Hide resolved
# Clean-up
yarn cleanall
yarn config delete ignore-engines

- name: Setup Node.js ${{ matrix.node-version }}
uses: actions/setup-node@v3
with:
Expand Down
4 changes: 3 additions & 1 deletion scripts/copy-files.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
import * as fs from "node:fs/promises";
import * as path from "node:path";
import * as glob from "glob";

const cp = async (fromGlob: string, toPath: string) => {
for await (const file of fs.glob(fromGlob)) {
const files = glob.sync(fromGlob);
novusnota marked this conversation as resolved.
Show resolved Hide resolved
for (const file of files) {
await fs.copyFile(file, path.join(toPath, path.basename(file)));
}
};
Expand Down
9 changes: 5 additions & 4 deletions src/types/resolveDescriptors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ import {
import { getRawAST } from "../grammar/store";
import { cloneNode } from "../grammar/clone";
import { crc16 } from "../utils/crc16";
import { isSubsetOf } from "../utils/isSubsetOf";
import { evalConstantExpression } from "../constEval";
import { resolveABIType, intMapFormats } from "./resolveABITypeRef";
import { enabledExternals } from "../config/features";
Expand Down Expand Up @@ -909,13 +910,13 @@ export function resolveDescriptors(ctx: CompilerContext) {
const paramSet = new Set(
a.params.map((typedId) => idText(typedId.name)),
);
if (!paramSet.isSubsetOf(shuffleArgSet)) {
if (!isSubsetOf(paramSet, shuffleArgSet)) {
throwCompilationError(
"asm argument rearrangement must mention all function parameters",
a.loc,
);
}
if (!shuffleArgSet.isSubsetOf(paramSet)) {
if (!isSubsetOf(shuffleArgSet, paramSet)) {
throwCompilationError(
"asm argument rearrangement must mention only function parameters",
a.loc,
Expand Down Expand Up @@ -973,13 +974,13 @@ export function resolveDescriptors(ctx: CompilerContext) {
// mutating functions also return `self` arg (implicitly in Tact, but explicitly in FunC)
retTupleSize += isMutating ? 1 : 0;
const returnValueSet = new Set([...Array(retTupleSize).keys()]);
if (!returnValueSet.isSubsetOf(shuffleRetSet)) {
if (!isSubsetOf(returnValueSet, shuffleRetSet)) {
throwCompilationError(
`asm return rearrangement must mention all return position numbers: [0..${retTupleSize - 1}]`,
a.loc,
);
}
if (!shuffleRetSet.isSubsetOf(returnValueSet)) {
if (!isSubsetOf(shuffleRetSet, returnValueSet)) {
throwCompilationError(
`asm return rearrangement must mention only valid return position numbers: [0..${retTupleSize - 1}]`,
a.loc,
Expand Down
91 changes: 91 additions & 0 deletions src/utils/isSubsetOf.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
import { ReadonlySetLike, isSubsetOf } from "./isSubsetOf";

// Tests are adapted from:
// https://github.com/zloirock/core-js/blob/227a758ef96fa585a66cc1e89741e7d0bb696f48/tests/unit-global/es.set.is-subset-of.js

describe("isSubsetOf", () => {
/* eslint-disable @typescript-eslint/no-explicit-any */
let s1: Set<any>;
let s2: ReadonlySetLike<unknown>;

it("should implement isSubsetOf correctly", () => {
s1 = new Set([1]);
s2 = new Set([1, 2, 3]);
expect(isSubsetOf(s1, s2)).toBe(true);

s1 = new Set([1]);
s2 = new Set([2, 3, 4]);
expect(isSubsetOf(s1, s2)).toBe(false);

s1 = new Set([1, 2, 3]);
s2 = new Set([5, 4, 3, 2, 1]);
expect(isSubsetOf(s1, s2)).toBe(true);

s1 = new Set([1, 2, 3]);
s2 = new Set([5, 4, 3, 2]);
expect(isSubsetOf(s1, s2)).toBe(false);

s1 = new Set([1]);
s2 = createSetLike([1, 2, 3]);
expect(isSubsetOf(s1, s2)).toBe(true);

s1 = new Set([1]);
s2 = createSetLike([2, 3, 4]);
expect(isSubsetOf(s1, s2)).toBe(false);

s1 = new Set([1, 2, 3]);
s2 = createSetLike([5, 4, 3, 2, 1]);
expect(isSubsetOf(s1, s2)).toBe(true);

s1 = new Set([1, 2, 3]);
s2 = createSetLike([5, 4, 3, 2]);
expect(isSubsetOf(s1, s2)).toBe(false);

s1 = new Set([1, 2, 3]);
s2 = new Set([1]);
expect(isSubsetOf(s1, s2)).toBe(false);

s1 = new Set([1, 2, 3]);
s2 = new Set();
expect(isSubsetOf(s1, s2)).toBe(false);

s1 = new Set();
s2 = new Set([1, 2, 3]);
expect(isSubsetOf(s1, s2)).toBe(true);
});
});

// Helper functions are adapted from:
// https://github.com/zloirock/core-js/blob/227a758ef96fa585a66cc1e89741e7d0bb696f48/tests/helpers/helpers.js

function createSetLike<T>(elements: T[]): ReadonlySetLike<T> {
return {
size: elements.length,
has(value: T): boolean {
return includes(elements, value);
},
keys(): Iterator<T> {
return createIterator(elements);
},
};
}

function includes<T>(target: T[], wanted: T) {
return target.some((element) => element === wanted);
}

function createIterator<T>(elements: T[]): Iterator<T> {
let index = 0;
const iterator = {
called: false,
/* eslint-disable @typescript-eslint/no-explicit-any */
next(): IteratorResult<any> {
iterator.called = true;
return {
value: elements[index++],
done: index > elements.length,
};
},
};
return iterator;
}
38 changes: 38 additions & 0 deletions src/utils/isSubsetOf.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
/** Taken from TypeScript collection lib to perfectly match the .isSubsetOf signature */
export interface ReadonlySetLike<T> {
verytactical marked this conversation as resolved.
Show resolved Hide resolved
/**
* Despite its name, returns an iterator of the values in the set-like.
*/
keys(): Iterator<T>;
/**
* @returns a boolean indicating whether an element with the specified value exists in the set-like or not.
*/
has(value: T): boolean;
/**
* @returns the number of (unique) elements in the set-like.
*/
readonly size: number;
}

/**
* @returns a boolean indicating whether all the elements in Set `one` are also in the `other`.
*/
export function isSubsetOf<T>(
one: Set<T>,
other: ReadonlySetLike<unknown>,
): boolean {
// If the builtin method exists, just call it
if ("isSubsetOf" in Set.prototype) {
return one.isSubsetOf(other);
}
// If not, provide the implementation
if (one.size > other.size) {
return false;
}
for (const element of one) {
if (!other.has(element)) {
return false;
}
}
return true;
}
Loading