Migration from register_attr to register_tool (#926)

* Accept `#[rust_gpu::spirv()]` attributes rather than `#[spirv()]` in backend
* Implemented `#[spirv(..)]` proc macro attribute for all platforms that conditionally translates to `#[rust_gpu::spirv()]` based on platform
* Changed `SpirvBuilder` to always apply `register_tool(rust_gpu)` attribute to shader crates
* Updated docs
* Added changelog

Author: Sylvester Hesp

CHANGELOG.md

@@ -0,0 +1,22 @@
+# `rust-gpu` Changelog
+
+All notable changes to this project will be documented in this file.
+
+The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
+and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
+
+## [Unreleased]
+
+### Changed 🛠️
+
+- 🚨BREAKING🚨 Migrated from `register_attr` to `register_tool`. [More information](docs/src/migration-to-register-tool.md).
+
+## [0.4.0-alpha.15]
+
+### Added ⭐
+
+- Build-time check for nightly toolchain version to provide user-friendly error messages.
+
+### Changed 🛠️
+
+- Updated rust toolchain to `nightly-2022-08-29`.

crates/rustc_codegen_spirv/src/symbols.rs

@@ -1,7 +1,7 @@
 use crate::attr::{Entry, ExecutionModeExtra, IntrinsicType, SpirvAttribute};
 use crate::builder::libm_intrinsics;
 use rspirv::spirv::{BuiltIn, ExecutionMode, ExecutionModel, StorageClass};
-use rustc_ast::ast::{Attribute, Lit, LitIntType, LitKind, NestedMetaItem};
+use rustc_ast::ast::{AttrKind, Attribute, Lit, LitIntType, LitKind, NestedMetaItem};
 use rustc_data_structures::fx::FxHashMap;
 use rustc_span::symbol::{Ident, Symbol};
 use rustc_span::Span;
@@ -16,6 +16,7 @@ pub struct Symbols {
     // Used by `is_blocklisted_fn`.
     pub fmt_decimal: Symbol,
 
+    pub rust_gpu: Symbol,
     pub spirv: Symbol,
     pub spirv_std: Symbol,
     pub libm: Symbol,
@@ -373,6 +374,7 @@ impl Symbols {
         Self {
             fmt_decimal: Symbol::intern("fmt_decimal"),
 
+            rust_gpu: Symbol::intern("rust_gpu"),
             spirv: Symbol::intern("spirv"),
             spirv_std: Symbol::intern("spirv_std"),
             libm: Symbol::intern("libm"),
@@ -411,20 +413,44 @@ pub(crate) fn parse_attrs_for_checking<'a>(
     attrs: &'a [Attribute],
 ) -> impl Iterator<Item = Result<(Span, SpirvAttribute), ParseAttrError>> + 'a {
     attrs.iter().flat_map(move |attr| {
-        let (whole_attr_error, args) = if !attr.has_name(sym.spirv) {
-            // Use an empty vec here to return empty
-            (None, Vec::new())
-        } else if let Some(args) = attr.meta_item_list() {
-            (None, args)
-        } else {
-            (
-                Some(Err((
-                    attr.span,
-                    "#[spirv(..)] attribute must have at least one argument".to_string(),
-                ))),
-                Vec::new(),
-            )
+        let (whole_attr_error, args) = match attr.kind {
+            AttrKind::Normal(ref normal) => {
+                // #[...]
+                let s = &normal.item.path.segments;
+                if s.len() > 1 && s[0].ident.name == sym.rust_gpu {
+                    // #[rust_gpu ...]
+                    if s.len() != 2 || s[1].ident.name != sym.spirv {
+                        // #[rust_gpu::...] but not #[rust_gpu::spirv]
+                        (
+                            Some(Err((
+                                attr.span,
+                                "unknown `rust_gpu` attribute, expected `rust_gpu::spirv`"
+                                    .to_string(),
+                            ))),
+                            Vec::new(),
+                        )
+                    } else if let Some(args) = attr.meta_item_list() {
+                        // #[rust_gpu::spirv(...)]
+                        (None, args)
+                    } else {
+                        // #[rust_gpu::spirv]
+                        (
+                            Some(Err((
+                                attr.span,
+                                "#[rust_gpu::spirv(..)] attribute must have at least one argument"
+                                    .to_string(),
+                            ))),
+                            Vec::new(),
+                        )
+                    }
+                } else {
+                    // #[...] but not #[rust_gpu ...]
+                    (None, Vec::new())
+                }
+            }
+            AttrKind::DocComment(..) => (None, Vec::new()), // doccomment
         };
+
         whole_attr_error
             .into_iter()
             .chain(args.into_iter().map(move |ref arg| {

crates/spirv-builder/src/lib.rs

@@ -424,6 +424,8 @@ fn invoke_rustc(builder: &SpirvBuilder) -> Result<PathBuf, SpirvBuilderError> {
         // the default until https://github.com/rust-lang/rust/pull/93969).
         "-Zbinary-dep-depinfo".to_string(),
         "-Csymbol-mangling-version=v0".to_string(),
+        "-Zcrate-attr=feature(register_tool)".to_string(),
+        "-Zcrate-attr=register_tool(rust_gpu)".to_string(),
     ];
 
     let mut llvm_args = vec![];

crates/spirv-std/macros/src/lib.rs

@@ -77,7 +77,7 @@ use proc_macro2::{Delimiter, Group, Ident, Span, TokenTree};
 
 use syn::{punctuated::Punctuated, spanned::Spanned, ItemFn, Token};
 
-use quote::ToTokens;
+use quote::{quote, ToTokens};
 use std::fmt::Write;
 
 /// A macro for creating SPIR-V `OpTypeImage` types. Always produces a
@@ -138,9 +138,16 @@ pub fn Image(item: TokenStream) -> TokenStream {
     output.into()
 }
 
+/// Replaces all (nested) occurrences of the `#[spirv(..)]` attribute with
+/// `#[cfg_attr(target_arch="spirv", rust_gpu::spirv(..))]`.
 #[proc_macro_attribute]
-pub fn spirv(_attr: TokenStream, item: TokenStream) -> TokenStream {
-    let mut tokens = Vec::new();
+pub fn spirv(attr: TokenStream, item: TokenStream) -> TokenStream {
+    let mut tokens: Vec<TokenTree> = Vec::new();
+
+    // prepend with #[rust_gpu::spirv(..)]
+    let attr: proc_macro2::TokenStream = attr.into();
+    tokens.extend(quote! { #[cfg_attr(target_arch="spirv", rust_gpu::spirv(#attr))] });
+
     let item: proc_macro2::TokenStream = item.into();
     for tt in item {
         match tt {
@@ -153,7 +160,11 @@ pub fn spirv(_attr: TokenStream, item: TokenStream) -> TokenStream {
                                 && matches!(group.stream().into_iter().next(), Some(TokenTree::Ident(ident)) if ident == "spirv")
                                 && matches!(sub_tokens.last(), Some(TokenTree::Punct(p)) if p.as_char() == '#') =>
                         {
-                            sub_tokens.pop();
+                            // group matches [spirv ...]
+                            let inner = group.stream(); // group stream doesn't include the brackets
+                            sub_tokens.extend(
+                                quote! { [cfg_attr(target_arch="spirv", rust_gpu::#inner)] },
+                            );
                         }
                         _ => sub_tokens.push(tt),
                     }

crates/spirv-std/shared/src/lib.rs

@@ -1,5 +1,6 @@
 //! Small shared crate, to share definitions between `spirv-std`
 //! and `spirv-std-macros`.
+
 #![no_std]
 
 pub mod image_params;

crates/spirv-std/src/byte_addressable_buffer.rs

@@ -45,7 +45,7 @@ impl<'a> ByteAddressableBuffer<'a> {
     /// transmute)
     pub unsafe fn load<T>(&self, byte_index: u32) -> T {
         if byte_index + mem::size_of::<T>() as u32 > self.data.len() as u32 {
-            panic!("Index out of range")
+            panic!("Index out of range");
         }
         buffer_load_intrinsic(self.data, byte_index)
     }
@@ -71,7 +71,7 @@ impl<'a> ByteAddressableBuffer<'a> {
     /// transmute)
     pub unsafe fn store<T>(&mut self, byte_index: u32, value: T) {
         if byte_index + mem::size_of::<T>() as u32 > self.data.len() as u32 {
-            panic!("Index out of range")
+            panic!("Index out of range");
         }
         buffer_store_intrinsic(self.data, byte_index, value);
     }

crates/spirv-std/src/lib.rs

@@ -6,10 +6,8 @@
         asm_experimental_arch,
         core_intrinsics,
         lang_items,
-        register_attr,
         repr_simd,
-    ),
-    register_attr(spirv)
+    )
 )]
 // BEGIN - Embark standard lints v0.4
 // do not change or add/remove here, but one can add exceptions after this section
@@ -93,8 +91,9 @@
 //! Core functions, traits, and more that make up a "standard library" for SPIR-V for use in
 //! rust-gpu.
 
-#[cfg_attr(not(target_arch = "spirv"), macro_use)]
+#[macro_use]
 pub extern crate spirv_std_macros as macros;
+pub use macros::spirv;
 
 pub mod arch;
 pub mod byte_addressable_buffer;

docs/src/attributes.md

@@ -2,6 +2,12 @@
 
 rust-gpu introduces a number of SPIR-V related attributes to express behavior specific to SPIR-V not exposed in the base rust language.
 
+Before you'll able to use these attributes, make sure you import the attribute from the `spirv-std` crate:
+
+```rust
+use spirv_std::spirv;
+```
+
 There are a few different categories of attributes:
 
 ## Entry points

docs/src/migration-to-register-tool.md

@@ -0,0 +1,45 @@
+# Migration to `register_tool`
+
+This document applies to [PR#926](https://github.com/EmbarkStudios/rust-gpu/pull/926)
+
+## What happened
+
+In a [recent nightly Rust update](https://github.com/rust-lang/rust/commit/76dd5c58a011bb734ad5b8e96fc560374893bc8f), the `register_attr` feature was removed in favor of `register_tool`. Unfortunately, rust-gpu made use of this feature to register the `spirv` attribute.
+
+## What does this mean for you as a shader maintainer
+
+You'll need to import the `spirv` proc macro attribute from `spirv-std` in order for the rust compiler:
+
+```rust
+use spirv_std::spirv;
+```
+
+If your shader code already contains this line but is conditionally only included for non-spirv builds, like so:
+
+```rust
+#[cfg(not(target_arch = "spirv"))]
+use spirv_std::spirv;
+```
+
+please remove the conditional attribute.
+
+For this macro attribute to work correctly, it is important that `spirv` is visible in the global score and you use it like you used it before: `#[spirv(..)]`. An attempt to scope the attribute (such as `#[spirv_std::spirv(..)]`) will confuse the macro and likely fail.
+
+You'll also need to remove the `feature(register_attr)` and `register_attr(spirv)` attributes from your shader crates. If you're building using `SpirvBuilder`, you don't need to do anything else; the new `register_tool` is applied automatically. If not, you'll need to include these attributes instead:
+
+```rust
+#![feature(register_tool)]
+#![register_tool(rust_gpu)]
+```
+
+That's it. Your shaders should now compile like before.
+
+## Technical Background
+
+Unfortunately, since the new Rust nightly toolchain in September 2022, `register_attr(spirv)` can no longer be used to register a global `spirv` attribute. Without this registration, the compiler would simply complain about `spirv` being an unknown attribute. However, the alternative, `register_tool`, requires us to scope the attribute in a namespace. For instance, as we've chosen the `rust_gpu` namespace, this would mean that you'd need to start writing `#[rust_gpu::spirv(..)]` instead, which would be quite tedious and would break a lot of code. And it's not possible to `use` a name from a tool namespace to bring it into scope.
+
+Instead, we opted to implement a proc macro attribute called `spirv` instead[^1]. This macro attribute scans the item it is applied to, and translates any `#[spirv(..)]` it finds into `#[rust_gpu::spirv(..)]` which will be subsequently handled by the codegen backend. Because it is now a proc macro attribute exported from `spirv_std`, you need to do `use spirv_std::spirv` to make it globally visible in your crate. ***Note that we recommend using the `spirv` proc macro attribute itself rather than the `rust_gpu::spirv` attribute it translates to, as the latter is subject to change.***
+
+We've also added the `feature(register_tool)` and `register_tool(rust_gpu)` crate attributes by default when compiling through `SpirvBuilder`. This will silence any error that you would otherwise get for applying a `rust_gpu` scoped attribute.
+
+[^1]: This is not entirely true. In reality, the `spirv` proc macro attribute already existed, but only for non-spirv builds. It was used to turn the `#[spirv(..)]` attribute into a no-op. The proc macro is now used on all platforms, and it emits `#[cfg_attr(target_arch="spirv", rust_gpu::spirv(..))]` for each usage of `#[spirv(..)]`.

examples/shaders/compute-shader/src/lib.rs

@@ -1,16 +1,9 @@
-#![cfg_attr(
-    target_arch = "spirv",
-    feature(register_attr),
-    register_attr(spirv),
-    no_std
-)]
+#![cfg_attr(target_arch = "spirv", no_std)]
 // HACK(eddyb) can't easily see warnings otherwise from `spirv-builder` builds.
 #![deny(warnings)]
 
 use glam::UVec3;
-use spirv_std::glam;
-#[cfg(not(target_arch = "spirv"))]
-use spirv_std::macros::spirv;
+use spirv_std::{glam, spirv};
 
 // Adapted from the wgpu hello-compute example
 

examples/shaders/mouse-shader/src/lib.rs

@@ -1,24 +1,17 @@
-#![cfg_attr(
-    target_arch = "spirv",
-    no_std,
-    feature(register_attr),
-    register_attr(spirv)
-)]
+#![cfg_attr(target_arch = "spirv", no_std)]
 // HACK(eddyb) can't easily see warnings otherwise from `spirv-builder` builds.
 #![deny(warnings)]
 
 use core::f32::consts::PI;
 use glam::{const_vec4, vec2, vec3, Mat2, Vec2, Vec3, Vec4, Vec4Swizzles};
 use shared::*;
+use spirv_std::spirv;
 
 // Note: This cfg is incorrect on its surface, it really should be "are we compiling with std", but
 // we tie #[no_std] above to the same condition, so it's fine.
 #[cfg(target_arch = "spirv")]
 use spirv_std::num_traits::Float;
 
-#[cfg(not(target_arch = "spirv"))]
-use spirv_std::macros::spirv;
-
 trait Shape: Copy {
     /// Distances indicate where the point is in relation to the shape:
     /// * negative distance: the point is "inside" the shape

examples/shaders/reduce/src/lib.rs

@@ -1,17 +1,11 @@
-#![cfg_attr(
-    target_arch = "spirv",
-    no_std,
-    feature(register_attr),
-    register_attr(spirv)
-)]
+#![cfg_attr(target_arch = "spirv", no_std)]
 #![allow(clippy::too_many_arguments, clippy::missing_safety_doc)]
 // HACK(eddyb) can't easily see warnings otherwise from `spirv-builder` builds.
 #![deny(warnings)]
 use spirv_std::glam::UVec3;
-#[cfg(not(target_arch = "spirv"))]
-use spirv_std::macros::spirv;
 #[cfg(target_arch = "spirv")]
 use spirv_std::memory::Scope;
+use spirv_std::spirv;
 
 #[doc(alias = "OpGroupNonUniformIAdd")]
 #[cfg(target_arch = "spirv")]

examples/shaders/simplest-shader/src/lib.rs

@@ -1,16 +1,9 @@
-#![cfg_attr(
-    target_arch = "spirv",
-    no_std,
-    feature(register_attr),
-    register_attr(spirv)
-)]
+#![cfg_attr(target_arch = "spirv", no_std)]
 // HACK(eddyb) can't easily see warnings otherwise from `spirv-builder` builds.
 #![deny(warnings)]
 
-#[cfg(not(target_arch = "spirv"))]
-use spirv_std::macros::spirv;
-
 use shared::glam::{vec4, Vec4};
+use spirv_std::spirv;
 
 #[spirv(fragment)]
 pub fn main_fs(output: &mut Vec4) {

examples/shaders/sky-shader/src/lib.rs

@@ -1,20 +1,13 @@
 //! Ported to Rust from <https://github.com/Tw1ddle/Sky-Shader/blob/master/src/shaders/glsl/sky.fragment>
 
-#![cfg_attr(
-    target_arch = "spirv",
-    no_std,
-    feature(register_attr, lang_items),
-    register_attr(spirv)
-)]
+#![cfg_attr(target_arch = "spirv", no_std)]
 // HACK(eddyb) can't easily see warnings otherwise from `spirv-builder` builds.
 #![deny(warnings)]
 
-#[cfg(not(target_arch = "spirv"))]
-use spirv_std::macros::spirv;
-
 use core::f32::consts::PI;
 use glam::{const_vec3, vec2, vec3, Vec2, Vec3, Vec4};
 use shared::*;
+use spirv_std::spirv;
 
 // Note: This cfg is incorrect on its surface, it really should be "are we compiling with std", but
 // we tie #[no_std] above to the same condition, so it's fine.

tests/src/main.rs

@@ -123,8 +123,7 @@ impl Runner {
                 "--crate-type dylib",
                 "-Zunstable-options",
                 "-Zcrate-attr=no_std",
-                "-Zcrate-attr=feature(register_attr,asm_const,asm_experimental_arch)",
-                "-Zcrate-attr=register_attr(spirv)",
+                "-Zcrate-attr=feature(asm_const,asm_experimental_arch)",
             ]
             .join(" ")
         }
@@ -333,6 +332,8 @@ fn rust_flags(codegen_backend_path: &Path) -> String {
         "-Cembed-bitcode=no",
         &format!("-Ctarget-feature=+{}", target_features.join(",+")),
         "-Csymbol-mangling-version=v0",
+        "-Zcrate-attr=feature(register_tool)",
+        "-Zcrate-attr=register_tool(rust_gpu)",
     ]
     .join(" ")
 }

tests/ui/arch/all.rs

@@ -1,3 +1,5 @@
+use spirv_std::spirv;
+
 // build-pass
 
 #[spirv(fragment)]