Skip to content

Commit

Permalink
Make test params readonly so they can't be accidentally permanently m…
Browse files Browse the repository at this point in the history
…odified (#3097)

This should hopefully categorically prevent bugs like the one fixed in
#3096
  • Loading branch information
kainino0x authored Oct 26, 2023
1 parent e5f120e commit f3196f8
Show file tree
Hide file tree
Showing 34 changed files with 449 additions and 351 deletions.
16 changes: 10 additions & 6 deletions src/common/framework/params_builder.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { Merged, mergeParams, mergeParamsChecked } from '../internal/params_utils.js';
import { comparePublicParamsPaths, Ordering } from '../internal/query/compare.js';
import { stringifyPublicParams } from '../internal/query/stringify_params.js';
import { DeepReadonly } from '../util/types.js';
import { assert, mapLazy, objectEquals } from '../util/util.js';

import { TestParams } from './fixture.js';
Expand Down Expand Up @@ -98,7 +99,7 @@ export type ParamTypeOf<
* - `[case params, undefined]` if not.
*/
export type CaseSubcaseIterable<CaseP, SubcaseP> = Iterable<
readonly [CaseP, Iterable<SubcaseP> | undefined]
readonly [DeepReadonly<CaseP>, Iterable<DeepReadonly<SubcaseP>> | undefined]
>;

/**
Expand Down Expand Up @@ -143,7 +144,7 @@ export function builderIterateCasesWithSubcases(
*/
export class CaseParamsBuilder<CaseP extends {}>
extends ParamsBuilderBase<CaseP, {}>
implements Iterable<CaseP>, ParamsBuilder {
implements Iterable<DeepReadonly<CaseP>>, ParamsBuilder {
*iterateCasesWithSubcases(caseFilter: TestParams | null): CaseSubcaseIterable<CaseP, {}> {
for (const caseP of this.cases(caseFilter)) {
if (caseFilter) {
Expand All @@ -155,12 +156,12 @@ export class CaseParamsBuilder<CaseP extends {}>
}
}

yield [caseP, undefined];
yield [caseP as DeepReadonly<typeof caseP>, undefined];
}
}

[Symbol.iterator](): Iterator<CaseP> {
return this.cases(null);
[Symbol.iterator](): Iterator<DeepReadonly<CaseP>> {
return this.cases(null) as Iterator<DeepReadonly<CaseP>>;
}

/** @inheritDoc */
Expand Down Expand Up @@ -302,7 +303,10 @@ export class SubcaseParamsBuilder<CaseP extends {}, SubcaseP extends {}>

const subcases = Array.from(this.subcases(caseP));
if (subcases.length) {
yield [caseP, subcases];
yield [
caseP as DeepReadonly<typeof caseP>,
subcases as DeepReadonly<typeof subcases[number]>[],
];
}
}
}
Expand Down
7 changes: 5 additions & 2 deletions src/common/internal/test_group.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import {
stringifyPublicParamsUniquely,
} from '../internal/query/stringify_params.js';
import { validQueryPart } from '../internal/query/validQueryPart.js';
import { DeepReadonly } from '../util/types.js';
import { assert, unreachable } from '../util/util.js';

import { logToWebsocket } from './websocket_logger.js';
Expand Down Expand Up @@ -82,9 +83,11 @@ export function makeTestGroupForUnitTesting<F extends Fixture>(
/** Parameter name for batch number (see also TestBuilder.batch). */
const kBatchParamName = 'batch__';

type TestFn<F extends Fixture, P extends {}> = (t: F & { params: P }) => Promise<void> | void;
type TestFn<F extends Fixture, P extends {}> = (
t: F & { params: DeepReadonly<P> }
) => Promise<void> | void;
type BeforeAllSubcasesFn<S extends SubcaseBatchState, P extends {}> = (
s: S & { params: P }
s: S & { params: DeepReadonly<P> }
) => Promise<void> | void;

export class TestGroup<F extends Fixture> implements TestGroupBuilder<F> {
Expand Down
39 changes: 39 additions & 0 deletions src/common/util/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,45 @@ export type TypeEqual<X, Y> = (<T>() => T extends X ? 1 : 2) extends <T>() => T
/* eslint-disable-next-line @typescript-eslint/no-unused-vars */
export function assertTypeTrue<T extends true>() {}

/** `ReadonlyArray` of `ReadonlyArray`s. */
export type ROArrayArray<T> = ReadonlyArray<ReadonlyArray<T>>;
/** `ReadonlyArray` of `ReadonlyArray`s of `ReadonlyArray`s. */
export type ROArrayArrayArray<T> = ReadonlyArray<ReadonlyArray<ReadonlyArray<T>>>;

/**
* Deep version of the Readonly<> type, with support for tuples (up to length 7).
* <https://gist.github.com/masterkidan/7322752f569b1bba53e0426266768623>
*/
export type DeepReadonly<T> = T extends [infer A]
? DeepReadonlyObject<[A]>
: T extends [infer A, infer B]
? DeepReadonlyObject<[A, B]>
: T extends [infer A, infer B, infer C]
? DeepReadonlyObject<[A, B, C]>
: T extends [infer A, infer B, infer C, infer D]
? DeepReadonlyObject<[A, B, C, D]>
: T extends [infer A, infer B, infer C, infer D, infer E]
? DeepReadonlyObject<[A, B, C, D, E]>
: T extends [infer A, infer B, infer C, infer D, infer E, infer F]
? DeepReadonlyObject<[A, B, C, D, E, F]>
: T extends [infer A, infer B, infer C, infer D, infer E, infer F, infer G]
? DeepReadonlyObject<[A, B, C, D, E, F, G]>
: T extends Map<infer U, infer V>
? ReadonlyMap<DeepReadonlyObject<U>, DeepReadonlyObject<V>>
: T extends Set<infer U>
? ReadonlySet<DeepReadonlyObject<U>>
: T extends Promise<infer U>
? Promise<DeepReadonlyObject<U>>
: T extends Primitive
? T
: T extends (infer A)[]
? DeepReadonlyArray<A>
: DeepReadonlyObject<T>;

type Primitive = string | number | boolean | undefined | null | Function | symbol;
type DeepReadonlyArray<T> = ReadonlyArray<DeepReadonly<T>>;
type DeepReadonlyObject<T> = { readonly [P in keyof T]: DeepReadonly<T[P]> };

/**
* Computes the intersection of a set of types, given the union of those types.
*
Expand Down
4 changes: 2 additions & 2 deletions src/common/util/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -346,7 +346,7 @@ interface TypedArrayMap {

type TypedArrayParam<K extends keyof TypedArrayMap> = {
type: K;
data: number[];
data: readonly number[];
};

/**
Expand Down Expand Up @@ -387,7 +387,7 @@ export function typedArrayParam<K extends keyof TypedArrayMap>(

export function createTypedArray<K extends keyof TypedArrayMap>(
type: K,
data: number[]
data: readonly number[]
): TypedArrayMap[K] {
return new kTypedArrayBufferViews[type](data) as TypedArrayMap[K];
}
Expand Down
30 changes: 10 additions & 20 deletions src/unittests/floating_point.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2582,8 +2582,7 @@ g.test('atanInterval')
return ulp_error * trait.oneULP(n);
};

t.params.expected = applyError(t.params.expected, error);
const expected = trait.toInterval(t.params.expected);
const expected = trait.toInterval(applyError(t.params.expected, error));

const got = trait.atanInterval(t.params.input);
t.expect(
Expand Down Expand Up @@ -2760,8 +2759,7 @@ g.test('cosInterval')
return t.params.trait === 'f32' ? 2 ** -11 : 2 ** -7;
};

t.params.expected = applyError(t.params.expected, error);
const expected = trait.toInterval(t.params.expected);
const expected = trait.toInterval(applyError(t.params.expected, error));

const got = trait.cosInterval(t.params.input);
t.expect(
Expand Down Expand Up @@ -2941,8 +2939,7 @@ g.test('expInterval')
return ulp_error * trait.oneULP(x);
};

t.params.expected = applyError(t.params.expected, error);
const expected = trait.toInterval(t.params.expected);
const expected = trait.toInterval(applyError(t.params.expected, error));
const got = trait.expInterval(t.params.input);

t.expect(
Expand Down Expand Up @@ -3001,8 +2998,7 @@ g.test('exp2Interval')
return ulp_error * trait.oneULP(x);
};

t.params.expected = applyError(t.params.expected, error);
const expected = trait.toInterval(t.params.expected);
const expected = trait.toInterval(applyError(t.params.expected, error));

const got = trait.exp2Interval(t.params.input);
t.expect(
Expand Down Expand Up @@ -3197,8 +3193,7 @@ g.test('inverseSqrtInterval')
return 2 * trait.oneULP(n);
};

t.params.expected = applyError(t.params.expected, error);
const expected = trait.toInterval(t.params.expected);
const expected = trait.toInterval(applyError(t.params.expected, error));

const got = trait.inverseSqrtInterval(t.params.input);
t.expect(
Expand Down Expand Up @@ -3322,8 +3317,7 @@ g.test('logInterval')
return 3 * trait.oneULP(n);
};

t.params.expected = applyError(t.params.expected, error);
const expected = trait.toInterval(t.params.expected);
const expected = trait.toInterval(applyError(t.params.expected, error));

const got = trait.logInterval(t.params.input);
t.expect(
Expand Down Expand Up @@ -3373,8 +3367,7 @@ g.test('log2Interval')
return 3 * trait.oneULP(n);
};

t.params.expected = applyError(t.params.expected, error);
const expected = trait.toInterval(t.params.expected);
const expected = trait.toInterval(applyError(t.params.expected, error));

const got = trait.log2Interval(t.params.input);
t.expect(
Expand Down Expand Up @@ -3720,8 +3713,7 @@ g.test('sinInterval')
return t.params.trait === 'f32' ? 2 ** -11 : 2 ** -7;
};

t.params.expected = applyError(t.params.expected, error);
const expected = trait.toInterval(t.params.expected);
const expected = trait.toInterval(applyError(t.params.expected, error));

const got = trait.sinInterval(t.params.input);
t.expect(
Expand Down Expand Up @@ -3855,8 +3847,7 @@ g.test('sqrtInterval')
return 2.5 * trait.oneULP(n);
};

t.params.expected = applyError(t.params.expected, error);
const expected = trait.toInterval(t.params.expected);
const expected = trait.toInterval(applyError(t.params.expected, error));

const got = trait.sqrtInterval(t.params.input);
t.expect(
Expand Down Expand Up @@ -4429,10 +4420,9 @@ g.test('divisionInterval')
};

const [x, y] = t.params.input;
t.params.expected = applyError(t.params.expected, error);

// Do not swizzle here, so the correct implementation under test is called.
const expected = FP[t.params.trait].toInterval(t.params.expected);
const expected = FP[t.params.trait].toInterval(applyError(t.params.expected, error));
const got = FP[t.params.trait].divisionInterval(x, y);
t.expect(
objectEquals(expected, got),
Expand Down
4 changes: 2 additions & 2 deletions src/unittests/maths.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,8 @@ function withinOneULPF32(got: number, expected: number, mode: FlushMode): boolea
* FTZ occur during comparison
**/
function compareArrayOfNumbersF32(
got: Array<number>,
expect: Array<number>,
got: readonly number[],
expect: readonly number[],
mode: FlushMode = 'flush'
): boolean {
return (
Expand Down
4 changes: 2 additions & 2 deletions src/webgpu/api/validation/render_pipeline/inter_stage.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ function getVarName(i: number) {
}

class InterStageMatchingValidationTest extends CreateRenderPipelineValidationTest {
getVertexStateWithOutputs(outputs: string[]): GPUVertexState {
getVertexStateWithOutputs(outputs: readonly string[]): GPUVertexState {
return {
module: this.device.createShaderModule({
code: `
Expand All @@ -32,7 +32,7 @@ class InterStageMatchingValidationTest extends CreateRenderPipelineValidationTes
}

getFragmentStateWithInputs(
inputs: string[],
inputs: readonly string[],
hasBuiltinPosition: boolean = false
): GPUFragmentState {
return {
Expand Down
2 changes: 1 addition & 1 deletion src/webgpu/format_info.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1255,7 +1255,7 @@ export const kFeaturesForFormats = getFeaturesForFormats(kTextureFormats);
/**
* Given an array of texture formats return the number of bytes per sample.
*/
export function computeBytesPerSampleFromFormats(formats: GPUTextureFormat[]) {
export function computeBytesPerSampleFromFormats(formats: readonly GPUTextureFormat[]) {
let bytesPerSample = 0;
for (const format of formats) {
const info = kTextureFormatInfo[format];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,11 @@ import { onlyConstInputSource, run } from '../expression.js';

import { abstractBinary } from './binary.js';

const additionVectorScalarInterval = (v: number[], s: number): FPVector => {
const additionVectorScalarInterval = (v: readonly number[], s: number): FPVector => {
return FP.abstract.toVector(v.map(e => FP.abstract.additionInterval(e, s)));
};

const additionScalarVectorInterval = (s: number, v: number[]): FPVector => {
const additionScalarVectorInterval = (s: number, v: readonly number[]): FPVector => {
return FP.abstract.toVector(v.map(e => FP.abstract.additionInterval(s, e)));
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,11 @@ import { onlyConstInputSource, run } from '../expression.js';

import { abstractBinary } from './binary.js';

const divisionVectorScalarInterval = (v: number[], s: number): FPVector => {
const divisionVectorScalarInterval = (v: readonly number[], s: number): FPVector => {
return FP.abstract.toVector(v.map(e => FP.abstract.divisionInterval(e, s)));
};

const divisionScalarVectorInterval = (s: number, v: number[]): FPVector => {
const divisionScalarVectorInterval = (s: number, v: readonly number[]): FPVector => {
return FP.abstract.toVector(v.map(e => FP.abstract.divisionInterval(s, e)));
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,11 @@ import { onlyConstInputSource, run } from '../expression.js';

import { abstractBinary } from './binary.js';

const multiplicationVectorScalarInterval = (v: number[], s: number): FPVector => {
const multiplicationVectorScalarInterval = (v: readonly number[], s: number): FPVector => {
return FP.abstract.toVector(v.map(e => FP.abstract.multiplicationInterval(e, s)));
};

const multiplicationScalarVectorInterval = (s: number, v: number[]): FPVector => {
const multiplicationScalarVectorInterval = (s: number, v: readonly number[]): FPVector => {
return FP.abstract.toVector(v.map(e => FP.abstract.multiplicationInterval(s, e)));
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,11 @@ import { onlyConstInputSource, run } from '../expression.js';

import { abstractBinary } from './binary.js';

const remainderVectorScalarInterval = (v: number[], s: number): FPVector => {
const remainderVectorScalarInterval = (v: readonly number[], s: number): FPVector => {
return FP.abstract.toVector(v.map(e => FP.abstract.remainderInterval(e, s)));
};

const remainderScalarVectorInterval = (s: number, v: number[]): FPVector => {
const remainderScalarVectorInterval = (s: number, v: readonly number[]): FPVector => {
return FP.abstract.toVector(v.map(e => FP.abstract.remainderInterval(s, e)));
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,11 @@ import { onlyConstInputSource, run } from '../expression.js';

import { abstractBinary } from './binary.js';

const subtractionVectorScalarInterval = (v: number[], s: number): FPVector => {
const subtractionVectorScalarInterval = (v: readonly number[], s: number): FPVector => {
return FP.abstract.toVector(v.map(e => FP.abstract.subtractionInterval(e, s)));
};

const subtractionScalarVectorInterval = (s: number, v: number[]): FPVector => {
const subtractionScalarVectorInterval = (s: number, v: readonly number[]): FPVector => {
return FP.abstract.toVector(v.map(e => FP.abstract.subtractionInterval(s, e)));
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,11 @@ import { allInputSources, run } from '../expression.js';

import { binary, compoundBinary } from './binary.js';

const additionVectorScalarInterval = (v: number[], s: number): FPVector => {
const additionVectorScalarInterval = (v: readonly number[], s: number): FPVector => {
return FP.f16.toVector(v.map(e => FP.f16.additionInterval(e, s)));
};

const additionScalarVectorInterval = (s: number, v: number[]): FPVector => {
const additionScalarVectorInterval = (s: number, v: readonly number[]): FPVector => {
return FP.f16.toVector(v.map(e => FP.f16.additionInterval(s, e)));
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,11 @@ import { allInputSources, run } from '../expression.js';

import { binary, compoundBinary } from './binary.js';

const divisionVectorScalarInterval = (v: number[], s: number): FPVector => {
const divisionVectorScalarInterval = (v: readonly number[], s: number): FPVector => {
return FP.f16.toVector(v.map(e => FP.f16.divisionInterval(e, s)));
};

const divisionScalarVectorInterval = (s: number, v: number[]): FPVector => {
const divisionScalarVectorInterval = (s: number, v: readonly number[]): FPVector => {
return FP.f16.toVector(v.map(e => FP.f16.divisionInterval(s, e)));
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,11 @@ import { allInputSources, run } from '../expression.js';

import { binary, compoundBinary } from './binary.js';

const multiplicationVectorScalarInterval = (v: number[], s: number): FPVector => {
const multiplicationVectorScalarInterval = (v: readonly number[], s: number): FPVector => {
return FP.f16.toVector(v.map(e => FP.f16.multiplicationInterval(e, s)));
};

const multiplicationScalarVectorInterval = (s: number, v: number[]): FPVector => {
const multiplicationScalarVectorInterval = (s: number, v: readonly number[]): FPVector => {
return FP.f16.toVector(v.map(e => FP.f16.multiplicationInterval(s, e)));
};

Expand Down
Loading

0 comments on commit f3196f8

Please sign in to comment.