Skip to content

fix: preserve scopes around unrolled raw code - #2960

Open
oxura wants to merge 1 commit into
software-mansion:mainfrom
oxura:fix/2897-raw-block-pruning
Open

fix: preserve scopes around unrolled raw code#2960
oxura wants to merge 1 commit into
software-mansion:mainfrom
oxura:fix/2897-raw-block-pruning

Conversation

@oxura

@oxura oxura commented Sep 2, 2026

Copy link
Copy Markdown

Raw WGSL snippets are opaque to the generator, so the block-pruning pass could not see declarations inside them. During tgpu.unroll, that flattened every iteration into the function scope and produced invalid WGSL when a snippet declared the same name more than once.

This marks raw snippets as potentially declaring names, preserves that marker when snippets are copied, and feeds it into the existing definesInNearestScope decision. Each affected unrolled iteration now keeps its own brace scope; generated expressions that are not raw snippets retain the current pruning behavior.

Verification:

  • focused unroll/raw-snippet suites: 37 passed
  • TypeGPU type tests
  • changed-file Oxlint and Oxfmt checks
  • fast unit suite on Node 24: 2,809 passed, 2 skipped
  • typegpu package build

Closes #2897.

Copilot AI lite review requested due to automatic review settings September 2, 2026 08:20

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.

🔵 Needs a closer look

The change touches core WGSL generation/inlining behavior and warrants a final human review despite the added regression test coverage.

Pull request overview

This PR fixes invalid WGSL generation during tgpu.unroll when an unrolled loop body contains raw WGSL snippets that may declare names (e.g., const a = 1;). It does this by marking raw snippets as potentially introducing declarations and using that marker to prevent the existing block-pruning/inlining logic from removing necessary brace scopes around each unrolled iteration.

Changes:

  • Introduces a mayDeclareVariables marker on Snippet and preserves it through snippet-copy helpers.
  • Marks raw WGSL snippets (rawCodeSnippet) as mayDeclareVariables, so they are treated as scope-affecting during generation.
  • Feeds the marker into WGSL block inlining decisions via definesInNearestScope, and adds a regression test covering unrolled raw snippet scoping.
File summaries
File Description
packages/typegpu/tests/unroll.test.ts Adds regression test ensuring unrolled raw snippets keep per-iteration brace scopes.
packages/typegpu/src/tgsl/wgslGenerator.ts Uses expr.mayDeclareVariables to prevent inlining blocks that contain raw snippet statements.
packages/typegpu/src/data/snippet.ts Adds and propagates a mayDeclareVariables flag through snippet construction/copying.
packages/typegpu/src/core/rawCodeSnippet/tgpuRawCodeSnippet.ts Marks raw code snippet-generated Snippets as mayDeclareVariables.
Review details
  • Files reviewed: 4/4 changed files
  • Comments generated: 0
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@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.

✅ No new issues found.

Reviewed changes

  • snip.ts / snippet.ts — added a mayDeclareVariables?: true flag to Snippet, threaded it through the SnippetImpl constructor and all copy helpers (snip, withDataType, withValue, withSideEffects), installed via Object.defineProperty so it stays non-enumerable and absent when false.
  • tgpuRawCodeSnippet.ts — both resolve and codegenMode.getBaseSnippet now mark raw snippets as potentially declaring variables.
  • wgslGenerator.ts — the generic expression-statement handler now reports definesInNearestScope: expr.mayDeclareVariables ?? false, so a block containing a raw snippet as a bare statement is no longer flattened.
  • unroll.test.ts — regression snapshot asserting each unrolled iteration keeps its own brace scope.

The fix is clean and correctly targeted. Each unrolled iteration's body is generated via _blockStatement with allowInlining: true; a raw snippet statement now forces definesInNearestScope: true, so the iteration body is wrapped in {...} while _block returns definesInNearestScope: false for the wrapped block, keeping the outer unrolled loop inline-able. Function bodies use allowInlining: false, so there is no extra-brace regression at the top level, and GlslGenerator inherits the same fix. The new snapshot fails without the change, so it is genuine coverage.

The only trade-off is intentional and documented: all raw snippets are flagged conservatively even when they don't declare names, but since the flag is only read by the bare expression-statement path, normal codegen (let/const, if, expressions) is unaffected.

Pullfrog  | View workflow run | Using DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

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: Raw code snippet block pruning in unroll

2 participants