-
Notifications
You must be signed in to change notification settings - Fork 1k
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
[naga] constants aren't named expressions #7304
base: trunk
Are you sure you want to change the base?
Conversation
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 this was already clear to everyone else from the discussion in the issue, but I'll note that removing constants from named_expressions
has the side effect that they're less likely to appear in the output (see here), and if they do appear, they will appear without their names and not as const
.
Otherwise, this looks good to me, normally I would approve, but I think it would be good to have a stamp from somebody more familiar with the code than I am.
It would be nice to merge this soon, because it is going to conflict with the tests I am writing for my fix for #7356.
What Naga promises is to preserve your shader's behavior. What we aspire to is legible output that has a reasonable relation to what you wrote. So it's not really a blocker if local const expressions get removed. Note that everything with abstract type will always be removed. The rationale for omitting local consts is explained in #6207 (comment). This isn't the ideal solution, but it will do for now. |
Signed-off-by: sagudev <[email protected]>
…ession Signed-off-by: sagudev <[email protected]>
Signed-off-by: sagudev <[email protected]>
Signed-off-by: sagudev <[email protected]>
Rebased! |
Gentle ping @jimblandy |
Connections
Fix #6207
Description
Do not threat local constants as named expressions, and do not emit constable expr as constant in wgsl-out.
Testing
Existing tests
Squash or Rebase? Squash
Checklist
cargo fmt
.taplo format
.cargo clippy
. If applicable, add:--target wasm32-unknown-unknown
cargo xtask test
to run tests.CHANGELOG.md
entry.