diff --git a/packages/typegpu/src/data/snippet.ts b/packages/typegpu/src/data/snippet.ts index b67fc50065..c12a48a8a9 100644 --- a/packages/typegpu/src/data/snippet.ts +++ b/packages/typegpu/src/data/snippet.ts @@ -1,7 +1,8 @@ -import { undecorate } from './dataTypes.ts'; -import type { AnyData, UnknownData } from './dataTypes.ts'; +import { stitch } from '../core/resolve/stitch.ts'; import { DEV } from '../shared/env.ts'; -import { isNumericSchema } from './wgslTypes.ts'; +import type { AnyData, UnknownData } from './dataTypes.ts'; +import { undecorate } from './dataTypes.ts'; +import { isNaturallyEphemeral, isNumericSchema } from './wgslTypes.ts'; export type Origin = | 'uniform' @@ -29,12 +30,45 @@ export type Origin = | 'constant-tgpu-const-ref' /* turns into a `const` when assigned to a variable */ | 'runtime-tgpu-const-ref' /* turns into a `let` when assigned to a variable */; -export function isEphemeralOrigin(space: Origin) { - return space === 'runtime' || space === 'constant' || space === 'argument'; +export function isEphemeralSnippet(snippet: Snippet) { + if (snippet.origin === 'argument') { + // Arguments are considered ephemeral if their data type + // is naturally ephemeral. + // (primitives => true, non-primitives => false). + return isNaturallyEphemeral(snippet.dataType); + } + return snippet.origin === 'runtime' || snippet.origin === 'constant'; } -export function isEphemeralSnippet(snippet: Snippet) { - return isEphemeralOrigin(snippet.origin); +/** + * Returns a reason if the snippet is immutable, or undefined if it is mutable. + */ +export function getImmutableReason(snippet: Snippet): string | undefined { + if (snippet.origin === 'argument') { + return stitch`Cannot mutate '${snippet}', arguments are immutable. You can copy them into a local variable first.`; + } + + if ( + snippet.origin === 'constant' || + snippet.origin === 'constant-tgpu-const-ref' || + snippet.origin === 'runtime-tgpu-const-ref' + ) { + return stitch`Cannot mutate '${snippet}', constants are immutable.`; + } + + if (snippet.origin === 'uniform') { + return stitch`Cannot mutate '${snippet}', uniforms are immutable.`; + } + + if (snippet.origin === 'handle') { + return stitch`Cannot mutate '${snippet}', handles are immutable.`; + } + + if (snippet.origin === 'readonly') { + return stitch`Cannot mutate '${snippet}', it's bound as a readonly resource.`; + } + + return undefined; // Mutable 🎉 } export const originToPtrParams = { diff --git a/packages/typegpu/src/tgsl/wgslGenerator.ts b/packages/typegpu/src/tgsl/wgslGenerator.ts index 900a51edc4..553f505717 100644 --- a/packages/typegpu/src/tgsl/wgslGenerator.ts +++ b/packages/typegpu/src/tgsl/wgslGenerator.ts @@ -11,10 +11,10 @@ import { } from '../data/dataTypes.ts'; import { bool, i32, u32 } from '../data/numeric.ts'; import { - isEphemeralOrigin, isEphemeralSnippet, isSnippet, type Origin, + type ResolvedSnippet, snip, type Snippet, } from '../data/snippet.ts'; @@ -224,41 +224,10 @@ ${this.ctx.pre}}`; public blockVariable( varType: 'var' | 'let' | 'const', - id: string, - dataType: wgsl.AnyWgslData | UnknownData, - origin: Origin, - ): Snippet { - const naturallyEphemeral = wgsl.isNaturallyEphemeral(dataType); - - let varOrigin: Origin; - if ( - origin === 'constant-tgpu-const-ref' || - origin === 'runtime-tgpu-const-ref' - ) { - // Even types that aren't naturally referential (like vectors or structs) should - // be treated as constant references when assigned to a const. - varOrigin = origin; - } else if (origin === 'argument') { - if (naturallyEphemeral) { - varOrigin = 'runtime'; - } else { - varOrigin = 'argument'; - } - } else if (!naturallyEphemeral) { - varOrigin = isEphemeralOrigin(origin) ? 'this-function' : origin; - } else if (origin === 'constant' && varType === 'const') { - varOrigin = 'constant'; - } else { - varOrigin = 'runtime'; - } - - const snippet = snip( - this.ctx.makeNameValid(id), - dataType, - /* origin */ varOrigin, - ); - this.ctx.defineVariable(id, snippet); - return snippet; + lhs: ResolvedSnippet, + rhs: Snippet, + ): string { + return stitch`${this.ctx.pre}${varType} ${lhs.value} = ${rhs};`; } public identifier(id: string): Snippet { @@ -916,10 +885,6 @@ ${this.ctx.pre}else ${alternate}`; ); } - const ephemeral = isEphemeralSnippet(eq); - let dataType = eq.dataType as wgsl.AnyWgslData; - const naturallyEphemeral = wgsl.isNaturallyEphemeral(dataType); - if (isLooseData(eq.dataType)) { throw new Error( `Cannot create variable '${rawId}' with loose data type.`, @@ -947,10 +912,40 @@ ${this.ctx.pre}else ${alternate}`; };`; } + const ephemeral = isEphemeralSnippet(eq); + let dataType = eq.dataType as wgsl.AnyWgslData; + let varOrigin: Origin = 'runtime'; + const naturallyEphemeral = wgsl.isNaturallyEphemeral(dataType); + + if (eq.origin === 'argument') { + varOrigin = ephemeral ? 'runtime' : 'argument'; + + if (stmtType === NODE.let && !ephemeral) { + const rhsStr = this.ctx.resolve(eq.value).value; + const rhsTypeStr = this.ctx.resolve(unptr(eq.dataType)).value; + + throw new WgslTypeError( + `'let ${rawId} = ${rhsStr}' is invalid, because references to arguments cannot be assigned to 'let' variable declarations. + ----- + - Try 'let ${rawId} = ${rhsTypeStr}(${rhsStr})' if you need to reassign '${rawId}' later + - Try 'const ${rawId} = ${rhsStr}' if you won't reassign '${rawId}' later. + -----`, + ); + } + + if (stmtType === NODE.const) { + // Arguments cannot be mutated, so we 'let' them be (kill me) + varType = 'let'; + } + } // // Assigning a reference to a `const` variable means we store the pointer // of the rhs. - if (!ephemeral) { + else if (!ephemeral) { // Referential + + // It's a reference to something else, so we inherit their origin + varOrigin = eq.origin; + if (stmtType === NODE.let) { const rhsStr = this.ctx.resolve(eq.value).value; const rhsTypeStr = this.ctx.resolve(unptr(eq.dataType)).value; @@ -991,41 +986,41 @@ ${this.ctx.pre}else ${alternate}`; } else { // Non-referential - if (stmtType === NODE.const) { - if (eq.origin === 'argument') { - // Arguments cannot be mutated, so we 'let' them be (kill me) - varType = 'let'; - } else if (naturallyEphemeral) { + if (naturallyEphemeral) { + // Primitives + + if (stmtType === NODE.const) { varType = eq.origin === 'constant' ? 'const' : 'let'; + varOrigin = eq.origin === 'constant' ? 'constant' : 'runtime'; } } else { - // stmtType === NODE.let - - if (eq.origin === 'argument') { - if (!naturallyEphemeral) { - const rhsStr = this.ctx.resolve(eq.value).value; - const rhsTypeStr = this.ctx.resolve(unptr(eq.dataType)).value; + // Non-primitives - throw new WgslTypeError( - `'let ${rawId} = ${rhsStr}' is invalid, because references to arguments cannot be assigned to 'let' variable declarations. - ----- - - Try 'let ${rawId} = ${rhsTypeStr}(${rhsStr})' if you need to reassign '${rawId}' later - - Try 'const ${rawId} = ${rhsStr}' if you won't reassign '${rawId}' later. - -----`, - ); - } - } + // We're creating a new variable with an ephemeral snippet, + // but referencing that variable down the line will have the + // origin of 'this-function'. + varOrigin = 'this-function'; } } - const snippet = this.blockVariable( + if ( + eq.origin === 'constant-tgpu-const-ref' || + eq.origin === 'runtime-tgpu-const-ref' + ) { + // Even types that aren't naturally referential (like vectors or structs) should + // be treated as constant references when assigned to a const. + varOrigin = eq.origin; + } + + const varName = this.ctx.makeNameValid(rawId); + const lhs = snip(varName, concretize(dataType), varOrigin); + this.ctx.defineVariable(rawId, lhs); + + return this.blockVariable( varType, - rawId, - concretize(dataType), - eq.origin, + lhs, + tryConvertSnippet(eq, dataType, false), ); - return stitch`${this.ctx.pre}${varType} ${snippet - .value as string} = ${tryConvertSnippet(eq, dataType, false)};`; } if (statement[0] === NODE.block) { diff --git a/packages/typegpu/tests/bufferUsage.test.ts b/packages/typegpu/tests/bufferUsage.test.ts index f619c91013..f1d4245b36 100644 --- a/packages/typegpu/tests/bufferUsage.test.ts +++ b/packages/typegpu/tests/bufferUsage.test.ts @@ -88,6 +88,22 @@ describe('TgpuBufferUniform', () => { .as('uniform') ).toThrow(); }); + + it('cannot be mutated', ({ root }) => { + const Boid = d.struct({ + pos: d.vec3f, + vel: d.vec3u, + }); + + const boidUniform = root.createUniform(Boid); + + const main = () => { + 'use gpu'; + boidUniform.$.pos.x += 1; + }; + + expect(() => tgpu.resolve([main])).toThrowErrorMatchingInlineSnapshot(); + }); }); describe('TgpuBufferMutable', () => { diff --git a/packages/typegpu/tests/constant.test.ts b/packages/typegpu/tests/constant.test.ts index 3fbfd9eaa1..392ebcc11b 100644 --- a/packages/typegpu/tests/constant.test.ts +++ b/packages/typegpu/tests/constant.test.ts @@ -121,49 +121,4 @@ describe('tgpu.const', () => { }" `); }); - - it('cannot be passed directly to shellless functions', () => { - const fn1 = (v: d.v3f) => { - 'use gpu'; - return v.x * v.y * v.z; - }; - - const foo = tgpu.const(d.vec3f, d.vec3f(1, 2, 3)); - const fn2 = () => { - 'use gpu'; - return fn1(foo.$); - }; - - expect(() => tgpu.resolve([fn2])).toThrowErrorMatchingInlineSnapshot(` - [Error: Resolution of the following tree failed: - - - - fn*:fn2 - - fn*:fn2(): Cannot pass constant references as function arguments. Explicitly copy them by wrapping them in a schema: 'vec3f(...)'] - `); - }); - - it('cannot be mutated', () => { - const boid = tgpu.const(Boid, { - pos: d.vec3f(1, 2, 3), - vel: d.vec3u(4, 5, 6), - }); - - const fn = () => { - 'use gpu'; - // @ts-expect-error: Cannot assign to read-only property - boid.$.pos = d.vec3f(0, 0, 0); - }; - - expect(() => tgpu.resolve([fn])).toThrowErrorMatchingInlineSnapshot(` - [Error: Resolution of the following tree failed: - - - - fn*:fn - - fn*:fn(): 'boid.pos = vec3f()' is invalid, because boid.pos is a constant.] - `); - - // Since we freeze the object, we cannot mutate when running the function in JS either - expect(() => fn()).toThrowErrorMatchingInlineSnapshot( - `[TypeError: Cannot assign to read only property 'pos' of object '#']`, - ); - }); }); diff --git a/packages/typegpu/tests/tgsl/wgslGenerator.test.ts b/packages/typegpu/tests/tgsl/wgslGenerator.test.ts index 3c35557242..0db4cd2d2f 100644 --- a/packages/typegpu/tests/tgsl/wgslGenerator.test.ts +++ b/packages/typegpu/tests/tgsl/wgslGenerator.test.ts @@ -1,10 +1,11 @@ +/** biome-ignore-all lint/style/noNonNullAssertion: it's useful */ import * as tinyest from 'tinyest'; import { beforeEach, describe, expect } from 'vitest'; import { namespace } from '../../src/core/resolve/namespace.ts'; import * as d from '../../src/data/index.ts'; import { abstractFloat, abstractInt } from '../../src/data/numeric.ts'; import { snip } from '../../src/data/snippet.ts'; -import { Void, type WgslArray } from '../../src/data/wgslTypes.ts'; +import type { WgslArray } from '../../src/data/wgslTypes.ts'; import { provideCtx } from '../../src/execMode.ts'; import tgpu from '../../src/index.ts'; import { ResolutionCtxImpl } from '../../src/resolutionCtx.ts'; @@ -14,6 +15,7 @@ import * as std from '../../src/std/index.ts'; import wgslGenerator from '../../src/tgsl/wgslGenerator.ts'; import { CodegenState } from '../../src/types.ts'; import { it } from '../utils/extendedIt.ts'; +import { expectDataTypeOf } from '../utils/parseResolved.ts'; const { NodeTypeCatalog: NODE } = tinyest; @@ -257,107 +259,34 @@ describe('wgslGenerator', () => { }); it('generates correct resources for nested struct with atomics in a complex expression', ({ root }) => { - const testBuffer = root - .createBuffer( - d - .struct({ - a: d.vec4f, - b: d - .struct({ - aa: d.arrayOf( - d - .struct({ x: d.atomic(d.u32), y: d.atomic(d.i32) }) - .$name('DeeplyNestedStruct'), - 64, - ), - }) - .$name('NestedStruct'), - }) - .$name('TestStruct'), - ) - .$usage('storage'); - - const testUsage = testBuffer.as('mutable'); - - const testFn = tgpu.fn([d.u32], d.vec4f)((idx) => { - // biome-ignore lint/style/noNonNullAssertion: - const value = std.atomicLoad(testUsage.value.b.aa[idx]!.y); - const vec = std.mix(d.vec4f(), testUsage.value.a, value); - // biome-ignore lint/style/noNonNullAssertion: - std.atomicStore(testUsage.value.b.aa[idx]!.x, vec.y); - return vec; - }); - - const astInfo = getMetaData( - testFn[$internal].implementation as (...args: unknown[]) => unknown, - ); - - if (!astInfo?.ast) { - throw new Error('Expected prebuilt AST to be present'); - } - - expect(JSON.stringify(astInfo.ast.body)).toMatchInlineSnapshot( - `"[0,[[13,"value",[6,[7,"std","atomicLoad"],[[7,[8,[7,[7,[7,"testUsage","value"],"b"],"aa"],"idx"],"y"]]]],[13,"vec",[6,[7,"std","mix"],[[6,[7,"d","vec4f"],[]],[7,[7,"testUsage","value"],"a"],"value"]]],[6,[7,"std","atomicStore"],[[7,[8,[7,[7,[7,"testUsage","value"],"b"],"aa"],"idx"],"x"],[7,"vec","y"]]],[10,"vec"]]]"`, - ); - - if ( - astInfo.ast.params.filter((arg) => arg.type !== 'i').length > 0 - ) { - throw new Error('Expected arguments as identifier names in ast'); - } - - const args = astInfo.ast.params.map((arg) => - snip( - (arg as { type: 'i'; name: string }).name, - d.u32, - /* origin */ 'runtime', - ) + const testMutable = root.createMutable( + d.struct({ + a: d.vec4f, + b: d.struct({ + aa: d.arrayOf( + d.struct({ x: d.atomic(d.u32), y: d.atomic(d.i32) }) + .$name('DeeplyNestedStruct'), + 64, + ), + }).$name('NestedStruct'), + }).$name('TestStruct'), ); - provideCtx(ctx, () => { - ctx[$internal].itemStateStack.pushFunctionScope( - 'normal', - args, - {}, - d.vec4f, - (astInfo.externals as () => Record)() ?? {}, - ); - - // Check for: const value = std.atomicLoad(testUsage.value.b.aa[idx]!.y); - // ^ this part should be a i32 - const res = wgslGenerator.expression( - (astInfo.ast?.body[1][0] as tinyest.Const)[2], - ); - - expect(res.dataType).toStrictEqual(d.i32); - - // Check for: const vec = std.mix(d.vec4f(), testUsage.value.a, value); - // ^ this part should be a vec4f - ctx[$internal].itemStateStack.pushBlockScope(); - wgslGenerator.blockVariable('var', 'value', d.i32, 'runtime'); - const res2 = wgslGenerator.expression( - (astInfo.ast?.body[1][1] as tinyest.Const)[2], - ); - ctx[$internal].itemStateStack.popBlockScope(); - - expect(res2.dataType).toStrictEqual(d.vec4f); + expectDataTypeOf(() => { + 'use gpu'; + std.atomicLoad(testMutable.$.b.aa[0]!.y); + }).toStrictEqual(d.i32); - // Check for: std.atomicStore(testUsage.value.b.aa[idx]!.x, vec.y); - // ^ this part should be an atomic u32 - // ^ this part should be void - ctx[$internal].itemStateStack.pushBlockScope(); - wgslGenerator.blockVariable('var', 'vec', d.vec4f, 'function'); - const res3 = wgslGenerator.expression( - (astInfo.ast?.body[1][2] as tinyest.Call)[2][0] as tinyest.Expression, - ); - const res4 = wgslGenerator.expression( - astInfo.ast?.body[1][2] as tinyest.Expression, - ); - ctx[$internal].itemStateStack.popBlockScope(); + expectDataTypeOf(() => { + 'use gpu'; + const value = std.atomicLoad(testMutable.$.b.aa[0]!.y); + std.mix(d.vec4f(), testMutable.$.a, value); + }).toStrictEqual(d.vec4f); - expect(res3.dataType).toStrictEqual(d.atomic(d.u32)); - expect(res4.dataType).toStrictEqual(Void); - }); + expectDataTypeOf(() => { + 'use gpu'; + testMutable.$.b.aa[0]!.x; + }).toStrictEqual(d.atomic(d.u32)); }); it('creates correct code for for statements', () => { diff --git a/packages/typegpu/tests/utils/parseResolved.ts b/packages/typegpu/tests/utils/parseResolved.ts index fc59207de0..44e4a8fd92 100644 --- a/packages/typegpu/tests/utils/parseResolved.ts +++ b/packages/typegpu/tests/utils/parseResolved.ts @@ -12,9 +12,7 @@ import { $internal } from '../../src/shared/symbols.ts'; import { CodegenState } from '../../src/types.ts'; function extractSnippetFromFn(cb: () => unknown): Snippet { - const ctx = new ResolutionCtxImpl({ - namespace: namespace({ names: 'strict' }), - }); + const ctx = new ResolutionCtxImpl({ namespace: namespace() }); return provideCtx( ctx,