Don't require 1:1 mapping between OpTypeStruct
s and their names.
#406
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.
The linker deduplicates types based on their definition anyway (though maybe we should do less of that - see #398 (comment)), so the main side-effect of
SpirvType::Adt
having contained aname: String
was that some types would be considered different during codegen itself (even when otherwise compatible). Specifically, the added test used to fail with:But with this PR, a single type definition is generated, and it gets two
OpName
s, instead:(snippet obtained via
DUMP_PRE_LINK
, to avoid linker deduplication, #398, etc.)If we had some compile failure tests akin to
ui
tests inrust-lang/rust
, it would've been useful to see how bad the "show the first name" approach can get. If we think printingSpirvType
s in errors is relatively rare, we could show all names even etc.