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

extensions/amd/shader_info: Replace unreachable ShaderInfoResult with 3 specialized functions #901

Merged
merged 1 commit into from
Apr 1, 2024

Conversation

MarijnS95
Copy link
Collaborator

Since the extension modularization in #894 this enum ShaderInfoResult is no longer reachable through the crate hierarchy, making it impossible for callers to match on its variants.

Not that this type was ideal to begin with: the returned enum variant depended purely on the info_type parameter, leading to ugly unreachable!()-like unwraps in caller code when the variant should always be of the type that a caller requested.

To solve both issues, create 3 instances of the get_shader_info() function for each of the 3 vk::ShaderInfoTypeAMDs that a caller can request.

…th 3 specialized functions

Since the extension modularization in #894 this `enum ShaderInfoResult`
is no longer reachable through the crate hierarchy, making it impossible
for callers to match on its variants.

Not that this type was ideal to begin with: the returned `enum`
variant depended purely on the `info_type` parameter, leading to ugly
`unreachable!()`-like unwraps in caller code when the variant should
always be of the type that a caller requested.

To solve both issues, create 3 instances of the `get_shader_info()`
function for each of the 3 `vk::ShaderInfoTypeAMD`s that a caller can
request.
@MarijnS95 MarijnS95 requested a review from Ralith March 31, 2024 21:24
@MarijnS95 MarijnS95 merged commit cd6e1ea into master Apr 1, 2024
20 checks passed
@MarijnS95 MarijnS95 deleted the amd-shader-info-separate-entry-points branch April 1, 2024 07:40
@MarijnS95 MarijnS95 added this to the Ash 0.38 milestone Dec 3, 2024
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