Clearer error messages for private functions with #[spirv(fragment)] attribute

[zanciks]

Working off of this ticket
Unsure about this one honestly. I do think that it works, but I'm having to clone item and I'm unsure if that's wanted.
Also not sure if the language used for the error is exactly what is wanted, but I do think that this technically works and it clearer than what was seen before.
This is also currently only detecting for fragment shaders just since only that was mentioned in the original ticket. However, it'd be pretty easy to add other checks (such as for #[spirv(vertex)]).

Any feedback is appreciated, thanks!

[zanciks]

Oops, forgot to run cargo fmt before. Sorry about that, new commit now.

[zanciks]

Hi again! Was feeling unpleased with this solution and wanted to go back to it.
Attempting to simply use if let instead of getting a string from the spirv macro.

// rust-gpu/crates/spirv-std/macros/src/lib.rs/spirv()
    // prepend with #[rust_gpu::spirv(..)]
    let attr: proc_macro2::TokenStream = attr.into();
    tokens.extend(quote! { #[cfg_attr(target_arch="spirv", rust_gpu::spirv(#attr))] });

-   if attr.to_string().trim() == "fragment" {
-       let item_clone = item.clone();
-       let input = syn::parse_macro_input!(item_clone as ItemFn);
-       if !matches!(input.vis, Visibility::Public(_)) {
-           panic!("The `spirv` macro can only be applied to public functions.");
-       }
-   }

+   if let Ok(item_fn) = syn::parse::<ItemFn>(item.clone()) {
+       if !matches!(item_fn.vis, Visibility::Public(_)) {
+           panic!("The `spirv` macro can only be applied to public functions.");
+       }
+   }

    let item: proc_macro2::TokenStream = item.into();
    for tt in item {

This works great! However, there are some functions instead of the spirv_std source that are not public. I don't know much about these functions, so I'd love to know if that was intentional, or if we can go ahead and make them public. I've pushed a new commit with those changes to be viewed. However, if the two function changes are not wanted, we can do it another way!

[zanciks]

Oops, apparently I missed a lot more private functions. I can fix this in the morning if that's the way that the code owners want to go with it

[LegNeato]

Thanks for the PR again! I'm so glad you chose us to continue your open source adventure 🍻.

For this one I think we'll want a different fix. Looking at #155, we do detect the case already somewhere, the message is just bad. The message also points out that there is a valid way to not have an entry point entirely ("Linkage"), though I am personally unfamiliar with that functionality.

So I would suggest enumerating the cases, finding the error message from #155, and see if the code at that point has enough info to differentiate the different cases and have better / more specific error messages. If not, figure out how to plumb the required information down to the point that message is displayed if possible.

Again, I am unfamiliar with this code personally so I don't know if this is still a quick fix or very involved...more exploration needed!

[zanciks]

So! As far as I can tell, nothing close to this error message shows up anywhere in the rust-gpu repo. However, the exact same error message showed up in another repo, SPIRV-Tools#2538. They fixed is by going from SPV_ENV_VULKAN_1_0 to SPV_ENV_UNIVERSAL_1_3 whatever that means. This makes me think that the error is probably on the vulkan or spirv side of things, rather than the rust-gpu side.

In addition to this, when I change functions from public to private in the rust-gpu examples, such as compute-shader or mouse-shader, the error I get when using cargo build -p [crate] is simply

error: function `main_fs` is never used
   --> examples/shaders/mouse-shader/src/lib.rs:140:4
    |
140 | fn main_fs(
    |    ^^^^^^^

which makes it seem even more to me like it isn't being checked on the rust side.
Going to keep investigating, but figured I'd share some results. Happy Thanksgiving all!