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

Add warning when using CompareFunction::*Equal without an invariant Attribute #2887

Merged
merged 2 commits into from
Jul 21, 2022

Conversation

cwfitzgerald
Copy link
Member

Checklist

  • Run cargo clippy.
  • Run RUSTFLAGS=--cfg=web_sys_unstable_apis cargo clippy --target wasm32-unknown-unknown if applicable.
  • Add change to CHANGELOG.md. See simple instructions inside file.

Description

This is a common issue that only actually manifests on specific sets of hardware (apple based gpus) and I know of no cases where this will be a false positive, so having a global warning for this is useful.

Testing

Tested by changing an example to use Equal.

@cwfitzgerald cwfitzgerald changed the title Invariant warning Add warning when using CompareFunction::*Equal without an invariant Attribute Jul 15, 2022
@cwfitzgerald cwfitzgerald requested a review from jimblandy July 15, 2022 15:54
@nical
Copy link
Contributor

nical commented Jul 20, 2022

Nice! would it make sense for the warning to be only emitted in debug builds?
Edit: I guess it's up to the application developer to not output warning in release builds and the subject of this warning looks like it is not likely to be a false positive.

@cwfitzgerald
Copy link
Member Author

Yeah I'm generally unconvinced that wgpu itself should be making any decisions (besides literal debug assertions) based off of debug_assertions - we should let the user decide what is best for them. In this case logging frameworks let you filter based on module as you please, so we should be okay here.

@cwfitzgerald cwfitzgerald merged commit 537c6be into gfx-rs:master Jul 21, 2022
@cwfitzgerald cwfitzgerald deleted the invariant-warning branch July 21, 2022 06:24
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.

2 participants