fix: validate comparison operand schemas - #2928
Open
huytdps13400 wants to merge 1 commit into
Open
Conversation
Reject mismatched vector kinds consistently in CPU execution and WGSL signature resolution so comparisons cannot emit invalid mixed-schema expressions.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR tightens runtime and WGSL generator validation for boolean comparison helpers so mixed vector kinds (e.g., vec3u vs vec3f) are rejected consistently before CPU execution or WGSL emission, aligning behavior with existing TypeScript overload constraints.
Changes:
- Added shared signature validation to reject mismatched operand vector kinds for
allEq,eq,ne,lt,le,gt, andge. - Added CPU-path guards for
eqandltto prevent executing comparisons on mixed-kinds vector instances. - Added a regression test ensuring both direct execution and
tgpu.resolvefail with the same unsupported-signature error for mismatched vector schemas.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| packages/typegpu/src/std/boolean.ts | Adds kind-matching validation in both signature resolution and CPU implementations for comparisons to prevent invalid mixed-kind comparisons/WGSL. |
| packages/typegpu/tests/std/boolean/lt.test.ts | Adds a regression test covering mismatched vector schemas for both CPU execution and generator resolution paths. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Contributor
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
- Removed the type-promiscuous
(...argTypes)signatures fromeq/ne/lt/le/gt/geandallEqinstd/boolean.ts, replacing them with a sharedcomparisonSignature/matchingArgTypeshelper backed byassertMatchingKinds, which throws on mismatched operand types at WGSL signature-resolution time. - Added the same
assertMatchingKinds(lhs.kind, rhs.kind)guard to the CPU pathscpuEq/cpuLt(shared byallEq,eq,ne,lt,le,gt,ge), since the direct runtime call does not go through signature resolution. - Added a
ltregression asserting both the CPU call and thetgpu.resolve([f])GPU path throw the same "Unsupported signature…" error for avec3uvsvec3fcomparison.
Notes
- The GPU path compares
lhs.type(schema string) while the CPU path compares.kind; both resolve to the same strings for vector instances, so the two paths stay consistent — the test pins the shared message. - I checked existing callers (
typegpu-radiance-cascades,typegpu-noise,std/boolean,core/root/init): all use same-kind operands and wrap scalar literal comparisons explicitly (e.g.d.vec2f(0)), so the stricter check breaks nothing. WGSL comparison operators require matching operand types, so rejecting scalar↔vector mixes aligns with valid WGSL. - The thrown error is a plain
Errorrather thanSignatureNotSupportedError; this matches the file's sibling CPU impls (cpuNot,select), so I'm not flagging it as a change.
DeepSeek Flash (free via Pullfrog for OSS) | 𝕏
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

Summary
The TypeScript overloads already require matching vector schemas, but the runtime and generator paths accepted mixed kinds and could emit invalid WGSL such as vec3u < vec3f.
TDD evidence
RED: the focused test failed because neither the direct lt call nor tgpu.resolve threw for vec3u versus vec3f.
GREEN: the focused test passes and both paths now return the same descriptive unsupported-signature error.
Verification
Fixes #2924