-
Notifications
You must be signed in to change notification settings - Fork 1.2k
[naga] Fix issues with const evaluation of bitwise shifts #8907
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
base: trunk
Are you sure you want to change the base?
Conversation
764a3c5 to
72cc87d
Compare
|
Looks like CI is unhappy. |
72cc87d to
da6fde6
Compare
|
The previous version was pretty broken (in that it would not raise the error in the cases it intended to) because the constant evaluator is only capable of resolving the type of constant expressions. So I had to move the check to the validator. I also fixed handling of 64-bit shift operands and added some tests in The substantive changes from the previous version are limited to the last commit. |
naga/tests/naga/wgsl_errors.rs
Outdated
| fn check_error_matches(input: &str, expected_substring: &str) { | ||
| use std::borrow::Cow; | ||
| let input = if input.trim_start().starts_with("var ") { | ||
| Cow::from(format!("fn foo() {{ {} }}", input)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think doing this here is unexpected. If we need this abstracted, adding a separate function that does this would be better.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, probably so. I removed it... when it comes at the overhead of an extra function call, it doesn't really feel like it's worth the characters it saves over writing fn foo() { } in each test case.
da6fde6 to
ed9f200
Compare
Fixes two issues with const evaluation of bitwise shifts:
Also includes, as a separate commit, cleanup of some noisy log messages in the naga expression validator.
Testing
Enables CTS tests.
Squash or Rebase? Rebase
Checklist
cargo fmt.taplo format.cargo clippy --tests. If applicable, add:--target wasm32-unknown-unknowncargo xtask testto run tests.CHANGELOG.mdentry.