impr: Better error on JS array runtime access#2407
Conversation
|
pkg.pr.new packages benchmark commit |
📊 Bundle Size Comparison
👀 Notable resultsStatic test results:No major changes. Dynamic test results:No major changes. 📋 All resultsClick to reveal the results table (350 entries).
If you wish to run a comparison for other, slower bundlers, run the 'Tree-shake test' from the GitHub Actions menu. |
Resolution Time Benchmark---
config:
themeVariables:
xyChart:
plotColorPalette: "#E63946, #3B82F6, #059669"
---
xychart
title "Random Branching (🔴 PR | 🔵 main | 🟢 release)"
x-axis "max depth" [1, 2, 3, 4, 5, 6, 7, 8]
y-axis "time (ms)"
line [0.91, 1.88, 3.93, 6.30, 7.38, 9.03, 20.63, 20.27]
line [0.94, 1.87, 3.87, 6.46, 6.63, 10.77, 19.37, 21.29]
line [0.88, 1.78, 3.62, 6.20, 7.16, 10.35, 19.41, 23.08]
---
config:
themeVariables:
xyChart:
plotColorPalette: "#E63946, #3B82F6, #059669"
---
xychart
title "Linear Recursion (🔴 PR | 🔵 main | 🟢 release)"
x-axis "max depth" [1, 2, 3, 4, 5, 6, 7, 8]
y-axis "time (ms)"
line [0.28, 0.54, 0.71, 0.88, 1.15, 1.16, 1.40, 1.54]
line [0.28, 0.49, 0.67, 0.78, 1.13, 1.21, 1.42, 1.54]
line [0.29, 0.50, 0.66, 0.77, 1.06, 1.11, 1.40, 1.54]
---
config:
themeVariables:
xyChart:
plotColorPalette: "#E63946, #3B82F6, #059669"
---
xychart
title "Full Tree (🔴 PR | 🔵 main | 🟢 release)"
x-axis "max depth" [1, 2, 3, 4, 5, 6, 7, 8]
y-axis "time (ms)"
line [0.83, 1.83, 3.39, 6.39, 12.66, 25.26, 56.67, 113.29]
line [0.86, 2.00, 3.87, 6.10, 12.15, 24.53, 52.52, 106.51]
line [0.89, 1.97, 3.28, 6.19, 12.18, 24.02, 52.50, 106.33]
|
There was a problem hiding this comment.
Pull request overview
Improves the diagnostics produced during TGSL → WGSL resolution when user code attempts to index into JS/runtime values using a runtime-known index, making the resulting error actionable and easier to understand.
Changes:
- Update the index-access failure error message in
wgslGeneratorto include the full attempted access expression (target[index]). - Adjust
accessIndexbehavior to avoid “guessing” JS indexing when the index is not known at compile time, allowing the generator to emit a clearer error. - Update/add snapshot tests to validate the new error output for runtime index access.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| packages/typegpu/tests/tgsl/wgslGenerator.test.ts | Updates snapshots and adds coverage for the improved index-access error message. |
| packages/typegpu/src/tgsl/wgslGenerator.ts | Improves the thrown error message for invalid index-access scenarios. |
| packages/typegpu/src/tgsl/accessIndex.ts | Prevents JS-level indexing when the index is runtime-known, deferring to generator error handling. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
iwoplaza
left a comment
There was a problem hiding this comment.
Left a nit. Thanks for this fix!
Co-authored-by: Iwo Plaza <iwoplaza@gmail.com>
No description provided.