Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(wgsl-in): include user and unknown rules in diagnostic(…) tracking #6537

Conversation

ErichDonGubler
Copy link
Member

@ErichDonGubler ErichDonGubler commented Nov 13, 2024

Connections

Description

Prior to this PR, unknown and user diagnostic(…) triggering rules for filters would get ignored, and certain important validations like this were not rejecting the program properly:

fn myfunc() {
	if (true) @diagnostic(off, my.lint) {
		//    ^^^^^^^^^^^^^^^^^^^^^^^^^ not yet supported, should report an error
	}
}
fn myfunc() {
	if (true) @diagnostic(off, wtf) {
		//    ^^^^^^^^^^^^^^^^^^^^^ should emit a warning
	}
}
diagnostic(off, my.lint);
diagnostic(warning, my.lint); // should report a conflict error
diagnostic(off, wat_is_this);
diagnostic(warning, wat_is_this); // should report a conflict error

Testing

  • Tests have been added for the above cases, and a bit more. N.B. that operation and validation coverage is missing for standard rule cases still.

Checklist

  • Run cargo fmt.
  • Run taplo format.
  • Run cargo clippy. If applicable, add:
    • --target wasm32-unknown-unknown
    • --target wasm32-unknown-emscripten
  • Run cargo xtask test to run tests.
  • Add change to CHANGELOG.md. See simple instructions inside file.

@ErichDonGubler ErichDonGubler changed the title feat(wgsl-in): use user and unknown rules to diagnostic(…) checks fix(wgsl-in): include user and unknown rules in diagnostic(…) tracking Nov 15, 2024
@ErichDonGubler ErichDonGubler force-pushed the non-std-diagnostics-validation branch 7 times, most recently from 89c07ae to 32dd4f5 Compare November 15, 2024 15:37
@ErichDonGubler ErichDonGubler self-assigned this Nov 15, 2024
@ErichDonGubler ErichDonGubler added area: validation Issues related to validation, diagnostics, and error handling area: correctness We're behaving incorrectly naga Shader Translator area: naga front-end lang: WGSL WebGPU Shading Language labels Nov 15, 2024
@ErichDonGubler ErichDonGubler marked this pull request as ready for review November 15, 2024 15:51
@ErichDonGubler ErichDonGubler requested a review from a team as a code owner November 15, 2024 15:51
Copy link
Member

@jimblandy jimblandy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me - thanks!

naga/src/diagnostic_filter.rs Outdated Show resolved Hide resolved
@ErichDonGubler ErichDonGubler enabled auto-merge (rebase) November 18, 2024 21:58
@ErichDonGubler ErichDonGubler merged commit e59f003 into gfx-rs:trunk Nov 18, 2024
27 checks passed
@ErichDonGubler ErichDonGubler deleted the non-std-diagnostics-validation branch November 18, 2024 22:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: correctness We're behaving incorrectly area: naga front-end area: validation Issues related to validation, diagnostics, and error handling lang: WGSL WebGPU Shading Language naga Shader Translator
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants