[naga wgsl-in] Reject loads of types that contain atomics

[drakeo338]

Connections

Fixes #10046.

Description

apply_load_rule only rejected a load when the pointee was itself an atomic, so loading a struct or array that merely contained an atomic member sailed through the WGSL front end and hit unreachable!() in the HLSL backend. The fix walks the pointee type recursively through struct members and array bases so any nested atomic is caught as a normal WGSL error instead of a panic.

Testing

cargo test -p naga --all-features --test naga wgsl_errors::: 358 passed, 0 failed here, versus 356 passed, 0 failed on the merge-base with origin/trunk. The two extra cases are the new/updated regression tests, and both pass.

Squash or Rebase?

Single commit; fine to rebase as-is.

LLM Use?

An LLM-assisted agent helped locate the recursive-type-walk fix and write the accompanying test. I reviewed the diff, understand the root cause and the fix, and take responsibility for the code.

Checklist

• This PR was automatically created by an Agent.

• The description was automatically written by an Agent.

• I self-reviewed and fully understand this PR.

• CHANGELOG.md entries for the user-facing effects of this change are present.

• The PR is minimal, and doesn't make sense to land as multiple PRs. (not run as a check — judgment call, believed true but unverified against repo policy)

• Commits are logically scoped and individually reviewable. (single commit, not separately re-checked)

• The PR description has enough context to understand the motivation and solution implemented. (left for reviewers to judge)

• (If applicable) WebGPU implementations built with wgpu may be affected behaviorally. (not evaluated)

• (If applicable) Validation and feature gates are in place to confine behavioral changes. (not applicable, not verified)

• (If applicable) Tests demonstrate the validation and altered logic works. (cargo test -p naga --all-features --test naga wgsl_errors:: run on this HEAD, 358 passed 0 failed, verified discriminating against the pre-fix baseline)

[ErichDonGubler]

The fix itself LGTM, just a couple of nits.

[drakeo338]

I went looking for this before answering, rather than guessing. Checked out the CTS at the revision we pin (cts_runner/revision.txt, 83cbc838309ba7bddf6f3ec5c17574564773732b) and read every validation spec file that touches atomics: types/atomics.spec.ts, types/array.spec.ts, types/pointer.spec.ts, decl/var.spec.ts, decl/let.spec.ts, statement/phony.spec.ts, and functions/restrictions.spec.ts.

They cover atomic type declaration and parsing, address-space restrictions, direct access to a bare atomic variable, and constructibility of an atomic-containing struct as a function return value or in a phony assignment. None of them cover the actual path this fix closes: loading a whole value (struct or array) whose type recursively contains a nested atomic member, through a pointer, several levels of struct/array nesting deep. The closest case is functions/restrictions.spec.ts's function_return_types with the atomic_struct case, but that exercises value construction (atomic_struct()), not a Load off a pointer, so it's a different code path from the one that used to reach the backends and panic in HLSL.

So there's nothing to point a test.lst selector at for this specific regression — the naga-level unit test in wgsl_errors.rs is the actual coverage, and that's the right home for it anyway: per docs/testing.md, front-end-only WGSL validation with no GPU involved is a naga unit test, CTS is for things that need to run against a real backend.

Separately, and unrelated to this PR: webgpu:shader,validation,types,* is currently in fail.lst, not test.lst — it's not run in CI at all right now, noted at ~95% passing with a few known gaps including atomics. This PR doesn't touch anything those tests exercise, so it doesn't move that number either way. Promoting that whole selector is a bigger, separate job.

If you'd still like CTS-level coverage for this exact case, I can write a new case upstream in gpuweb/cts (extending phony.spec.ts or adding a case to types/atomics.spec.ts for a struct/array with a nested atomic being loaded) as a follow-up PR there. But I don't think it's needed to land this one.

[ErichDonGubler]

This change LGTM now. Congrats, your first wgpu contribution! 🎉

I want to publicly observe that there seems to have been a lot of relatively unedited LLM output with this PR. This makes it much harder to collaborate, because the review process is all about getting a clear signal that at change is beneficial enough to merge. Assuming that guess is right, note that in the future, we may reject such contributions on basis of too much shepherding effort vs. the time you yourself may have put in.

[ErichDonGubler]

@Mergifyio queue