diff --git a/packages/typegpu/src/core/buffer/bufferUsage.ts b/packages/typegpu/src/core/buffer/bufferUsage.ts index 24f28dcbca..d8a1028e78 100644 --- a/packages/typegpu/src/core/buffer/bufferUsage.ts +++ b/packages/typegpu/src/core/buffer/bufferUsage.ts @@ -118,7 +118,7 @@ class TgpuFixedBufferImpl implements TgpuConst, } [$resolve](ctx: ResolutionCtx): ResolvedSnippet { - const id = ctx.getUniqueName(this); + const id = ctx.makeUniqueIdentifier(getName(this), 'global'); const resolvedDataType = ctx.resolve(this.dataType).value; const resolvedValue = ctx.resolve(this.#value, this.dataType).value; diff --git a/packages/typegpu/src/core/function/fnCore.ts b/packages/typegpu/src/core/function/fnCore.ts index 77d676c0d5..78f0f74d20 100644 --- a/packages/typegpu/src/core/function/fnCore.ts +++ b/packages/typegpu/src/core/function/fnCore.ts @@ -3,6 +3,7 @@ import { undecorate } from '../../data/dataTypes.ts'; import { type ResolvedSnippet, snip } from '../../data/snippet.ts'; import { type BaseData, isWgslData, isWgslStruct, Void } from '../../data/wgslTypes.ts'; import { MissingLinksError } from '../../errors.ts'; +import { validateIdentifier } from '../../nameUtils.ts'; import { getMetaData, getName } from '../../shared/meta.ts'; import { $getNameForward } from '../../shared/symbols.ts'; import type { ResolutionCtx, TgpuShaderStage } from '../../types.ts'; @@ -74,21 +75,28 @@ export function createFnCore( applyExternals(externalMap, externals); } - const id = ctx.getUniqueName(this); + const id = ctx.makeUniqueIdentifier(getName(this), 'global'); if (typeof implementation === 'string') { if (!returnType) { throw new Error('Explicit return type is required for string implementation'); } - const validArgNames = entryInput - ? Object.fromEntries( - entryInput.positionalArgs.map((a) => [a.schemaKey, ctx.makeNameValid(a.schemaKey)]), - ) - : undefined; + if (entryInput) { + for (const arg of entryInput.positionalArgs) { + const result = validateIdentifier(arg.schemaKey); + if (!result.success) { + throw new Error( + `Invalid argument name "${arg.schemaKey}"${result.error ? `: ${result.error}` : ''}`, + ); + } + } - if (validArgNames && Object.keys(validArgNames).length > 0) { - applyExternals(externalMap, { in: validArgNames }); + applyExternals(externalMap, { + in: Object.fromEntries( + entryInput.positionalArgs.map((a) => [a.schemaKey, a.schemaKey]), + ), + }); } const replacedImpl = replaceExternalsInWgsl(ctx, externalMap, implementation); @@ -96,15 +104,15 @@ export function createFnCore( let header = ''; let body = ''; - if (functionType !== 'normal' && entryInput && validArgNames) { + if (functionType !== 'normal' && entryInput) { const { dataSchema, positionalArgs } = entryInput; const parts: string[] = []; if (dataSchema && isArgUsedInBody('in', replacedImpl)) { parts.push(`in: ${ctx.resolve(dataSchema).value}`); } for (const a of positionalArgs) { - const argName = validArgNames[a.schemaKey] ?? ''; - if (argName !== '' && isArgUsedInBody(argName, replacedImpl)) { + const argName = a.schemaKey; + if (isArgUsedInBody(argName, replacedImpl)) { parts.push(`${getAttributesString(a.type)}${argName}: ${ctx.resolve(a.type).value}`); } } diff --git a/packages/typegpu/src/core/resolve/namespace.ts b/packages/typegpu/src/core/resolve/namespace.ts index f112cd4fb0..90fcba08f7 100644 --- a/packages/typegpu/src/core/resolve/namespace.ts +++ b/packages/typegpu/src/core/resolve/namespace.ts @@ -1,6 +1,5 @@ import type { ResolvedSnippet } from '../../data/snippet.ts'; -import { type NameRegistry, RandomNameRegistry, StrictNameRegistry } from '../../nameRegistry.ts'; -import { getName } from '../../shared/meta.ts'; +import { bannedTokens, builtins } from '../../nameUtils.ts'; import { $internal } from '../../shared/symbols.ts'; import { ShelllessRepository } from '../../tgsl/shellless.ts'; import type { TgpuLazy, TgpuSlot } from '../slot/slotTypes.ts'; @@ -8,8 +7,9 @@ import type { TgpuLazy, TgpuSlot } from '../slot/slotTypes.ts'; type SlotToValueMap = Map, unknown>; export interface NamespaceInternal { - readonly nameRegistry: NameRegistry; + readonly takenGlobalIdentifiers: Set; readonly shelllessRepo: ShelllessRepository; + readonly strategy: 'random' | 'strict'; memoizedResolves: WeakMap< // WeakMap because if the item does not exist anymore, @@ -24,73 +24,32 @@ export interface NamespaceInternal { TgpuLazy, { slotToValueMap: SlotToValueMap; result: unknown }[] >; - - listeners: { - [K in keyof NamespaceEventMap]: Set<(event: NamespaceEventMap[K]) => void>; - }; } -type NamespaceEventMap = { - name: { target: object; name: string }; -}; - -type DetachListener = () => void; - export interface Namespace { readonly [$internal]: NamespaceInternal; - - on( - event: TEvent, - listener: (event: NamespaceEventMap[TEvent]) => void, - ): DetachListener; } class NamespaceImpl implements Namespace { readonly [$internal]: NamespaceInternal; - constructor(nameRegistry: NameRegistry) { + constructor(strategy: 'random' | 'strict') { this[$internal] = { - nameRegistry, + strategy, + takenGlobalIdentifiers: new Set([...bannedTokens, ...builtins]), shelllessRepo: new ShelllessRepository(), memoizedResolves: new WeakMap(), memoizedLazy: new WeakMap(), - listeners: { - name: new Set(), - }, }; } - - on( - event: TEvent, - listener: (event: NamespaceEventMap[TEvent]) => void, - ): DetachListener { - if (event === 'name') { - const listeners = this[$internal].listeners.name; - listeners.add(listener); - - return () => listeners.delete(listener); - } - - throw new Error(`Unsupported event: ${event}`); - } } export interface NamespaceOptions { names?: 'random' | 'strict' | undefined; } -export function getUniqueName(namespace: NamespaceInternal, resource: object): string { - const name = namespace.nameRegistry.makeUnique(getName(resource), true); - for (const listener of namespace.listeners.name) { - listener({ target: resource, name }); - } - return name; -} - export function namespace(options?: NamespaceOptions): Namespace { const { names = 'strict' } = options ?? {}; - return new NamespaceImpl( - names === 'strict' ? new StrictNameRegistry() : new RandomNameRegistry(), - ); + return new NamespaceImpl(names); } diff --git a/packages/typegpu/src/core/resolve/resolveData.ts b/packages/typegpu/src/core/resolve/resolveData.ts index 19e5da347d..bc5320c80d 100644 --- a/packages/typegpu/src/core/resolve/resolveData.ts +++ b/packages/typegpu/src/core/resolve/resolveData.ts @@ -38,6 +38,7 @@ import type { WgslArray, WgslStruct, } from '../../data/wgslTypes.ts'; +import { getName } from '../../shared/meta.ts'; import { $internal } from '../../shared/symbols.ts'; import { assertExhaustive } from '../../shared/utilityTypes.ts'; import type { ResolutionCtx } from '../../types.ts'; @@ -127,7 +128,7 @@ function resolveStruct(ctx: ResolutionCtx, struct: WgslStruct) { if (struct[$internal].isAbstruct) { throw new Error('Cannot resolve abstract struct types to WGSL.'); } - const id = ctx.getUniqueName(struct); + const id = ctx.makeUniqueIdentifier(getName(struct), 'global'); ctx.addDeclaration(`\ struct ${id} { @@ -155,7 +156,7 @@ ${Object.entries(struct.propTypes) * ``` */ function resolveUnstruct(ctx: ResolutionCtx, unstruct: Unstruct) { - const id = ctx.getUniqueName(unstruct); + const id = ctx.makeUniqueIdentifier(getName(unstruct), 'global'); ctx.addDeclaration(`\ struct ${id} { diff --git a/packages/typegpu/src/core/sampler/sampler.ts b/packages/typegpu/src/core/sampler/sampler.ts index 4daaf1c917..adf10ee8ce 100644 --- a/packages/typegpu/src/core/sampler/sampler.ts +++ b/packages/typegpu/src/core/sampler/sampler.ts @@ -99,7 +99,7 @@ export class TgpuLaidOutSamplerImpl< } [$resolve](ctx: ResolutionCtx): ResolvedSnippet { - const id = ctx.getUniqueName(this); + const id = ctx.makeUniqueIdentifier(getName(this), 'global'); const group = ctx.allocateLayoutEntry(this.#membership.layout); ctx.addDeclaration( @@ -186,7 +186,7 @@ class TgpuFixedSamplerImpl } [$resolve](ctx: ResolutionCtx): ResolvedSnippet { - const id = ctx.getUniqueName(this); + const id = ctx.makeUniqueIdentifier(getName(this), 'global'); const { group, binding } = ctx.allocateFixedEntry( this.schema.type === 'sampler_comparison' diff --git a/packages/typegpu/src/core/texture/externalTexture.ts b/packages/typegpu/src/core/texture/externalTexture.ts index c9d457c48b..78ae4cd582 100644 --- a/packages/typegpu/src/core/texture/externalTexture.ts +++ b/packages/typegpu/src/core/texture/externalTexture.ts @@ -37,7 +37,7 @@ export class TgpuExternalTextureImpl implements TgpuExternalTexture, SelfResolva } [$resolve](ctx: ResolutionCtx): ResolvedSnippet { - const id = ctx.getUniqueName(this); + const id = ctx.makeUniqueIdentifier(getName(this), 'global'); const group = ctx.allocateLayoutEntry(this.#membership.layout); ctx.addDeclaration( diff --git a/packages/typegpu/src/core/texture/texture.ts b/packages/typegpu/src/core/texture/texture.ts index 27923995d6..fbe7295d83 100644 --- a/packages/typegpu/src/core/texture/texture.ts +++ b/packages/typegpu/src/core/texture/texture.ts @@ -600,7 +600,7 @@ class TgpuFixedTextureViewImpl } [$resolve](ctx: ResolutionCtx): ResolvedSnippet { - const id = ctx.getUniqueName(this); + const id = ctx.makeUniqueIdentifier(getName(this), 'global'); const { group, binding } = ctx.allocateFixedEntry( isWgslStorageTexture(this.schema) ? { @@ -642,7 +642,7 @@ export class TgpuLaidOutTextureViewImpl } [$resolve](ctx: ResolutionCtx): ResolvedSnippet { - const id = ctx.getUniqueName(this); + const id = ctx.makeUniqueIdentifier(getName(this), 'global'); const pre = `var<${this.#scope}> ${id}: ${ctx.resolve(this.#dataType).value}`; if (this.#initialValue) { diff --git a/packages/typegpu/src/data/autoStruct.ts b/packages/typegpu/src/data/autoStruct.ts index 647d13b79c..4fc745cccb 100644 --- a/packages/typegpu/src/data/autoStruct.ts +++ b/packages/typegpu/src/data/autoStruct.ts @@ -1,5 +1,5 @@ import { createIoSchema } from '../core/function/ioSchema.ts'; -import { isValidProp } from '../nameRegistry.ts'; +import { validateProp } from '../nameUtils.ts'; import { getName, setName } from '../shared/meta.ts'; import { $internal, $repr, $resolve } from '../shared/symbols.ts'; import type { ResolutionCtx, SelfResolvable } from '../types.ts'; @@ -75,8 +75,9 @@ export class AutoStruct implements BaseData, SelfResolvable { `Property name '${wgslKey}' causes naming clashes. Choose a different name.`, ); } - if (!isValidProp(wgslKey)) { - throw new Error(`Property key '${key}' is a reserved WGSL word. Choose a different name.`); + const result = validateProp(wgslKey); + if (!result.success) { + throw new Error(`Invalid property key '${key}'${result.error ? `: ${result.error}` : ''}`); } this.#usedWgslKeys.add(wgslKey); diff --git a/packages/typegpu/src/data/struct.ts b/packages/typegpu/src/data/struct.ts index c044da5f8f..24b400f2d4 100644 --- a/packages/typegpu/src/data/struct.ts +++ b/packages/typegpu/src/data/struct.ts @@ -1,4 +1,4 @@ -import { isValidProp } from '../nameRegistry.ts'; +import { validateProp } from '../nameUtils.ts'; import { getName, setName } from '../shared/meta.ts'; import { $internal } from '../shared/symbols.ts'; import { schemaCallWrapper } from './schemaCallWrapper.ts'; @@ -40,8 +40,9 @@ export function INTERNAL_createStruct>( isAbstruct: boolean, ): WgslStruct { Object.keys(props).forEach((key) => { - if (!isValidProp(key)) { - throw new Error(`Property key '${key}' is a reserved WGSL word. Choose a different name.`); + const result = validateProp(key); + if (!result.success) { + throw new Error(`Invalid property key '${key}'${result.error ? `: ${result.error}` : ''}`); } }); diff --git a/packages/typegpu/src/nameRegistry.ts b/packages/typegpu/src/nameUtils.ts similarity index 51% rename from packages/typegpu/src/nameRegistry.ts rename to packages/typegpu/src/nameUtils.ts index aa2cd1ada0..4ec4ea7b46 100644 --- a/packages/typegpu/src/nameRegistry.ts +++ b/packages/typegpu/src/nameUtils.ts @@ -1,6 +1,4 @@ -import { invariant } from './errors.ts'; - -const bannedTokens = new Set([ +export const bannedTokens = new Set([ // keywords 'alias', 'break', @@ -181,7 +179,7 @@ const bannedTokens = new Set([ 'storage', ]); -const builtins = new Set([ +export const builtins = new Set([ // constructors 'array', 'bool', @@ -361,198 +359,102 @@ const builtins = new Set([ 'quadSwapY', ]); -export interface NameRegistry { - /** - * Creates a valid WGSL identifier, each guaranteed to be unique - * in the lifetime of a single resolution process - * (excluding non-global identifiers from popped scopes). - * Should append "_" to primer, followed by some id. - * @param primer Used in the generation process, makes the identifier more recognizable. - * @param global Whether the name should be registered in the global scope (true), or in the current function scope (false) - */ - makeUnique(primer: string | undefined, global: boolean): string; - - /** - * Creates a valid WGSL identifier. - * Renames identifiers that are WGSL reserved words. - * @param primer Used in the generation process. - * - * @example - * makeValid("notAKeyword"); // "notAKeyword" - * makeValid("struct"); // makeUnique("struct") - * makeValid("struct_1"); // makeUnique("struct_1") (to avoid potential name collisions) - * makeValid("_"); // ERROR (too difficult to make valid to care) - */ - makeValid(primer: string): string; - - pushFunctionScope(): void; - popFunctionScope(): void; - pushBlockScope(): void; - popBlockScope(): void; -} - -function sanitizePrimer(primer: string | undefined) { +/*#__NO_SIDE_EFFECTS__*/ +export function sanitizePrimer(primer: string | undefined) { if (primer) { - // sanitizing - return primer + const base = primer .replaceAll(/\s/g, '_') // whitespaces .replaceAll(/[^\w\d]/g, ''); // removing illegal characters + + if (base === '_' || base === '' || base.startsWith('__')) { + return 'item'; + } + return base; } return 'item'; } +type ValidationResult = + | { + success: true; + error?: undefined; + } + | { + success: false; + error?: string | undefined; + }; + /** * A function for checking whether an identifier needs renaming. * Throws if provided with an invalid identifier that cannot be easily renamed. * @example - * isValidIdentifier("ident"); // true - * isValidIdentifier("struct"); // false - * isValidIdentifier("struct_1"); // false - * isValidIdentifier("_"); // ERROR - * isValidIdentifier("my variable"); // ERROR + * validateIdentifier("ident"); // { success: true } + * validateIdentifier("struct"); // { success: false, error: "Identifiers cannot start with reserved keywords." } + * validateIdentifier("struct_1"); { success: false, error: "Identifiers cannot start with reserved keywords." } + * validateIdentifier("_"); // { success: false } + * validateIdentifier("my variable"); // { success: false, error: "Identifiers cannot contain whitespace." } */ -function isValidIdentifier(ident: string): boolean { - if (ident === '_' || ident.startsWith('__') || /\s/.test(ident)) { - throw new Error( - `Invalid identifier '${ident}'. Choose an identifier without whitespaces or leading underscores.`, - ); +/*#__NO_SIDE_EFFECTS__*/ +export function validateIdentifier(ident: string): ValidationResult { + if (ident === '_') { + return { + success: false, + }; + } + if (/\s/.test(ident)) { + return { + success: false, + error: `Identifiers cannot contain whitespace.`, + }; + } + if (ident.startsWith('__')) { + return { + success: false, + error: `Identifiers cannot start with double underscores.`, + }; } const prefix = ident.split('_')[0] as string; - return !bannedTokens.has(prefix) && !builtins.has(prefix); + if (bannedTokens.has(prefix) || builtins.has(prefix)) { + return { + success: false, + error: `Identifiers cannot start with reserved keywords.`, + }; + } + return { + success: true, + }; } /** - * Same as `isValidIdentifier`, except does not check for builtin clashes. + * Same as `validateIdentifier`, except does not check for builtin clashes. */ -export function isValidProp(ident: string): boolean { - if (ident === '_' || ident.startsWith('__') || /\s/.test(ident)) { - throw new Error( - `Invalid identifier '${ident}'. Choose an identifier without whitespaces or leading underscores.`, - ); - } - const prefix = ident.split('_')[0] as string; - return !bannedTokens.has(prefix); -} -type FunctionScopeLayer = { - type: 'functionScope'; -}; - -type BlockScopeLayer = { - type: 'blockScope'; - usedBlockScopeNames: Set; -}; - -type ScopeLayer = FunctionScopeLayer | BlockScopeLayer; - -abstract class NameRegistryImpl implements NameRegistry { - abstract getUniqueVariant(base: string): string; - - readonly #usedNames: Set; - readonly #scopeStack: ScopeLayer[]; - - constructor() { - this.#usedNames = new Set([...bannedTokens, ...builtins]); - this.#scopeStack = []; +/*#__NO_SIDE_EFFECTS__*/ +export function validateProp(ident: string): ValidationResult { + if (ident === '_') { + return { + success: false, + }; } - - get #usedBlockScopeNames(): Set | undefined { - return (this.#scopeStack[this.#scopeStack.length - 1] as BlockScopeLayer | undefined) - ?.usedBlockScopeNames; + if (/\s/.test(ident)) { + return { + success: false, + error: `Identifiers cannot contain whitespace.`, + }; } - - makeUnique(primer: string | undefined, global: boolean): string { - const sanitizedPrimer = sanitizePrimer(primer); - const name = this.getUniqueVariant(sanitizedPrimer); - - if (global) { - this.#usedNames.add(name); - } else { - this.#usedBlockScopeNames?.add(name); - } - - return name; + if (ident.startsWith('__')) { + return { + success: false, + error: `Identifiers cannot start with double underscores.`, + }; } - - #isUsedInBlocksBefore(name: string): boolean { - const functionScopeIndex = this.#scopeStack.findLastIndex( - (scope) => scope.type === 'functionScope', - ); - return this.#scopeStack - .slice(functionScopeIndex + 1) - .some((scope) => (scope as BlockScopeLayer).usedBlockScopeNames.has(name)); - } - - makeValid(primer: string): string { - if ( - isValidIdentifier(primer) && - !this.#usedNames.has(primer) && - !this.#isUsedInBlocksBefore(primer) - ) { - this.#usedBlockScopeNames?.add(primer); - return primer; - } - return this.makeUnique(primer, false); - } - - isUsed(name: string): boolean { - return this.#usedNames.has(name) || this.#isUsedInBlocksBefore(name); - } - - pushFunctionScope(): void { - this.#scopeStack.push({ type: 'functionScope' }); - this.#scopeStack.push({ - type: 'blockScope', - usedBlockScopeNames: new Set(), - }); - } - - popFunctionScope(): void { - const functionScopeIndex = this.#scopeStack.findLastIndex( - (scope) => scope.type === 'functionScope', - ); - - if (functionScopeIndex === -1) { - throw new Error('Tried to pop function scope when no scope was present.'); - } - - this.#scopeStack.splice(functionScopeIndex); - } - - pushBlockScope(): void { - this.#scopeStack.push({ - type: 'blockScope', - usedBlockScopeNames: new Set(), - }); - } - popBlockScope(): void { - invariant( - this.#scopeStack[this.#scopeStack.length - 1]?.type === 'blockScope', - 'Tried to pop block scope, but it is not present', - ); - this.#scopeStack.pop(); - } -} - -export class RandomNameRegistry extends NameRegistryImpl { - #lastUniqueId = 0; - - getUniqueVariant(base: string): string { - let name = `${base}_${this.#lastUniqueId++}`; - while (this.isUsed(name)) { - name = `${base}_${this.#lastUniqueId++}`; - } - return name; - } -} - -export class StrictNameRegistry extends NameRegistryImpl { - getUniqueVariant(base: string): string { - let index = 0; - let name = base; - while (this.isUsed(name)) { - index++; - name = `${base}_${index}`; - } - return name; + const prefix = ident.split('_')[0] as string; + if (bannedTokens.has(prefix)) { + return { + success: false, + error: `Identifiers cannot start with reserved keywords.`, + }; } + return { + success: true, + }; } diff --git a/packages/typegpu/src/resolutionCtx.ts b/packages/typegpu/src/resolutionCtx.ts index 5eedec298d..2b986f6ebb 100644 --- a/packages/typegpu/src/resolutionCtx.ts +++ b/packages/typegpu/src/resolutionCtx.ts @@ -1,5 +1,5 @@ import { isTgpuFn } from './core/function/tgpuFn.ts'; -import { getUniqueName, type Namespace, type NamespaceInternal } from './core/resolve/namespace.ts'; +import type { Namespace, NamespaceInternal } from './core/resolve/namespace.ts'; import { stitch } from './core/resolve/stitch.ts'; import { ConfigurableImpl } from './core/root/configurableImpl.ts'; import type { Configurable, ExperimentalTgpuRoot } from './core/root/rootTypes.ts'; @@ -37,6 +37,7 @@ import { coerceToSnippet, concretize, numericLiteralToSnippet } from './tgsl/gen import type { ShaderGenerator } from './tgsl/shaderGenerator.ts'; import wgslGenerator from './tgsl/wgslGenerator.ts'; import type { + BlockScopeLayer, ExecMode, ExecState, FnToWgslOptions, @@ -59,6 +60,7 @@ import type { IOData } from './core/function/fnTypes.ts'; import { AutoStruct } from './data/autoStruct.ts'; import { EntryInputRouter } from './core/function/entryInputRouter.ts'; import type { FunctionArgument } from './tgsl/shaderGenerator_members.ts'; +import { validateIdentifier, sanitizePrimer } from './nameUtils.ts'; /** * Inserted into bind group entry definitions that belong @@ -99,6 +101,10 @@ class ItemStateStackImpl implements ItemStateStack { return this._stack.findLast((e) => e.type === 'functionScope'); } + get topBlockScope(): BlockScopeLayer | undefined { + return this._stack.findLast((e) => e.type === 'blockScope'); + } + pushItem() { this._itemDepth++; this._stack.push({ @@ -136,6 +142,7 @@ class ItemStateStackImpl implements ItemStateStack { pushBlockScope() { this._stack.push({ type: 'blockScope', + takenLocalIdentifiers: new Set(), declarations: new Map(), externals: new Map(), }); @@ -213,6 +220,26 @@ class ItemStateStackImpl implements ItemStateStack { return undefined; } + isIdentifierTakenLocally(id: string): boolean { + for (let i = this._stack.length - 1; i >= 0; --i) { + const layer = this._stack[i]; + + if (layer?.type === 'functionScope') { + // Since functions cannot access resources from the calling scope, we + // return early here. + return false; + } + + if (layer?.type === 'blockScope') { + if (layer.takenLocalIdentifiers.has(id)) { + return true; + } + } + } + + return false; + } + defineBlockVariable(id: string, snippet: Snippet): void { if (snippet.dataType === UnknownData) { throw Error(`Tried to define variable '${id}' of unknown type`); @@ -375,6 +402,11 @@ export class ResolutionCtxImpl implements ResolutionCtx { public readonly enableExtensions: WgslExtension[] | undefined; public expectedType: BaseData | undefined; + /** + * A counter used to generate unique identifiers for globally-scoped definitions in the 'random' strategy. + */ + #lastUniqueId = 0; + constructor(opts: ResolutionCtxImplOptions) { this.enableExtensions = opts.enableExtensions; this.gen = opts.shaderGenerator ?? wgslGenerator; @@ -382,12 +414,46 @@ export class ResolutionCtxImpl implements ResolutionCtx { this.#namespaceInternal = opts.namespace[$internal]; } - getUniqueName(resource: object): string { - return getUniqueName(this.#namespaceInternal, resource); + isIdentifierTaken(name: string): boolean { + return ( + this.#namespaceInternal.takenGlobalIdentifiers.has(name) || + this._itemStateStack.isIdentifierTakenLocally(name) + ); } - makeNameValid(name: string): string { - return this.#namespaceInternal.nameRegistry.makeValid(name); + makeUniqueIdentifier(primer: string = 'item', scope: 'global' | 'block'): string { + if ( + scope === 'block' && + validateIdentifier(primer).success && + !this.isIdentifierTaken(primer) + ) { + // Preserving local definitions as they are, provided they are valid and not already taken. + this.reserveIdentifier(primer, 'block'); + return primer; + } + + const base = sanitizePrimer(primer); + let index = 0; + const random = this.#namespaceInternal.strategy === 'random'; + let name = random ? `${base}_${this.#lastUniqueId++}` : base; + while (this.isIdentifierTaken(name)) { + name = random ? `${base}_${this.#lastUniqueId++}` : `${base}_${++index}`; + } + + this.reserveIdentifier(name, scope); + return name; + } + + reserveIdentifier(name: string, scope: 'global' | 'block'): void { + if (scope === 'block') { + const blockScope = this._itemStateStack.topBlockScope; + if (blockScope) { + blockScope.takenLocalIdentifiers.add(name); + return; + } + // Fall through if no block scope is present, treating as global. + } + this.#namespaceInternal.takenGlobalIdentifiers.add(name); } get pre(): string { @@ -441,12 +507,10 @@ export class ResolutionCtxImpl implements ResolutionCtx { } pushBlockScope() { - this.#namespaceInternal.nameRegistry.pushBlockScope(); this._itemStateStack.pushBlockScope(); } popBlockScope() { - this.#namespaceInternal.nameRegistry.popBlockScope(); this._itemStateStack.pop('blockScope'); } @@ -467,19 +531,24 @@ export class ResolutionCtxImpl implements ResolutionCtx { } fnToWgsl(options: FnToWgslOptions): { code: string; returnType: BaseData } { - let fnScopePushed = false; - try { - this.#namespaceInternal.nameRegistry.pushFunctionScope(); + const scope = this._itemStateStack.pushFunctionScope( + options.functionType, + {}, + options.returnType, + options.externalMap, + ); + // Pushing a block scope as well, so that any identifiers declared at this point will be scoped to the function body. + this._itemStateStack.pushBlockScope(); + const args: FunctionArgument[] = []; - const argAccess: Record = {}; if (options.entryInput) { const { dataSchema, positionalArgs } = options.entryInput; const firstParam = options.params[0]; const structArg = dataSchema - ? createArgument(this.makeNameValid('_arg_0'), dataSchema) + ? createArgument(this.makeUniqueIdentifier('_arg_0', 'block'), dataSchema) : undefined; if (structArg) { @@ -491,31 +560,31 @@ export class ResolutionCtxImpl implements ResolutionCtx { for (const { name, alias } of firstParam.props) { const argInfo = positionalArgs.find((a) => a.schemaKey === name); if (argInfo) { - const arg = createArgument(this.makeNameValid(alias), argInfo.type); + const arg = createArgument(this.makeUniqueIdentifier(alias, 'block'), argInfo.type); args.push(arg); - argAccess[alias] = arg.access; + scope.argAccess[alias] = arg.access; } else if (structArg) { - argAccess[alias] = createArgumentPropAccess(structArg.access, name); + scope.argAccess[alias] = createArgumentPropAccess(structArg.access, name); } } } else if (firstParam?.type === FuncParameterType.identifier) { // Create named arg snippets, then a proxy for property access routing. const proxyEntries: Array<{ schemaKey: string; arg: FunctionArgumentAccess }> = []; for (const a of positionalArgs) { - const argName = this.makeNameValid(`_arg_${a.schemaKey}`); + const argName = this.makeUniqueIdentifier(`_arg_${a.schemaKey}`, 'block'); const arg = createArgument(argName, a.type); args.push(arg); proxyEntries.push({ schemaKey: a.schemaKey, arg: arg.access }); } const router = new EntryInputRouter(structArg?.access, proxyEntries); - argAccess[firstParam.name] = () => snip('N/A', router, 'argument'); + scope.argAccess[firstParam.name] = () => snip('N/A', router, 'argument'); } else { // No first param: push positional args with schema key names. for (const a of positionalArgs) { - const argName = this.makeNameValid(`_arg_${a.schemaKey}`); + const argName = this.makeUniqueIdentifier(`_arg_${a.schemaKey}`, 'block'); const arg = createArgument(argName, a.type); args.push(arg); - argAccess[argName] = arg.access; + scope.argAccess[argName] = arg.access; } } } else { @@ -538,16 +607,24 @@ export class ResolutionCtxImpl implements ResolutionCtx { switch (astParam?.type) { case FuncParameterType.identifier: { - const arg = createArgument(this.makeNameValid(astParam.name), argType, origin); + const arg = createArgument( + this.makeUniqueIdentifier(astParam.name, 'block'), + argType, + origin, + ); args.push(arg); - argAccess[astParam.name] = arg.access; + scope.argAccess[astParam.name] = arg.access; break; } case FuncParameterType.destructuredObject: { - const objArg = createArgument(this.makeNameValid(`_arg_${i}`), argType, origin); + const objArg = createArgument( + this.makeUniqueIdentifier(`_arg_${i}`, 'block'), + argType, + origin, + ); args.push(objArg); for (const { name, alias } of astParam.props) { - argAccess[alias] = createArgumentPropAccess(objArg.access, name); + scope.argAccess[alias] = createArgumentPropAccess(objArg.access, name); } break; } @@ -557,7 +634,7 @@ export class ResolutionCtxImpl implements ResolutionCtx { // have any properties anyway. if (!(argType instanceof AutoStruct)) { args.push({ - name: this.makeNameValid(`_arg_${i}`), + name: this.makeUniqueIdentifier(`_arg_${i}`, 'block'), access: () => { throw new Error( `Unreachable: Accessing an argument that wasn't named in the function signature`, @@ -572,14 +649,6 @@ export class ResolutionCtxImpl implements ResolutionCtx { } } - const scope = this._itemStateStack.pushFunctionScope( - options.functionType, - argAccess, - options.returnType, - options.externalMap, - ); - fnScopePushed = true; - let returnType: BaseData | undefined; const code = this.gen.functionDefinition({ @@ -642,10 +711,8 @@ export class ResolutionCtxImpl implements ResolutionCtx { returnType, }; } finally { - if (fnScopePushed) { - this._itemStateStack.pop('functionScope'); - } - this.#namespaceInternal.nameRegistry.popFunctionScope(); + this._itemStateStack.pop('blockScope'); + this._itemStateStack.pop('functionScope'); } } diff --git a/packages/typegpu/src/tgsl/wgslGenerator.ts b/packages/typegpu/src/tgsl/wgslGenerator.ts index f76d4baccd..16d0ecf6f5 100644 --- a/packages/typegpu/src/tgsl/wgslGenerator.ts +++ b/packages/typegpu/src/tgsl/wgslGenerator.ts @@ -241,8 +241,12 @@ ${this.ctx.pre}}`; } } + public _blockStatement(block: tinyest.Block, externalMap?: ExternalMap): string { + return `${this.ctx.pre}${this._block(block, externalMap)}`; + } + public refVariable(id: string, dataType: wgsl.StorableData): string { - const varName = this.ctx.makeNameValid(id); + const varName = this.ctx.makeUniqueIdentifier(id, 'block'); const ptrType = ptrFn(dataType); const snippet = snip( new RefOperator(snip(varName, dataType, 'function'), ptrType), @@ -280,7 +284,11 @@ ${this.ctx.pre}}`; varOrigin = 'runtime'; } - const snippet = snip(this.ctx.makeNameValid(id), dataType, /* origin */ varOrigin); + const snippet = snip( + this.ctx.makeUniqueIdentifier(id, 'block'), + dataType, + /* origin */ varOrigin, + ); this.ctx.defineVariable(id, snippet); return snippet; } @@ -1035,7 +1043,7 @@ Try 'return ${typeStr}(${str});' instead. return this._statement(node); } // simplify 'if (true) {A} else {B}' to '{A}' - return `${this.ctx.pre}${this._block(blockifySingleStatement(node))}`; + return this._blockStatement(blockifySingleStatement(node)); } const consequent = this._block(blockifySingleStatement(consNode)); @@ -1168,7 +1176,7 @@ ${this.ctx.pre}else ${alternate}`; } if (statement[0] === NODE.block) { - return `${this.ctx.pre}${this._block(statement)}`; + return this._blockStatement(statement); } if (statement[0] === NODE.for) { @@ -1264,12 +1272,9 @@ ${this.ctx.pre}else ${alternate}`; const blocks = elements.map( (e, i) => - `${this.ctx.pre}// unrolled iteration #${i}\n${this.ctx.pre}${this._block( - blockified, - { - [originalLoopVarName]: e, - }, - )}`, + `${this.ctx.pre}// unrolled iteration #${i}\n${this._blockStatement(blockified, { + [originalLoopVarName]: e, + })}`, ); return blocks.join('\n'); @@ -1277,7 +1282,7 @@ ${this.ctx.pre}else ${alternate}`; this.#unrolling = false; - const index = this.ctx.makeNameValid('i'); + const index = this.ctx.makeUniqueIdentifier('i', 'block'); const forHeaderStr = stitch`${this.ctx.pre}for (var ${index} = ${range.start}; ${index} ${range.comparison} ${range.end}; ${index} += ${range.step})`; @@ -1290,7 +1295,7 @@ ${this.ctx.pre}else ${alternate}`; } else { this.ctx.indent(); ctxIndent = true; - const loopVarName = this.ctx.makeNameValid(originalLoopVarName); + const loopVarName = this.ctx.makeUniqueIdentifier(originalLoopVarName, 'block'); const elementSnippet = forOfUtils.getElementSnippet( iterableSnippet, snip(index, u32, 'runtime'), @@ -1304,7 +1309,7 @@ ${this.ctx.pre}else ${alternate}`; false, )};`; - bodyStr = `{\n${loopVarDeclStr}\n${this.ctx.pre}${this._block(blockified, { + bodyStr = `{\n${loopVarDeclStr}\n${this._blockStatement(blockified, { [originalLoopVarName]: snip(loopVarName, elementType, elementSnippet.origin), })}\n`; this.ctx.dedent(); diff --git a/packages/typegpu/src/types.ts b/packages/typegpu/src/types.ts index 75f9e71697..d2b7892faf 100644 --- a/packages/typegpu/src/types.ts +++ b/packages/typegpu/src/types.ts @@ -131,6 +131,7 @@ export type SlotBindingLayer = { export type BlockScopeLayer = { type: 'blockScope'; + takenLocalIdentifiers: Set; declarations: Map; externals: Map; }; @@ -140,6 +141,7 @@ export type StackLayer = ItemLayer | SlotBindingLayer | FunctionScopeLayer | Blo export interface ItemStateStack { readonly itemDepth: number; readonly topItem: ItemLayer; + readonly topBlockScope: BlockScopeLayer | undefined; readonly topFunctionScope: FunctionScopeLayer | undefined; pushItem(): void; @@ -330,8 +332,27 @@ export interface ResolutionCtx { */ withRenamed(item: object, name: string | undefined, callback: () => T): T; - getUniqueName(resource: object): string; - makeNameValid(name: string): string; + /** + * @param primer The basis for the unique identifier. Depending on the strategy, or + * the names already taken, this may be modified to ensure uniqueness. + * @param scope The scope in which to generate the identifier. 'global' means + * the identifier is meant to be unique across the entire program, while + * 'block' means it cannot shadow any existing identifiers visible from + * within the current block. After the block is popped, any identifiers + * defined within it are no longer visible. + * @returns an identifier that is unique within the given scope + */ + makeUniqueIdentifier(primer: string | undefined, scope: 'global' | 'block'): string; + + isIdentifierTaken(name: string): boolean; + + /** + * Makes sure the given identifier cannot be generated by {@link makeUniqueIdentifier} + * within the given scope. + * @param name The name to reserve + * @param scope See {@link makeUniqueIdentifier} for a description of the scope parameter. + */ + reserveIdentifier(name: string, scope: 'global' | 'block'): void; } /** diff --git a/packages/typegpu/tests/namespace.test.ts b/packages/typegpu/tests/namespace.test.ts index ee6fe33099..7ac6407b43 100644 --- a/packages/typegpu/tests/namespace.test.ts +++ b/packages/typegpu/tests/namespace.test.ts @@ -1,4 +1,4 @@ -import { describe, expect, vi } from 'vitest'; +import { describe, expect } from 'vitest'; import tgpu, { d } from '../src/index.js'; import { it } from 'typegpu-testing-utility'; @@ -71,34 +71,6 @@ describe('tgpu.namespace', () => { `); }); - it('fires "name" event', () => { - const Boid = d.struct({ - pos: d.vec3f, - }); - - const names = tgpu['~unstable'].namespace(); - - const listener = vi.fn((event) => {}); - names.on('name', listener); - - const code = tgpu.resolve([Boid], { names }); - - expect(listener).toHaveBeenCalledTimes(1); - expect(listener).toHaveBeenCalledWith({ name: 'Boid', target: Boid }); - - expect(code).toMatchInlineSnapshot(` - "struct Boid { - pos: vec3f, - }" - `); - - const code2 = tgpu.resolve([Boid], { names }); - - // No more events - expect(listener).toHaveBeenCalledTimes(1); - expect(code2).toMatchInlineSnapshot(`""`); - }); - it('handles name collision', () => { let code1: string, code2: string; const names = tgpu['~unstable'].namespace(); diff --git a/packages/typegpu/tests/renderPipeline.test.ts b/packages/typegpu/tests/renderPipeline.test.ts index be0c2888e4..4e66956ef4 100644 --- a/packages/typegpu/tests/renderPipeline.test.ts +++ b/packages/typegpu/tests/renderPipeline.test.ts @@ -1327,7 +1327,7 @@ describe('root.createRenderPipeline', () => { - - renderPipeline:pipeline - renderPipelineCore - - autoVertexFn: Invalid identifier '__myProp'. Choose an identifier without whitespaces or leading underscores.] + - autoVertexFn: Invalid property key '__myProp': Identifiers cannot start with double underscores.] `); }); @@ -1349,7 +1349,7 @@ describe('root.createRenderPipeline', () => { - - renderPipeline:pipeline - renderPipelineCore - - autoVertexFn: Property key 'loop' is a reserved WGSL word. Choose a different name.] + - autoVertexFn: Invalid property key 'loop': Identifiers cannot start with reserved keywords.] `); }); diff --git a/packages/typegpu/tests/resolve.test.ts b/packages/typegpu/tests/resolve.test.ts index 254730def2..83e8e1e787 100644 --- a/packages/typegpu/tests/resolve.test.ts +++ b/packages/typegpu/tests/resolve.test.ts @@ -1,6 +1,6 @@ import { describe, expect, vi } from 'vitest'; import tgpu, { d } from '../src/index.js'; -import { setName } from '../src/shared/meta.ts'; +import { getName, setName } from '../src/shared/meta.ts'; import { $gpuValueOf, $internal, $ownSnippet, $resolve } from '../src/shared/symbols.ts'; import type { ResolutionCtx } from '../src/types.ts'; import { it } from 'typegpu-testing-utility'; @@ -55,7 +55,7 @@ describe('tgpu resolve', () => { } as unknown as number, [$resolve](ctx: ResolutionCtx) { - const name = ctx.getUniqueName(this); + const name = ctx.makeUniqueIdentifier(getName(this), 'global'); ctx.addDeclaration(`@group(0) @binding(0) var ${name}: f32;`); return snip(name, d.f32, /* origin */ 'runtime'); }, diff --git a/packages/typegpu/tests/struct.test.ts b/packages/typegpu/tests/struct.test.ts index 14dc0984aa..ceb029af8a 100644 --- a/packages/typegpu/tests/struct.test.ts +++ b/packages/typegpu/tests/struct.test.ts @@ -374,13 +374,13 @@ describe('struct', () => { it('throws when struct prop has whitespace in name', () => { expect(() => struct({ 'my prop': f32 })).toThrowErrorMatchingInlineSnapshot( - `[Error: Invalid identifier 'my prop'. Choose an identifier without whitespaces or leading underscores.]`, + `[Error: Invalid property key 'my prop': Identifiers cannot contain whitespace.]`, ); }); it('throws when struct prop uses a reserved word', () => { expect(() => struct({ struct: f32 })).toThrowErrorMatchingInlineSnapshot( - `[Error: Property key 'struct' is a reserved WGSL word. Choose a different name.]`, + `[Error: Invalid property key 'struct': Identifiers cannot start with reserved keywords.]`, ); }); diff --git a/packages/typegpu/tests/tgsl/wgslGenerator.test.ts b/packages/typegpu/tests/tgsl/wgslGenerator.test.ts index 43f7b6ce77..e03c045144 100644 --- a/packages/typegpu/tests/tgsl/wgslGenerator.test.ts +++ b/packages/typegpu/tests/tgsl/wgslGenerator.test.ts @@ -566,7 +566,7 @@ describe('wgslGenerator', () => { `); }); - it('throws error when "for ... of ..." loop variable name is not correct in wgsl', () => { + it('renames "for ... of ..." loop variable name when it is not correct in WGSL', () => { const main = () => { 'use gpu'; const arr = [1, 2, 3]; @@ -575,11 +575,16 @@ describe('wgslGenerator', () => { } }; - expect(() => tgpu.resolve([main])).toThrowErrorMatchingInlineSnapshot(` - [Error: Resolution of the following tree failed: - - - - fn*:main - - fn*:main(): Invalid identifier '__foo'. Choose an identifier without whitespaces or leading underscores.] + expect(tgpu.resolve([main])).toMatchInlineSnapshot(` + "fn main() { + var arr = array(1, 2, 3); + for (var i = 0u; i < 3u; i += 1u) { + let item = arr[i]; + { + continue; + } + } + }" `); }); @@ -1166,24 +1171,24 @@ describe('wgslGenerator', () => { `); }); - it('throws when an identifier starts with underscores', () => { + it('assigns a different name when an identifier starts with underscores', () => { const main1 = tgpu.fn([])(() => { const _ = 1; }); const main2 = tgpu.fn([])(() => { - const __my_var = 1; + const __my_var = 2; }); - expect(() => tgpu.resolve([main1])).toThrowErrorMatchingInlineSnapshot(` - [Error: Resolution of the following tree failed: - - - - fn:main1: Invalid identifier '_'. Choose an identifier without whitespaces or leading underscores.] + expect(tgpu.resolve([main1])).toMatchInlineSnapshot(` + "fn main1() { + const item = 1; + }" `); - expect(() => tgpu.resolve([main2])).toThrowErrorMatchingInlineSnapshot(` - [Error: Resolution of the following tree failed: - - - - fn:main2: Invalid identifier '__my_var'. Choose an identifier without whitespaces or leading underscores.] + expect(tgpu.resolve([main2])).toMatchInlineSnapshot(` + "fn main2() { + const item = 2; + }" `); });