Skip to content

[naga wgsl-in] Short-circuiting of && and || operators #7339

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

Draft
wants to merge 1 commit into
base: trunk
Choose a base branch
from

Conversation

andyleiserson
Copy link
Contributor

@andyleiserson andyleiserson commented Mar 14, 2025

Connections
Possible fix for #4394 and #6302.

Description
The WGSL && and || operators should short-circuit, i.e., not evaluate their RHS if the result can be determined from the LHS alone. This is accomplished by transforming expressions with these operators into an if statement, guarding the evaluation of the RHS. In the case of nested expressions using these operators, it is necessary to emit nested if statements.

Things I am not sure about:

  • I add a function with_nested_runtime_expression_ctx to emit the expressions for the RHS within the if statement. I don't understand the code well enough yet to be confident this approach is sound.
  • I raised a few issues about short-circuiting of constant expressions in WGSL: Const. eval. short-circuiting #6302 (comment). The implementation for constant expressions in this PR is quite simple, but has the drawback that it omits nearly all validation of the RHS. So we'd get correct behavior for correct programs, but in exchange we'd accept some programs that aren't valid.
  • I think this is unlikely to be worth the effort, but we could attempt to undo this transformation in the backends that support short-circuit behavior of these operators.

Things that aren't done yet:

  • Implement this transformation in the glsl and hlsl front-ends.
  • Additional tests.

Testing
Adds one snapshot test, but more tests are needed.

Squash or Rebase?
TBD

Checklist

  • Run cargo fmt.
  • Run taplo format.
  • Run cargo clippy. If applicable, add:
    • --target wasm32-unknown-unknown
  • Run cargo xtask test to run tests.
  • Needs a CHANGELOG.md entry.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant