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

Allow 64-bit hex literals and const-evaluate unary ops #6616

Merged
merged 4 commits into from
Nov 26, 2024

Conversation

SludgePhD
Copy link
Contributor

@SludgePhD SludgePhD commented Nov 25, 2024

Connections
Fixes #6615

Description
lu and li suffixes on hex literals now treat them as u64 and i64, respectively, to match the behavior with decimal literals.
Unary operations on 64-bit numbers can now be const-evaluated to allow negative literals.

Testing
The existing test was modified to use hex literals in addition to decimal literals.

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.

Sorry, something went wrong.

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.

LGTM.

@jimblandy jimblandy merged commit a9f14c7 into gfx-rs:trunk Nov 26, 2024
27 checks passed
@SludgePhD SludgePhD deleted the int64-fixes branch November 26, 2024 03:34
@ErichDonGubler
Copy link
Member

Is this functionality gated behind a feature? I'm concerned this might be exposed by default.

@SludgePhD
Copy link
Contributor Author

@ErichDonGubler the validation pass checks for u64 and i64 and fails unless the INT64 feature is enabled. I think that will catch all literals too?

@jimblandy
Copy link
Member

@ErichDonGubler the validation pass checks for u64 and i64 and fails unless the INT64 feature is enabled. I think that will catch all literals too?

@ErichDonGubler That's a great thing to bring up. But it looks to me like we're doing this right.

The right way to gate this is for the WGSL front end to accept everything, and for the validator to decide what's permitted and what is not. If I'm reading correctly, this should be working fine:

  • Validator::validate_const_expression and Validator::validate_expression both handle Expression::Literal by calling Validator::validate_literal.
  • Validator::validate_literal calls Validator::check_width.
  • Validator::check_width checks the right Capabilities flags for 64-bit floats and integer literals.

In Validator::validate_type, we also have calls to check_width for TypeInner::Scalar, Vector, Matrix, and ValuePointer. Atomic does its own width validation, checking different Capabilities flags for 64-bit atomics.

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.

0x8000000000000000 is not accepted as a u64 literal
3 participants