Skip to content

fix: Keep generated identifiers off the template-parameter enumerants - #3151

Open
dchaudhari7177 wants to merge 1 commit into
software-mansion:mainfrom
dchaudhari7177:fix/3065-restrict-template-keywords
Open

dchaudhari7177 wants to merge 1 commit into
software-mansion:mainfrom
dchaudhari7177:fix/3065-restrict-template-keywords

Conversation

@dchaudhari7177

Copy link
Copy Markdown

Fixes #3065.

Cause

WGSL's address spaces (function, private, workgroup, uniform, storage), access modes (read, write, read_write) and texel formats (rgba8unorm, …) are predeclared enumerants, and a declaration can shadow them. Template parameters refer to them by name: var<storage, read_write>, texture_storage_2d<rgba8unorm, write>. Before this change, a buffer named read_write came out as

@group(0) @binding(0) var<storage, read_write> read_write: u32;

and any later read_write in a template resolves to that variable. A local let write = ... does the same inside its function. uniform and storage were already in bannedTokens. The other enumerants weren't reserved anywhere.

Fix

  • nameUtils.ts gets a templateEnumerants set: the five address spaces, the three access modes, and every texel format in StorageTextureFormats (the 17 core WGSL formats plus the texture-formats-tier1 ones).
  • The namespace adds that set to takenGlobalIdentifiers, next to bannedTokens and builtins. makeUniqueIdentifier checks the set for both global and block scope, so module-scope and local names get a _1 suffix like the other reserved words.

I kept them out of bannedTokens on purpose. bannedTokens is also what validateProp checks for struct members and what function argument names are checked against, so adding them there would make d.struct({ read: d.u32 }) throw. A struct member never shadows a module-scope name, so read/write there are valid WGSL and keep working. If you'd rather have them in bannedTokens like uniform and storage, that's a one-line move.

Tests

tests/namespace.test.ts:

  • a mutable named read_write, a private var named rgba8unorm and a local write resolve to read_write_1, rgba8unorm_1 and write_1. On main this test fails: the names come out unsuffixed.
  • d.struct({ read: d.u32, write: d.u32 }) still resolves with its member names.

packages/typegpu: vitest run gives 192 files, 2733 passed. oxfmt is clean.

A module-scope or local declaration named like an address space, access
mode or texel format shadows that enumerant, so a later template such as
var<storage, read_write> would refer to the declaration. Reserve them in
the namespace so such names get a suffix. They stay valid as struct
member names, which shadow nothing.

Closes software-mansion#3065
Copilot AI balanced review requested due to automatic review settings October 3, 2026 09:53

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important

The new set misses the immediate address space — one TypeGPU itself emits as var<immediate> — so the shadowing bug this PR fixes is still reachable there.

Reviewed changes

  • templateEnumerants (packages/typegpu/src/nameUtils.ts) — adds the five core address spaces, three access modes, and 40 texel formats as reserved enumerants, deliberately kept out of bannedTokens so struct-member names stay legal.
  • Namespace wiring (packages/typegpu/src/core/resolve/namespace.ts) — seeds takenGlobalIdentifiers with the new set; makeUniqueIdentifier consults it for both global and block scopes.
  • Tests (packages/typegpu/tests/namespace.test.ts) — a snapshot that fails on main (mutable read_write, private var rgba8unorm, local write), plus a guard that struct members named read/write are untouched.

ℹ️ Nitpicks

  • The texel-format half of templateEnumerants re-lists the 40 members of StorageTextureFormats (packages/typegpu/src/core/texture/textureFormats.ts:14); a format added there in future won't be picked up here, silently reintroducing this bug for that format.

Pullfrog  | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using deepseek-v4.1-flash (free via Pullfrog for OSS) | 𝕏

Comment on lines +190 to +194
'function',
'private',
'workgroup',
'uniform',
'storage',

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The address-space block omits immediate, which TypeGPU itself emits as var<immediate> (packages/typegpu/src/tgsl/wgslGenerator.ts:221, packages/typegpu/src/core/immediate/immediateVar.ts:197). immediate is not a WGSL keyword, reserved word, or builtin, so a resource or local named immediate still shadows the enumerant — the exact bug this PR fixes for the other five address spaces.

Suggested change
'function',
'private',
'workgroup',
'uniform',
'storage',
'function',
'private',
'workgroup',
'uniform',
'storage',
'immediate',

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix: Restrict more keywords

2 participants