[naga wgsl-in] Use common type-formatting code. #7344
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jimblandy
force-pushed
the
naga-remove-front-wgsl-to-wgsl
branch
2 times, most recently
from
4b73b54 to
57a3bb3
Compare
2 tasks
|
Marked as draft because it doesn't make sense to have an implementation for |
jimblandy
force-pushed
the
naga-remove-front-wgsl-to-wgsl
branch
2 times, most recently
from
2a288bb to
f3d88f5
Compare
|
Okay, this is much better, I think. The key commit, "Use common type-formatting code", is a lot simpler. Lesson: sometimes it's easiest just to give people what they need, rather than giving them the bare minimum and making them turn that into something they can use. |
jimblandy
force-pushed
the
naga-remove-front-wgsl-to-wgsl
branch
from
f3d88f5 to
2869602
Compare
In `naga::common::wgsl`, move `DisplayFilterableTriggeringRule` and diagnostics-related impls into a new sub-module, `diagonostics`. This change is only code motion; there should be no change in behavior.
In `naga::common::wgsl`, move the `ToWgsl` and `TryToWgsl` trait definitions and impls into their own submodule, `to_wgsl`. This change is only code motion; there should be no change in behavior.
Move the guts of the `naga::common::wgsl::TypeContext` trait's `write_type_inner` method into a new function, `try_write_type_inner`, private to the module. This commit is only code motion; there should be no change to behavior. Subsequent commits will introduce a private error type, with the new function serving as a `try` block.
In `naga::common::wgsl::types`, remove `unwrap_to_wgsl` from `try_write_type_inner` and replace its uses with calls to a new trait method, `write_scalar`, with a default definition. This should have no effect on externally visible behavior, but it makes subsequent commits easier to read.
Extend the `naga::common::wgsl::TypeContext` trait with new methods, `write_non_wgsl_inner` and `write_non_wgsl_scalar`, which are called when `write_type`, `write_type_inner`, or `write_scalar` are presented with Naga IR that cannot be expressed in WGSL. Delegating these cases lets most uses of `unreachable!` be removed. In the WGSL backend, implement `write_non_wgsl_inner` and `write_non_wgsl_scalar` to call `unreachable!`, preserving existing behavior.
Put the `use` directives at the top of `front::wgsl::error` in the usual order: - identifiers defined by this crate itself - identifiers defined by other crates - identifiers from the standard library This commit should cause no change in behavior.
Add a new default method, `write_type_resolution`, to the `naga::common::wgsl::TypeResolution` trait, for writing `TypeResolution` values as WGSL.
In `naga::common::wgsl::types`, move the type parameter `<W: Write>`
from the `TypeContext` trait itself to the individual methods of the
trait. It is very unlikely for Naga to need to be able to implement
`TypeContext`, but only for one particular kind of output stream.
The motivation for this change is that the old parameterization makes
it awkward to provide utility methods for generating `String`s on the
trait, which we do in subsequent commits. In order to write to a
`String`, such utility methods need `Self` to implement
`TypeContext<String>`, so you can add bounds to the methods like this:
fn type_to_string(..., out: &mut W) -> ...
where Self: TypeContext<String>
{
... self.write(..., &mut string_buf)?;
}
That will compile, but if you try to actually call it, Rust gets
confused. Remember, the above is not a method on
`TypeContext<String>`, it's a method on `TypeContext<W>` that uses
`TypeContext<String>` internally. So when you write
`ctx.type_to_string(...)`, Rust needs to decide whether `ctx`
implements `TypeContext<W>` for some completely unspecified `W`, and
asks for type annotations.
You could supply type annotations, but this would be basically
supplying some never-used type that implements `core::fmt::Write`.
Instead of
ctx.type_to_string(handle)
you'd have to write
TypeContext::<String>::type_to_string(ctx, handle)
which is dumb.
(I don't *think* this explanation belongs in the code, since it's an
explanation of a design *not* used, replaced by a design that's pretty
natural --- so I'll leave it here.)
Add new default methods, `type_to_string`, `type_inner_to_string`, and `resolution_to_string` to the `naga::common::wgsl::TypeResolution` trait, for generating WGSL source strings from these types.
Provide default implementations for the `write_non_wgsl_inner` and `write_non_wgsl_scalar` methods of `naga::common::wgsl::TypeContext`. We will be implementing this trait for various types in the WGSL front end and the validator, and almost all of these applications will need basically identical implementations of these methods.
Add a new method to the `naga::common::wgsl::TryToWgsl` trait, `to_wgsl_for_diagnostics`, which always returns a `String`, falling back to the `Debug` representation of the type if necessary. Provide a custom implementation for `Scalar` that shows abstract types in a reasonable way.
ErichDonGubler
force-pushed
the
naga-remove-front-wgsl-to-wgsl
branch
from
2869602 to
60232de
Compare
|
Resolved conflicts with the change of |
ErichDonGubler
approved these changes
Mar 19, 2025
Remove the `naga::front::wgsl::to_wgsl` module. Instead, use the implementations of `naga::common::wgsl`'s `TryToWgsl` and `TypeContext` traits.
ErichDonGubler
force-pushed
the
naga-remove-front-wgsl-to-wgsl
branch
from
60232de to
4c161e2
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Replace the type printing code in
front::wgsl::to_wgslwith calls to the shared type printing code incommon::wgsl. Use this new API throughout the front end. No change in behavior.Preparation for #6833.
This is meant to be reviewed commit by commit.