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

Use Rust-1.78 #[diagnostic::on_unimplemented] to clarify push_next trait #915

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

MarijnS95
Copy link
Collaborator

When a user passes an invalid type into push_next(), since Rust 1.78 we can now customize the diagnostics error to more clearly decsribe what and why they are not allowed to pass a certain type into this structure instead of having to refer to the trait definition (which contains no docs either).

This turns the following, rather obnoxious error:

error[E0277]: the trait bound `HdrMetadataEXT<'_>: ExtendsDeviceCreateInfo` is not satisfied
    --> ash-examples/src/lib.rs:336:42
     |
336  |             device_create_info.push_next(&mut vk::HdrMetadataEXT::default());
     |                                --------- ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `ExtendsDeviceCreateInfo` is not implemented for `HdrMetadataEXT<'_>`
     |                                |
     |                                required by a bound introduced by this call
     |
     = help: the following other types implement trait `ExtendsDeviceCreateInfo`:
               PhysicalDeviceDeviceGeneratedCommandsFeaturesNV<'_>
               PhysicalDeviceDeviceGeneratedCommandsComputeFeaturesNV<'_>
               DevicePrivateDataCreateInfo<'_>
               PhysicalDevicePrivateDataFeatures<'_>
               PhysicalDeviceFeatures2<'_>
               PhysicalDeviceVariablePointersFeatures<'_>
               PhysicalDeviceMultiviewFeatures<'_>
               DeviceGroupDeviceCreateInfo<'_>
             and 186 others
note: required by a bound in `DeviceCreateInfo::<'a>::push_next`
    --> /newdata/Code/TraverseResearch/ash/ash/src/vk/definitions.rs:1383:25
     |
1383 |     pub fn push_next<T: ExtendsDeviceCreateInfo + ?Sized>(mut self, next: &'a mut T) -> Self {
     |                         ^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `DeviceCreateInfo::<'a>::push_next`

For more information about this error, try `rustc --explain E0277`.
error: could not compile `ash-examples` (lib) due to 1 previous error

Into something more digestible:

error[E0277]: `HdrMetadataEXT<'_>` is not allowed in the `pNext` chain of `vk::DeviceCreateInfo`
    --> ash-examples/src/lib.rs:336:42
     |
336  |             device_create_info.push_next(&mut vk::HdrMetadataEXT::default());
     |                                --------- ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `HdrMetadataEXT<'_>` does not extend `vk::DeviceCreateInfo`
     |                                |
     |                                required by a bound introduced by this call
     |
     = help: the trait `ExtendsDeviceCreateInfo` is not implemented for `HdrMetadataEXT<'_>`
     = help: the following other types implement trait `ExtendsDeviceCreateInfo`:
               PhysicalDeviceDeviceGeneratedCommandsFeaturesNV<'_>
               PhysicalDeviceDeviceGeneratedCommandsComputeFeaturesNV<'_>
               DevicePrivateDataCreateInfo<'_>
               PhysicalDevicePrivateDataFeatures<'_>
               PhysicalDeviceFeatures2<'_>
               PhysicalDeviceVariablePointersFeatures<'_>
               PhysicalDeviceMultiviewFeatures<'_>
               DeviceGroupDeviceCreateInfo<'_>
             and 186 others
note: required by a bound in `DeviceCreateInfo::<'a>::push_next`
    --> /newdata/Code/TraverseResearch/ash/ash/src/vk/definitions.rs:1391:25
     |
1391 |     pub fn push_next<T: ExtendsDeviceCreateInfo + ?Sized>(mut self, next: &'a mut T) -> Self {
     |                         ^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `DeviceCreateInfo::<'a>::push_next`

For more information about this error, try `rustc --explain E0277`.
error: could not compile `ash-examples` (lib) due to 1 previous error

Please help me bike-shed the wording though! I was also hoping to find some upstream documentation (or a list of current extensions) based on the vk.xml structextends field, to reference in a note = attribute.

Copy link
Collaborator

@Ralith Ralith left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome idea! The wording looks good to me. Could we link to e.g. https://registry.khronos.org/vulkan/specs/1.3-extensions/man/html/VkDeviceCreateInfo.html#VUID-VkDeviceCreateInfo-pNext-pNext? Not sure that's much more helpful than just linking the trait rustdoc, but it's authoritative and cross-referenced.

@MarijnS95
Copy link
Collaborator Author

Awesome idea! The wording looks good to me. Could we link to e.g. https://registry.khronos.org/vulkan/specs/1.3-extensions/man/html/VkDeviceCreateInfo.html#VUID-VkDeviceCreateInfo-pNext-pNext? Not sure that's much more helpful than just linking the trait rustdoc, but it's authoritative and cross-referenced.

Good one, I didn't realize that the VUID was so front and center, and is easy to generate links to.

…` trait

When a user passes an invalid type into `push_next()`, [since Rust 1.78]
we can now customize the diagnostics error to more clearly decsribe what
and why they are not allowed to pass a certain type into this structure
instead of having to refer to the trait definition (which contains no
docs either).

[since Rust 1.78]: https://blog.rust-lang.org/2024/05/02/Rust-1.78.0.html#diagnostic-attributes
quote!(pub unsafe trait #extends_name {})
let message = format!("`{{Self}}` is not allowed in the `pNext` chain of `vk::{name}`");
let label = format!("`{{Self}}` does not extend `vk::{name}`");
let note = format!("See the list of allowed extension structs at <https://registry.khronos.org/vulkan/specs/1.3-extensions/man/html/{}.html#VUID-{}-pNext-pNext>", struct_.name, struct_.name);
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unsure if the link should be wrapped in <>, let me check some other diagnostics.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants