[naga] parse @must_use attribute
#6801
Conversation
turbocrime
force-pushed
the
turbocrime/must_use
branch
2 times, most recently
from
17da613 to
1331f0f
Compare
turbocrime
changed the title
@must_use attribute@must_use attribute
turbocrime
force-pushed
the
turbocrime/must_use
branch
from
1331f0f to
201ce67
Compare
|
Looking at discussion in the
I expect that many particularities may complicate validation, so tests are provided for slightly complex valid examples. I'm unfamiliar with how naga's AST behaves, and looking through the validation code, it didn't seem obvious where/how to validate that a |
|
Are there likely to be important validation differences between constant and variable values? |
| pub struct FunctionResult<'a> { | ||
| pub ty: Handle<Type<'a>>, | ||
| pub binding: Option<Binding<'a>>, | ||
| pub must_use: bool, | ||
| } |
There was a problem hiding this comment.
must_use is a required member of wgsl ast FunctionResult.
| Some(ast::FunctionResult { | ||
| ty, | ||
| binding, | ||
| must_use, | ||
| }) |
There was a problem hiding this comment.
ast::FunctionResult creations are updated
naga/src/front/wgsl/parse/mod.rs
Outdated
| @@ -2455,6 +2460,7 @@ impl Parser { | |||
| let (mut bind_index, mut bind_group) = | |||
| (ParsedAttribute::default(), ParsedAttribute::default()); | |||
| let mut id = ParsedAttribute::default(); | |||
| let mut must_use = ParsedAttribute::default(); | |||
There was a problem hiding this comment.
attribute is handled in global_decl
is there another location in which @must_use might validly appear?
should invalid locations recognize the attribute at all?
possibly, some global declarations should not have the @must_use attribute. no checks are performed.
naga/src/front/wgsl/parse/mod.rs
Outdated
| @@ -2333,6 +2333,7 @@ impl Parser { | |||
| diagnostic_filter_leaf: Option<Handle<DiagnosticFilterNode>>, | |||
| out: &mut ast::TranslationUnit<'a>, | |||
| dependencies: &mut FastIndexSet<ast::Dependency<'a>>, | |||
| must_use: bool, | |||
There was a problem hiding this comment.
i added a parameter to function_decl for this attribute.
There was a problem hiding this comment.
these examples seem to be permitted, but as a rule never result in use of the return at runtime.
|
i'm going to revise this now that i can run conformance tests after #6840 it is not presently ready for review |
turbocrime
force-pushed
the
turbocrime/must_use
branch
from
201ce67 to
31b3e47
Compare
Signed-off-by: sagudev <[email protected]>
{"fail_fast": false, "matrix": [{"name": "WebGPU CTS", "workflow": "linux", "wpt_layout": "2020", "profile": "production", "unit_tests": false, "bencher": false, "wpt_args": "_webgpu"}]}
Unfortunately am not sure how relevant information from cts_runner actually is, as there is no concept of (bad) expectations (so only selected tests are run) and it uses old CTS version. The most useful results come from browsers running CTS, either Firefox or Servo as both run all test and have expectations properly set, so you know what results become different using your PR. In servo you can simply override wgpu in Cargo.toml (ensuring that your PR bases from same revision of wgpu previous used in servo) then run |
|
yes, realizing this since i patched things together. i was hoping to have canonical tests for edge cases and ambiguities, that could execute in a nice automated way, but it seems like the validation tests can't run in cts_runner without more api work. maybe this repo should remove cts_runner and the documentation that suggests using it, if it's no longer relevant. thanks for the instructions for executing in servo's ci! this is very useful |
not ready for review
to be revised now that i can run conformance tests
Connections
initial work on #5186
Description
This PR is based on an old commit, because it is the commit which Firefox nightly uses, but it doesn't seem to conflict with anything on the tip of main.
When the wgsl parser encounters the
@must_usefunction attribute, it should not error. This PR will recognize the attribute and emit a boolean flag in the AST.A skipped test is added for a single unacceptable example.
Several tests are added for various acceptable examples. They all pass, because the presence of the flag is a no-op, and no validation is implemented.
Testing
Checklist
cargo fmt.taplo format.cargo clippy. If applicable, add:--target wasm32-unknown-unknown--target wasm32-unknown-emscriptencargo xtask testto run tests.CHANGELOG.md. See simple instructions inside file.