Review Checklist

This is a collection of notes on things to watch out for when reviewing pull requests submitted to wgpu and Naga.

Ideally, we want to keep items off this list entirely:

• Using Rust effectively can turn some mistakes into compile-time errors. For example, in Naga, using exhaustive matching ensures that changes to the IR will cause compile-time errors in any code that hasn't been updated.

• Refactoring can gather together all the code responsible for enforcing some invariant in one place, making it clear whether a change preserves it or not. For example, Naga localizes all handle validation to naga::valid::Validator::validate_module_handles, allowing the rest of the validator to assume that all handles are valid.

• Offering custom abstractions can help contributors avoid implementing a weaker abstraction by themselves. For example, because HandleSet and HandleVec are used throughout Naga, contributors are less likely to write code that uses a BitSet or Vec on handle indices, which would invite bugs by erasing the handle types.

This checklist gathers up the concerns that we haven't found a satisfying way to address in a more robust way.

Naga

General

• If your change iterates over a collection, did you ensure the order of iteration was deterministic? Using HashMap and HashSet is fine, as long as you don't iterate over it.
• If you insert elements into a set or map that you expect are not already present, did you make an assertion about insert's return value?

WGSL Extensions

• If you added a new feature to WGSL that is not covered by the WebGPU specification:
  • Did you add a Capability flag for it?
  • Did you document the feature fully in that flag's doc comment?
  • Did you ensure the validator rejects programs that use the feature unless its capability is enabled?

IR changes

If your change adds or removes Handles from the IR:

• Did you update handle validation in valid::handles?
• Did you update the compactor in compact?
• Did you update back::pipeline_constants::adjust_expr?

If your change adds a new operation:

• Did you update the typifier in proc::typifier?
• Did you update the validator in valid::expression?
• If the operation can be used in constant expressions, did you update the constant evaluator in proc::constant_evaluator?

Backend changes

• If your change introduces any new identifiers to generated code, how did you ensure they won't conflict with the users' identifiers? (This is usually not relevant to the SPIR-V backend.)
  • Did you use the Namer to generate a fresh identifier?
  • Did you register the identifier as a reserved word with the Namer?
  • Did you use a reserved prefix registered with the Namer?