[Aquel32]

Closes #2708

• Implicit 'abstract-cast' that makes sure snippet value is valid for its type.
• 'abstract-cast' doesn't call explicit cast warning.
• Tests included

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.

An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing. Review the Copilot code review billing settings.

[pullfrog]

Important

The new abstractCast.test.ts will fail CI as committed: one inline snapshot is wrong (an empty function body renders on a single line), and oxfmt --check fails on the file. The src/ change itself is sound — the codegen output is correct.

( ◦°^°◦) Avast! I read the whole diff, ran the full TypeGPU test suite and the formatter, and spied two CI-breakin' snags in the new test file, matey.

Reviewed changes

This run reviewed the new abstract-cast conversion action and its tests.

• abstract-cast conversion action (packages/typegpu/src/tgsl/conversion.ts): getAutoConversionRank now emits a dedicated abstract-cast for abstractInt -> i32/u32; the action reuses the existing cast path, so negative or out-of-range abstract-int literals are wrapped into a valid concrete literal (-1 → 4294967295u) instead of generating invalid WGSL such as -1u.
• Conversion bookkeeping: findBestType and the action projection treat abstract-cast like cast when tracking targetType, and hasImplicitConversions deliberately counts only cast, so abstract casts no longer trip the implicit-conversion warning.
• Tests: new tests/abstractCast.test.ts (bitwise op, function argument, std.min, struct field, large int), plus updated tests/internal/getBestConversion.test.ts and one tests/tgsl/ternaryOperator.test.ts snapshot.

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

[pullfrog]

Important

The branch tip is still red: tests/abstractCast.test.ts fails because its inline snapshot expects a blank line inside the empty fn_1 body, but the resolver emits fn fn_1(_arg_0: u32) {} on one line. The earlier “Addressed in bc2f707” resolution only reformatted the file and left this snapshot stale — CI build-and-test fails on exactly this assertion at 1a959c7.

Reviewed changes

This run re-reviewed the commits since the last substantive review (d45ca49): a formatting pass on tests/abstractCast.test.ts and a merge of main.

• Reformatted tests/abstractCast.test.ts: normalized quotes, spacing, semicolons, and indentation; oxfmt --check now passes.
• Merged main into the branch: no authored change to the abstract-cast logic — the PR's conversion.ts delta is identical to the previously reviewed version.

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

[pullfrog]

✅ No new issues found.

( ◦°^°◦) 🏴‍☠️ Avast! I re-checked the new snapshot commit and the whole crew be green — tests pass and the formatter be happy, matey.

Reviewed changes

This run re-reviewed the commits since the prior pullfrog review (1a959c7d2): a merge of main and a snapshot fix on the branch tip.

• Corrected the stale inline snapshot in packages/typegpu/tests/abstractCast.test.ts: the empty fn_1 body now renders as fn fn_1(_arg_0: u32) {} on one line, matching resolver output. This is the exact fix the prior review asked for; npx vitest run tests/abstractCast.test.ts passes (5/5) and oxfmt --check is clean.
• Merged main into the branch — no authored change to the abstract-cast logic, so the previously reviewed conversion.ts delta is unchanged.

  ^{｜ View workflow run ｜ Using deepseek-v4.1-flash (free via Pullfrog for OSS) ｜ 𝕏}

[iwoplaza]

Looks good! Just include the JS behavior snapshot, and change the title of the PR to match our convention (e.g. fix: Better handling of abstract numeric casts)

[iwoplaza]

Could you also snapshot the JS behavior here?

Suggested change

	const fn = tgpu.fn([d.u32])(() => {
	'use gpu';
	});
	const main = () => {
	'use gpu';
	fn(-1);
	};
	const fn = tgpu.fn([d.u32])((x) => {
	'use gpu';
	return x;
	});
	const main = () => {
	'use gpu';
	return fn(-1);
	};
	expect(main()).toMatchInlineSnapshot(`4294967295`);

[pullfrog]

✅ No new issues found.

_{( ◦°^°◦) ✅ Avast! I re-checked the new JS-behavior snapshot and the whole crew be green — tests pass and the formatter be happy, matey.}

Reviewed changes

This run re-reviewed the commits since the prior pullfrog review (e22791ada): a new JS-behavior snapshot plus a merge of main.

• Added JS behavior snapshot (packages/typegpu/tests/abstractCast.test.ts): the "casts negative integer arguments to u32" test now declares fn as [d.u32] -> d.u32, has main return fn(-1), and asserts main() evaluates to 4294967295 on the JS side — exactly the coverage the human reviewer requested. The WGSL snapshot was updated to the -> u32 shape.
• Merged main into the branch (base 717142f) with no authored change to the abstract-cast logic.

Verified locally: npx vitest run tests/abstractCast.test.ts passes 5/5 and npx oxfmt --check packages/typegpu/tests/abstractCast.test.ts is clean.

  ^{｜ View workflow run ｜ Using deepseek-v4.1-flash (free via Pullfrog for OSS) ｜ 𝕏}

[aleksanderkatan]

Let's wait with this a bit, we accidentally introduced some weird behavior:

  it('...', () => {
    const fn = () => {
      'use gpu';
      const a = d.u32(1);
      return std.max(a, -1);
    };

    expect(fn()).toMatchInlineSnapshot(`1`);
    expect(tgpu.resolve([fn])).toMatchInlineSnapshot(`
      "fn fn_1() -> u32 {
        const a = 1u;
        return max(a, 4294967295u);
      }"
    `);
  });

  it('...', ({ root }) => {
    const buf = root.createUniform(d.u32, 1);
    const fn = () => {
      'use gpu';
      if (buf.$ > -1) {
        return true;
      }
      return false;
    };

    expect(tgpu.resolve([fn])).toMatchInlineSnapshot(`
      "@group(0) @binding(0) var<uniform> buf: u32;

      fn fn_1() -> bool {
        if ((buf > 4294967295u)) {
          return true;
        }
        return false;
      }"
    `);
  });

[pullfrog]

ℹ️ No critical issues — minor suggestions inline.

( ◦°^°◦) 🏴‍☠️ Avast! I re-checked the new warning commit, ran the whole TypeGPU suite, and spied one small polish in the new diagnostic, matey.

Reviewed changes

This run re-reviewed the single commit since the prior pullfrog review (1ed3f70e): the new implicit-abstract-conversion warning.

• Warn on value-changing abstract casts (packages/typegpu/src/tgsl/conversion.ts): convertToCommonType now records abstract-cast actions whose numeric value changes and emits an implicit-conversion warning naming the source literal, its type, and the target type.
• Warning coverage (packages/typegpu/tests/abstractCast.test.ts): added tests for std.max(a, -1) and buf > -1, pinning both the warning text and the generated WGSL.

  ^{｜ Fix all ➔ ｜ Fix 👍s ➔ ｜ View workflow run ｜ Using deepseek-v4.1-flash-1017 (free via Pullfrog for OSS) ｜ 𝕏}

[pullfrog]

Suggested change

	`Implicit abstract conversions from [\n${implicitAbstractConversions
	.map((c) => ` ${stringifySnippet(c.value)}: ${safeStringify(c.value.dataType)}`)
	`Implicit abstract conversions from [\n${implicitAbstractConversions
	.map((c) => ` ${stringifySnippet(c.value)} -> ${stringifySnippet(c.converted)}: ${safeStringify(c.value.dataType)}`)
	.join(',\n')}\n] to ${conversion.targetType.type} changes its value.
	Use an explicit conversion if this behavior is intended.`,