From ece29031f2881c0cf2b8a1e14355b0f0af126c43 Mon Sep 17 00:00:00 2001 From: mmoult <30711895+mmoult@users.noreply.github.com> Date: Thu, 13 Feb 2025 20:10:48 -0600 Subject: [PATCH 1/2] Remove errant underscore in AABB abbreviation AABB stands for Axis-Aligned Bounding Box. In one of the ray tracing flag enumerations, it is pluralized as AABBs. This confused the shouty snake case generation. There was already a special handling for NaN, add another case for AABBs and refactor the handling into its own function in utils. This compares to what has been done prior for converting strings into Ident. Update the autogen files to use the new naming rules. Format all code, which made small edits to code I didn't otherwise modify. --- autogen/src/binary.rs | 4 ++-- autogen/src/dr.rs | 4 +--- autogen/src/header.rs | 10 ++-------- autogen/src/main.rs | 3 ++- autogen/src/utils.rs | 11 ++++++++++- rspirv/binary/autogen_disas_operand.rs | 2 +- rspirv/dr/autogen_operand.rs | 2 +- rspirv/tests/spirv_blobs.rs | 3 ++- spirv/autogen_spirv.rs | 2 +- 9 files changed, 22 insertions(+), 19 deletions(-) diff --git a/autogen/src/binary.rs b/autogen/src/binary.rs index f880b8f9..c2258851 100644 --- a/autogen/src/binary.rs +++ b/autogen/src/binary.rs @@ -3,7 +3,7 @@ use std::collections::HashSet; use crate::structs; use crate::utils::*; -use heck::{ShoutySnakeCase, SnakeCase}; +use heck::SnakeCase; use proc_macro2::{Ident, TokenStream}; use quote::quote; @@ -168,7 +168,7 @@ fn gen_operand_param_parse_methods(grammar: &[structs::OperandKind]) -> Vec<(&st let decode = get_decode_method(element); quote! { dr::Operand::#op_kind(self.decoder.#decode()?) } }); - let bit = as_ident(&symbol.to_shouty_snake_case()); + let bit = as_ident(&as_shouty_snake_case(&symbol)); quote! { if #lo_kind.contains(spirv::#kind::#bit) { params.append(&mut vec![#(#params),*]); diff --git a/autogen/src/dr.rs b/autogen/src/dr.rs index bc5b4125..a3cf138f 100644 --- a/autogen/src/dr.rs +++ b/autogen/src/dr.rs @@ -394,9 +394,7 @@ pub fn gen_dr_operand_kinds(grammar: &[structs::OperandKind]) -> TokenStream { if seen_discriminator.get(&e.value).is_none() { let name = match category { structs::Category::BitEnum => { - use heck::ShoutySnakeCase; - - as_ident(&e.symbol.to_shouty_snake_case().replace("NA_N", "NAN")) + as_ident(&as_shouty_snake_case(&e.symbol)) } structs::Category::ValueEnum => { let name_str = if kind == "Dim" { diff --git a/autogen/src/header.rs b/autogen/src/header.rs index df61e728..9eb1afa3 100644 --- a/autogen/src/header.rs +++ b/autogen/src/header.rs @@ -1,7 +1,7 @@ use crate::structs; use crate::utils::*; -use heck::{ShoutySnakeCase, SnakeCase}; +use heck::SnakeCase; use proc_macro2::TokenStream; use quote::quote; use std::cmp::Ordering; @@ -102,13 +102,7 @@ fn gen_bit_enum_operand_kind(grammar: &structs::OperandKind) -> TokenStream { let mut additional_operands_list = vec![]; for enumerant in grammar.enumerants.iter() { - // Special treatment for "NaN" - let symbol = as_ident( - &enumerant - .symbol - .to_shouty_snake_case() - .replace("NA_N", "NAN"), - ); + let symbol = as_ident(&as_shouty_snake_case(&enumerant.symbol)); let value = enumerant.value; elements.push(quote! { diff --git a/autogen/src/main.rs b/autogen/src/main.rs index f0f5d749..41add121 100644 --- a/autogen/src/main.rs +++ b/autogen/src/main.rs @@ -9,7 +9,8 @@ mod table; mod utils; use std::{ - env, fs, + env, + fs, io::{Read, Write}, path::{Path, PathBuf}, process, diff --git a/autogen/src/utils.rs b/autogen/src/utils.rs index 5761ec8a..df555765 100644 --- a/autogen/src/utils.rs +++ b/autogen/src/utils.rs @@ -3,7 +3,7 @@ use crate::structs; use std::fs; use std::io::Write; -use heck::SnakeCase; +use heck::{ShoutySnakeCase, SnakeCase}; use proc_macro2::{Ident, Span, TokenStream}; use quote::quote; @@ -23,6 +23,15 @@ pub fn as_ident(ident: &str) -> Ident { Ident::new(ident, Span::call_site()) } +/// Returns an automatic conversion of the identifier to SHOUTY_SNAKE_CASE, +/// correcting known abbreviations as needed. +pub fn as_shouty_snake_case(ident: &str) -> String { + ident + .to_shouty_snake_case() + .replace("AAB_BS", "AABBS") // "AABBs" + .replace("NA_N", "NAN") // "NaN" +} + /// Returns the corresponding operand kind in data representation for the /// given operand `kind` in the grammar. pub fn get_dr_operand_kind(kind: &str) -> Ident { diff --git a/rspirv/binary/autogen_disas_operand.rs b/rspirv/binary/autogen_disas_operand.rs index 466836fe..9753e78d 100644 --- a/rspirv/binary/autogen_disas_operand.rs +++ b/rspirv/binary/autogen_disas_operand.rs @@ -353,7 +353,7 @@ impl Disassemble for spirv::RayFlags { if self.contains(spirv::RayFlags::SKIP_TRIANGLES_KHR) { bits.push("SkipTrianglesKHR") } - if self.contains(spirv::RayFlags::SKIP_AAB_BS_KHR) { + if self.contains(spirv::RayFlags::SKIP_AABBS_KHR) { bits.push("SkipAABBsKHR") } if self.contains(spirv::RayFlags::FORCE_OPACITY_MICROMAP2_STATE_EXT) { diff --git a/rspirv/dr/autogen_operand.rs b/rspirv/dr/autogen_operand.rs index c3832255..5b90a3dd 100644 --- a/rspirv/dr/autogen_operand.rs +++ b/rspirv/dr/autogen_operand.rs @@ -897,7 +897,7 @@ impl Operand { if v.intersects(s::RayFlags::FORCE_OPACITY_MICROMAP2_STATE_EXT) { result.extend_from_slice(&[spirv::Capability::RayTracingOpacityMicromapEXT]) }; - if v.intersects(s::RayFlags::SKIP_TRIANGLES_KHR | s::RayFlags::SKIP_AAB_BS_KHR) { + if v.intersects(s::RayFlags::SKIP_TRIANGLES_KHR | s::RayFlags::SKIP_AABBS_KHR) { result.extend_from_slice(&[spirv::Capability::RayTraversalPrimitiveCullingKHR]) }; result diff --git a/rspirv/tests/spirv_blobs.rs b/rspirv/tests/spirv_blobs.rs index f1f25647..40a13096 100644 --- a/rspirv/tests/spirv_blobs.rs +++ b/rspirv/tests/spirv_blobs.rs @@ -1,6 +1,7 @@ use rspirv::{ binary::{Assemble as _, Disassemble as _}, - dr, lift, + dr, + lift, }; use std::path::PathBuf; diff --git a/spirv/autogen_spirv.rs b/spirv/autogen_spirv.rs index b0d4c7b4..593e4838 100644 --- a/spirv/autogen_spirv.rs +++ b/spirv/autogen_spirv.rs @@ -15,7 +15,7 @@ bitflags! { # [doc = "SPIR-V operand kind: [FunctionControl](https://www.khronos bitflags! { # [doc = "SPIR-V operand kind: [MemorySemantics](https://www.khronos.org/registry/spir-v/specs/unified1/SPIRV.html#_a_id_memory_semantics_a_memory_semantics)"] # [derive (Clone , Copy , Debug , PartialEq , Eq , Hash)] # [cfg_attr (feature = "serialize" , derive (serde :: Serialize))] # [cfg_attr (feature = "deserialize" , derive (serde :: Deserialize))] pub struct MemorySemantics : u32 { const RELAXED = 0u32 ; const NONE = 0u32 ; const ACQUIRE = 2u32 ; const RELEASE = 4u32 ; const ACQUIRE_RELEASE = 8u32 ; const SEQUENTIALLY_CONSISTENT = 16u32 ; const UNIFORM_MEMORY = 64u32 ; const SUBGROUP_MEMORY = 128u32 ; const WORKGROUP_MEMORY = 256u32 ; const CROSS_WORKGROUP_MEMORY = 512u32 ; const ATOMIC_COUNTER_MEMORY = 1024u32 ; const IMAGE_MEMORY = 2048u32 ; const OUTPUT_MEMORY = 4096u32 ; const OUTPUT_MEMORY_KHR = 4096u32 ; const MAKE_AVAILABLE = 8192u32 ; const MAKE_AVAILABLE_KHR = 8192u32 ; const MAKE_VISIBLE = 16384u32 ; const MAKE_VISIBLE_KHR = 16384u32 ; const VOLATILE = 32768u32 ; } } bitflags! { # [doc = "SPIR-V operand kind: [MemoryAccess](https://www.khronos.org/registry/spir-v/specs/unified1/SPIRV.html#_a_id_memory_access_a_memory_access)"] # [derive (Clone , Copy , Debug , PartialEq , Eq , Hash)] # [cfg_attr (feature = "serialize" , derive (serde :: Serialize))] # [cfg_attr (feature = "deserialize" , derive (serde :: Deserialize))] pub struct MemoryAccess : u32 { const NONE = 0u32 ; const VOLATILE = 1u32 ; const ALIGNED = 2u32 ; const NONTEMPORAL = 4u32 ; const MAKE_POINTER_AVAILABLE = 8u32 ; const MAKE_POINTER_AVAILABLE_KHR = 8u32 ; const MAKE_POINTER_VISIBLE = 16u32 ; const MAKE_POINTER_VISIBLE_KHR = 16u32 ; const NON_PRIVATE_POINTER = 32u32 ; const NON_PRIVATE_POINTER_KHR = 32u32 ; const ALIAS_SCOPE_INTEL_MASK = 65536u32 ; const NO_ALIAS_INTEL_MASK = 131072u32 ; } } bitflags! { # [doc = "SPIR-V operand kind: [KernelProfilingInfo](https://www.khronos.org/registry/spir-v/specs/unified1/SPIRV.html#_a_id_kernel_profiling_info_a_kernel_profiling_info)"] # [derive (Clone , Copy , Debug , PartialEq , Eq , Hash)] # [cfg_attr (feature = "serialize" , derive (serde :: Serialize))] # [cfg_attr (feature = "deserialize" , derive (serde :: Deserialize))] pub struct KernelProfilingInfo : u32 { const NONE = 0u32 ; const CMD_EXEC_TIME = 1u32 ; } } -bitflags! { # [doc = "SPIR-V operand kind: [RayFlags](https://www.khronos.org/registry/spir-v/specs/unified1/SPIRV.html#_a_id_ray_flags_a_ray_flags)"] # [derive (Clone , Copy , Debug , PartialEq , Eq , Hash)] # [cfg_attr (feature = "serialize" , derive (serde :: Serialize))] # [cfg_attr (feature = "deserialize" , derive (serde :: Deserialize))] pub struct RayFlags : u32 { const NONE_KHR = 0u32 ; const OPAQUE_KHR = 1u32 ; const NO_OPAQUE_KHR = 2u32 ; const TERMINATE_ON_FIRST_HIT_KHR = 4u32 ; const SKIP_CLOSEST_HIT_SHADER_KHR = 8u32 ; const CULL_BACK_FACING_TRIANGLES_KHR = 16u32 ; const CULL_FRONT_FACING_TRIANGLES_KHR = 32u32 ; const CULL_OPAQUE_KHR = 64u32 ; const CULL_NO_OPAQUE_KHR = 128u32 ; const SKIP_TRIANGLES_KHR = 256u32 ; const SKIP_AAB_BS_KHR = 512u32 ; const FORCE_OPACITY_MICROMAP2_STATE_EXT = 1024u32 ; } } +bitflags! { # [doc = "SPIR-V operand kind: [RayFlags](https://www.khronos.org/registry/spir-v/specs/unified1/SPIRV.html#_a_id_ray_flags_a_ray_flags)"] # [derive (Clone , Copy , Debug , PartialEq , Eq , Hash)] # [cfg_attr (feature = "serialize" , derive (serde :: Serialize))] # [cfg_attr (feature = "deserialize" , derive (serde :: Deserialize))] pub struct RayFlags : u32 { const NONE_KHR = 0u32 ; const OPAQUE_KHR = 1u32 ; const NO_OPAQUE_KHR = 2u32 ; const TERMINATE_ON_FIRST_HIT_KHR = 4u32 ; const SKIP_CLOSEST_HIT_SHADER_KHR = 8u32 ; const CULL_BACK_FACING_TRIANGLES_KHR = 16u32 ; const CULL_FRONT_FACING_TRIANGLES_KHR = 32u32 ; const CULL_OPAQUE_KHR = 64u32 ; const CULL_NO_OPAQUE_KHR = 128u32 ; const SKIP_TRIANGLES_KHR = 256u32 ; const SKIP_AABBS_KHR = 512u32 ; const FORCE_OPACITY_MICROMAP2_STATE_EXT = 1024u32 ; } } bitflags! { # [doc = "SPIR-V operand kind: [FragmentShadingRate](https://www.khronos.org/registry/spir-v/specs/unified1/SPIRV.html#_a_id_fragment_shading_rate_a_fragment_shading_rate)"] # [derive (Clone , Copy , Debug , PartialEq , Eq , Hash)] # [cfg_attr (feature = "serialize" , derive (serde :: Serialize))] # [cfg_attr (feature = "deserialize" , derive (serde :: Deserialize))] pub struct FragmentShadingRate : u32 { const VERTICAL2_PIXELS = 1u32 ; const VERTICAL4_PIXELS = 2u32 ; const HORIZONTAL2_PIXELS = 4u32 ; const HORIZONTAL4_PIXELS = 8u32 ; } } #[doc = "SPIR-V operand kind: [SourceLanguage](https://www.khronos.org/registry/spir-v/specs/unified1/SPIRV.html#_a_id_source_language_a_source_language)"] #[repr(u32)] From 4583631a9fdaae1e4b947364c4b5c884f22a8cdd Mon Sep 17 00:00:00 2001 From: mmoult <30711895+mmoult@users.noreply.github.com> Date: Fri, 14 Feb 2025 19:19:37 -0600 Subject: [PATCH 2/2] Resolve clippy errors and catch extra shouty case Resolve the clippy and format errors to pass the CI. Also, replace a use of to_snake_case()...uppercase() with shouty_snake_case. --- autogen/src/binary.rs | 14 ++++---------- autogen/src/dr.rs | 5 +++-- autogen/src/structs.rs | 5 ++++- rspirv/binary/autogen_decode_operand.rs | 2 +- rspirv/binary/autogen_parse_operand.rs | 2 +- rspirv/binary/decoder.rs | 4 ++-- rspirv/binary/tracker.rs | 2 +- rspirv/sr/storage.rs | 8 ++++---- 8 files changed, 20 insertions(+), 22 deletions(-) diff --git a/autogen/src/binary.rs b/autogen/src/binary.rs index c2258851..c56f833a 100644 --- a/autogen/src/binary.rs +++ b/autogen/src/binary.rs @@ -111,7 +111,7 @@ pub fn gen_operand_decode_methods(grammar: &[structs::OperandKind]) -> TokenStre }); quote! { - impl<'a> Decoder<'a> { + impl Decoder<'_> { #(#methods)* } } @@ -168,7 +168,7 @@ fn gen_operand_param_parse_methods(grammar: &[structs::OperandKind]) -> Vec<(&st let decode = get_decode_method(element); quote! { dr::Operand::#op_kind(self.decoder.#decode()?) } }); - let bit = as_ident(&as_shouty_snake_case(&symbol)); + let bit = as_ident(&as_shouty_snake_case(symbol)); quote! { if #lo_kind.contains(spirv::#kind::#bit) { params.append(&mut vec![#(#params),*]); @@ -288,7 +288,7 @@ pub fn gen_operand_parse_methods(grammar: &[structs::OperandKind]) -> TokenStrea }); quote! { - impl<'c, 'd> Parser<'c, 'd> { + impl Parser<'_, '_> { fn parse_operand(&mut self, kind: GOpKind) -> Result> { Ok(match kind { #(#normal_cases),*, @@ -315,13 +315,7 @@ pub fn gen_disas_bit_enum_operands(grammar: &[structs::OperandKind]) -> TokenStr if enumerant.value == 0x0000 { None } else { - let symbol = as_ident( - &enumerant - .symbol - .to_snake_case() - .replace("na_n", "nan") - .to_uppercase(), - ); + let symbol = as_ident(&as_shouty_snake_case(&enumerant.symbol)); Some((quote! { #kind::#symbol }, &enumerant.symbol)) } }) diff --git a/autogen/src/dr.rs b/autogen/src/dr.rs index a3cf138f..9ac9330a 100644 --- a/autogen/src/dr.rs +++ b/autogen/src/dr.rs @@ -391,7 +391,8 @@ pub fn gen_dr_operand_kinds(grammar: &[structs::OperandKind]) -> TokenStream { let mut seen_discriminator = BTreeMap::new(); for e in enumerators { - if seen_discriminator.get(&e.value).is_none() { + use std::collections::btree_map::Entry; + if let Entry::Vacant(ve) = seen_discriminator.entry(e.value) { let name = match category { structs::Category::BitEnum => { as_ident(&as_shouty_snake_case(&e.symbol)) @@ -410,7 +411,7 @@ pub fn gen_dr_operand_kinds(grammar: &[structs::OperandKind]) -> TokenStream { _ => panic!("Unexpected operand type"), }; - seen_discriminator.insert(e.value, name.clone()); + ve.insert(name.clone()); capability_clauses .entry(&e.capabilities) diff --git a/autogen/src/structs.rs b/autogen/src/structs.rs index 138a895b..b3db0ff6 100644 --- a/autogen/src/structs.rs +++ b/autogen/src/structs.rs @@ -40,6 +40,7 @@ pub struct Enumerant { } #[derive(Debug, Deserialize, Clone)] +#[allow(dead_code)] pub struct OperandKind { pub category: Category, pub kind: String, @@ -53,6 +54,7 @@ pub struct OperandKind { #[derive(Debug, Deserialize)] pub struct Grammar { + #[allow(dead_code)] pub copyright: Vec, #[serde(deserialize_with = "num_or_hex")] pub magic_number: u32, @@ -64,6 +66,7 @@ pub struct Grammar { } #[derive(Debug, Deserialize)] +#[allow(dead_code)] pub struct ExtInstSetGrammar { pub copyright: Vec, pub version: u32, @@ -74,7 +77,7 @@ pub struct ExtInstSetGrammar { fn num_or_hex<'de, D: de::Deserializer<'de>>(d: D) -> result::Result { struct NumOrStr; - impl<'de> de::Visitor<'de> for NumOrStr { + impl de::Visitor<'_> for NumOrStr { type Value = u32; fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result { diff --git a/rspirv/binary/autogen_decode_operand.rs b/rspirv/binary/autogen_decode_operand.rs index 7821d94e..53aca282 100644 --- a/rspirv/binary/autogen_decode_operand.rs +++ b/rspirv/binary/autogen_decode_operand.rs @@ -2,7 +2,7 @@ // external/spirv.core.grammar.json. // DO NOT MODIFY! -impl<'a> Decoder<'a> { +impl Decoder<'_> { #[doc = "Decodes and returns the next SPIR-V word as\na SPIR-V ImageOperands value."] pub fn image_operands(&mut self) -> Result { if let Ok(word) = self.word() { diff --git a/rspirv/binary/autogen_parse_operand.rs b/rspirv/binary/autogen_parse_operand.rs index a8d4004a..a204192d 100644 --- a/rspirv/binary/autogen_parse_operand.rs +++ b/rspirv/binary/autogen_parse_operand.rs @@ -2,7 +2,7 @@ // external/spirv.core.grammar.json. // DO NOT MODIFY! -impl<'c, 'd> Parser<'c, 'd> { +impl Parser<'_, '_> { fn parse_operand(&mut self, kind: GOpKind) -> Result> { Ok(match kind { GOpKind::FPFastMathMode => vec![dr::Operand::FPFastMathMode( diff --git a/rspirv/binary/decoder.rs b/rspirv/binary/decoder.rs index 6e8e0136..48fc1c59 100644 --- a/rspirv/binary/decoder.rs +++ b/rspirv/binary/decoder.rs @@ -112,7 +112,7 @@ impl<'a> Decoder<'a> { } } -impl<'a> Decoder<'a> { +impl Decoder<'_> { /// Sets the limit to `num_words` words. /// /// The decoder will return [`State::LimitReached`](enum.ParseState.html) @@ -144,7 +144,7 @@ impl<'a> Decoder<'a> { } } -impl<'a> Decoder<'a> { +impl Decoder<'_> { /// Decodes and returns the next SPIR-V word as an id. pub fn id(&mut self) -> Result { self.word() diff --git a/rspirv/binary/tracker.rs b/rspirv/binary/tracker.rs index 9ca0af4a..d7689a10 100644 --- a/rspirv/binary/tracker.rs +++ b/rspirv/binary/tracker.rs @@ -116,7 +116,7 @@ impl ExtInstSetTracker { /// Returns true if the given extended instruction `set` has been /// recognized thus tracked. pub fn have(&self, set: spirv::Word) -> bool { - self.sets.get(&set).is_some() + self.sets.contains_key(&set) } /// Resolves the extended instruction with `opcode` in set `set`. diff --git a/rspirv/sr/storage.rs b/rspirv/sr/storage.rs index 4055f2d1..7e2daf18 100644 --- a/rspirv/sr/storage.rs +++ b/rspirv/sr/storage.rs @@ -112,8 +112,8 @@ mod tests { #[test] fn append_unique() { let mut storage: Storage = Storage::new(); - let t1 = storage.append(std::f64::NAN); - let t2 = storage.append(std::f64::NAN); + let t1 = storage.append(f64::NAN); + let t2 = storage.append(f64::NAN); assert!(t1 != t2); assert!(storage[t1] != storage[t2]); } @@ -130,8 +130,8 @@ mod tests { #[test] fn fetch_or_append_unique() { let mut storage: Storage = Storage::new(); - let t1 = storage.fetch_or_append(std::f64::NAN); - let t2 = storage.fetch_or_append(std::f64::NAN); + let t1 = storage.fetch_or_append(f64::NAN); + let t2 = storage.fetch_or_append(f64::NAN); assert!(t1 != t2); assert!(storage[t1] != storage[t2]); }