From 1c8d769b670533ef93bebfc5261ef4b3031903af Mon Sep 17 00:00:00 2001 From: Kevin Reid Date: Wed, 27 Aug 2025 20:42:02 -0700 Subject: [PATCH] [naga] Introduce `KeywordSet` and `CaseInsensitiveKeywordSet`. (#8136) --- CHANGELOG.md | 1 + naga/src/back/glsl/keywords.rs | 13 +-- naga/src/back/glsl/mod.rs | 2 +- naga/src/back/hlsl/keywords.rs | 19 ++-- naga/src/back/hlsl/writer.rs | 2 +- naga/src/back/msl/keywords.rs | 12 +-- naga/src/back/msl/writer.rs | 2 +- naga/src/back/wgsl/writer.rs | 2 +- naga/src/keywords/wgsl.rs | 12 +-- naga/src/proc/keyword_set.rs | 178 +++++++++++++++++++++++++++++++++ naga/src/proc/mod.rs | 2 + naga/src/proc/namer.rs | 77 +++----------- naga/src/racy_lock.rs | 16 +-- 13 files changed, 213 insertions(+), 125 deletions(-) create mode 100644 naga/src/proc/keyword_set.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index be4e1630a..56053f637 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -106,6 +106,7 @@ This allows using precompiled shaders without manually checking which backend's - Naga now requires that no type be larger than 1 GB. This limit may be lowered in the future; feedback on an appropriate value for the limit is welcome. By @andyleiserson in [#7950](https://github.com/gfx-rs/wgpu/pull/7950). - If the shader source contains control characters, Naga now replaces them with U+FFFD ("replacement character") in diagnostic output. By @andyleiserson in [#8049](https://github.com/gfx-rs/wgpu/pull/8049). - Add f16 IO polyfill on Vulkan backend to enable SHADER_F16 use without requiring `storageInputOutput16`. By @cryvosh in [#7884](https://github.com/gfx-rs/wgpu/pull/7884). +- For custom Naga backend authors: `naga::proc::Namer` now accepts reserved keywords using two new dedicated types, `proc::{KeywordSet, CaseInsensitiveKeywordSet}`. By @kpreid in [#8136](https://github.com/gfx-rs/wgpu/pull/8136). #### DX12 diff --git a/naga/src/back/glsl/keywords.rs b/naga/src/back/glsl/keywords.rs index ed217d9e6..aeaec5b03 100644 --- a/naga/src/back/glsl/keywords.rs +++ b/naga/src/back/glsl/keywords.rs @@ -1,7 +1,6 @@ +use crate::proc::KeywordSet; use crate::racy_lock::RacyLock; -use hashbrown::HashSet; - pub const RESERVED_KEYWORDS: &[&str] = &[ // // GLSL 4.6 keywords, from https://github.com/KhronosGroup/OpenGL-Registry/blob/d00e11dc1a1ffba581d633f21f70202051248d5c/specs/gl/GLSLangSpec.4.60.html#L2004-L2322 @@ -499,11 +498,5 @@ pub const RESERVED_KEYWORDS: &[&str] = &[ /// significant time during [`Namer::reset`](crate::proc::Namer::reset). /// /// See for benchmarks. -pub static RESERVED_KEYWORD_SET: RacyLock> = RacyLock::new(|| { - let mut set = HashSet::default(); - set.reserve(RESERVED_KEYWORDS.len()); - for &word in RESERVED_KEYWORDS { - set.insert(word); - } - set -}); +pub static RESERVED_KEYWORD_SET: RacyLock = + RacyLock::new(|| KeywordSet::from_iter(RESERVED_KEYWORDS)); diff --git a/naga/src/back/glsl/mod.rs b/naga/src/back/glsl/mod.rs index e78af74c8..4c5a9d8cb 100644 --- a/naga/src/back/glsl/mod.rs +++ b/naga/src/back/glsl/mod.rs @@ -663,7 +663,7 @@ impl<'a, W: Write> Writer<'a, W> { namer.reset( module, &keywords::RESERVED_KEYWORD_SET, - &[], + proc::CaseInsensitiveKeywordSet::empty(), &[ "gl_", // all GL built-in variables "_group", // all normal bindings diff --git a/naga/src/back/hlsl/keywords.rs b/naga/src/back/hlsl/keywords.rs index 7b2ab839d..13f48cef8 100644 --- a/naga/src/back/hlsl/keywords.rs +++ b/naga/src/back/hlsl/keywords.rs @@ -1,7 +1,6 @@ +use crate::proc::{CaseInsensitiveKeywordSet, KeywordSet}; use crate::racy_lock::RacyLock; -use hashbrown::HashSet; - // When compiling with FXC without strict mode, these keywords are actually case insensitive. // If you compile with strict mode and specify a different casing like "Pass" instead in an identifier, FXC will give this error: // "error X3086: alternate cases for 'pass' are deprecated in strict mode" @@ -927,17 +926,11 @@ pub const TYPES: &[&str] = &{ /// significant time during [`Namer::reset`](crate::proc::Namer::reset). /// /// See for benchmarks. -pub static RESERVED_SET: RacyLock> = RacyLock::new(|| { - let mut set = HashSet::default(); - set.reserve(RESERVED.len() + TYPES.len()); - for &word in RESERVED { - set.insert(word); - } - for &word in TYPES { - set.insert(word); - } - set -}); +pub static RESERVED_SET: RacyLock = + RacyLock::new(|| KeywordSet::from_iter(RESERVED.iter().chain(TYPES))); + +pub static RESERVED_CASE_INSENSITIVE_SET: RacyLock = + RacyLock::new(|| CaseInsensitiveKeywordSet::from_iter(RESERVED_CASE_INSENSITIVE)); pub const RESERVED_PREFIXES: &[&str] = &[ "__dynamic_buffer_offsets", diff --git a/naga/src/back/hlsl/writer.rs b/naga/src/back/hlsl/writer.rs index 357b85975..ab95b9327 100644 --- a/naga/src/back/hlsl/writer.rs +++ b/naga/src/back/hlsl/writer.rs @@ -155,7 +155,7 @@ impl<'a, W: fmt::Write> super::Writer<'a, W> { self.namer.reset( module, &super::keywords::RESERVED_SET, - super::keywords::RESERVED_CASE_INSENSITIVE, + &super::keywords::RESERVED_CASE_INSENSITIVE_SET, super::keywords::RESERVED_PREFIXES, &mut self.names, ); diff --git a/naga/src/back/msl/keywords.rs b/naga/src/back/msl/keywords.rs index e4f89bdc0..315af754e 100644 --- a/naga/src/back/msl/keywords.rs +++ b/naga/src/back/msl/keywords.rs @@ -1,7 +1,6 @@ +use crate::proc::KeywordSet; use crate::racy_lock::RacyLock; -use hashbrown::HashSet; - // MSLS - Metal Shading Language Specification: // https://developer.apple.com/metal/Metal-Shading-Language-Specification.pdf // @@ -364,11 +363,4 @@ pub const RESERVED: &[&str] = &[ /// significant time during [`Namer::reset`](crate::proc::Namer::reset). /// /// See for benchmarks. -pub static RESERVED_SET: RacyLock> = RacyLock::new(|| { - let mut set = HashSet::default(); - set.reserve(RESERVED.len()); - for &word in RESERVED { - set.insert(word); - } - set -}); +pub static RESERVED_SET: RacyLock = RacyLock::new(|| KeywordSet::from_iter(RESERVED)); diff --git a/naga/src/back/msl/writer.rs b/naga/src/back/msl/writer.rs index 9c0ac10d1..6fa5c135e 100644 --- a/naga/src/back/msl/writer.rs +++ b/naga/src/back/msl/writer.rs @@ -4279,7 +4279,7 @@ impl Writer { self.namer.reset( module, &super::keywords::RESERVED_SET, - &[], + proc::CaseInsensitiveKeywordSet::empty(), &[CLAMPED_LOD_LOAD_PREFIX], &mut self.names, ); diff --git a/naga/src/back/wgsl/writer.rs b/naga/src/back/wgsl/writer.rs index 8982242da..225a63343 100644 --- a/naga/src/back/wgsl/writer.rs +++ b/naga/src/back/wgsl/writer.rs @@ -101,7 +101,7 @@ impl Writer { module, &crate::keywords::wgsl::RESERVED_SET, // an identifier must not start with two underscore - &[], + proc::CaseInsensitiveKeywordSet::empty(), &["__", "_naga"], &mut self.names, ); diff --git a/naga/src/keywords/wgsl.rs b/naga/src/keywords/wgsl.rs index df464a10f..9bfce05d2 100644 --- a/naga/src/keywords/wgsl.rs +++ b/naga/src/keywords/wgsl.rs @@ -4,10 +4,9 @@ Keywords for [WGSL][wgsl] (WebGPU Shading Language). [wgsl]: https://gpuweb.github.io/gpuweb/wgsl.html */ +use crate::proc::KeywordSet; use crate::racy_lock::RacyLock; -use hashbrown::HashSet; - // https://gpuweb.github.io/gpuweb/wgsl/#keyword-summary // last sync: https://github.com/gpuweb/gpuweb/blob/39f2321f547c8f0b7f473cf1d47fba30b1691303/wgsl/index.bs pub const RESERVED: &[&str] = &[ @@ -238,11 +237,4 @@ pub const RESERVED: &[&str] = &[ /// significant time during [`Namer::reset`](crate::proc::Namer::reset). /// /// See for benchmarks. -pub static RESERVED_SET: RacyLock> = RacyLock::new(|| { - let mut set = HashSet::default(); - set.reserve(RESERVED.len()); - for &word in RESERVED { - set.insert(word); - } - set -}); +pub static RESERVED_SET: RacyLock = RacyLock::new(|| KeywordSet::from_iter(RESERVED)); diff --git a/naga/src/proc/keyword_set.rs b/naga/src/proc/keyword_set.rs new file mode 100644 index 000000000..c2ad5dd55 --- /dev/null +++ b/naga/src/proc/keyword_set.rs @@ -0,0 +1,178 @@ +use core::{fmt, hash}; + +use crate::racy_lock::RacyLock; +use crate::FastHashSet; + +/// A case-sensitive set of strings, +/// for use with [`Namer`][crate::proc::Namer] to avoid collisions with keywords and other reserved +/// identifiers. +/// +/// This is currently implemented as a hash table. +/// Future versions of Naga may change the implementation based on speed and code size +/// considerations. +#[derive(Clone, Debug, Default, Eq, PartialEq)] +pub struct KeywordSet(FastHashSet<&'static str>); + +impl KeywordSet { + /// Returns a new mutable empty set. + pub fn new() -> Self { + Self::default() + } + + /// Returns a reference to the empty set. + pub fn empty() -> &'static Self { + static EMPTY: RacyLock = RacyLock::new(Default::default); + &EMPTY + } + + /// Returns whether the set contains the given string. + #[inline] + pub fn contains(&self, identifier: &str) -> bool { + self.0.contains(identifier) + } +} + +impl Default for &'static KeywordSet { + fn default() -> Self { + KeywordSet::empty() + } +} + +impl FromIterator<&'static str> for KeywordSet { + fn from_iter>(iter: T) -> Self { + Self(iter.into_iter().collect()) + } +} + +/// Accepts double references so that `KeywordSet::from_iter(&["foo"])` works. +impl<'a> FromIterator<&'a &'static str> for KeywordSet { + fn from_iter>(iter: T) -> Self { + Self::from_iter(iter.into_iter().copied()) + } +} + +impl Extend<&'static str> for KeywordSet { + #[expect( + clippy::useless_conversion, + reason = "doing .into_iter() sooner reduces distinct monomorphizations" + )] + fn extend>(&mut self, iter: T) { + self.0.extend(iter.into_iter()) + } +} + +/// Accepts double references so that `.extend(&["foo"])` works. +impl<'a> Extend<&'a &'static str> for KeywordSet { + fn extend>(&mut self, iter: T) { + self.extend(iter.into_iter().copied()) + } +} + +/// A case-insensitive, ASCII-only set of strings, +/// for use with [`Namer`][crate::proc::Namer] to avoid collisions with keywords and other reserved +/// identifiers. +/// +/// This is currently implemented as a hash table. +/// Future versions of Naga may change the implementation based on speed and code size +/// considerations. +#[derive(Clone, Debug, Default, Eq, PartialEq)] +pub struct CaseInsensitiveKeywordSet(FastHashSet>); + +impl CaseInsensitiveKeywordSet { + /// Returns a new mutable empty set. + pub fn new() -> Self { + Self::default() + } + + /// Returns a reference to the empty set. + pub fn empty() -> &'static Self { + static EMPTY: RacyLock = RacyLock::new(Default::default); + &EMPTY + } + + /// Returns whether the set contains the given string, with comparison + /// by [`str::eq_ignore_ascii_case()`]. + #[inline] + pub fn contains(&self, identifier: &str) -> bool { + self.0.contains(&AsciiUniCase(identifier)) + } +} + +impl Default for &'static CaseInsensitiveKeywordSet { + fn default() -> Self { + CaseInsensitiveKeywordSet::empty() + } +} + +impl FromIterator<&'static str> for CaseInsensitiveKeywordSet { + fn from_iter>(iter: T) -> Self { + Self( + iter.into_iter() + .inspect(debug_assert_ascii) + .map(AsciiUniCase) + .collect(), + ) + } +} + +/// Accepts double references so that `CaseInsensitiveKeywordSet::from_iter(&["foo"])` works. +impl<'a> FromIterator<&'a &'static str> for CaseInsensitiveKeywordSet { + fn from_iter>(iter: T) -> Self { + Self::from_iter(iter.into_iter().copied()) + } +} + +impl Extend<&'static str> for CaseInsensitiveKeywordSet { + fn extend>(&mut self, iter: T) { + self.0.extend( + iter.into_iter() + .inspect(debug_assert_ascii) + .map(AsciiUniCase), + ) + } +} + +/// Accepts double references so that `.extend(&["foo"])` works. +impl<'a> Extend<&'a &'static str> for CaseInsensitiveKeywordSet { + fn extend>(&mut self, iter: T) { + self.extend(iter.into_iter().copied()) + } +} + +/// A string wrapper type with an ascii case insensitive Eq and Hash impl +#[derive(Clone, Copy)] +struct AsciiUniCase + ?Sized>(S); + +impl> fmt::Debug for AsciiUniCase { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + self.0.as_ref().fmt(f) + } +} + +impl> PartialEq for AsciiUniCase { + #[inline] + fn eq(&self, other: &Self) -> bool { + self.0.as_ref().eq_ignore_ascii_case(other.0.as_ref()) + } +} + +impl> Eq for AsciiUniCase {} + +impl> hash::Hash for AsciiUniCase { + #[inline] + fn hash(&self, hasher: &mut H) { + for byte in self + .0 + .as_ref() + .as_bytes() + .iter() + .map(|b| b.to_ascii_lowercase()) + { + hasher.write_u8(byte); + } + } +} + +fn debug_assert_ascii(s: &&'static str) { + debug_assert!(s.is_ascii(), "{s:?} not ASCII") +} diff --git a/naga/src/proc/mod.rs b/naga/src/proc/mod.rs index 413e49c1e..26f873a94 100644 --- a/naga/src/proc/mod.rs +++ b/naga/src/proc/mod.rs @@ -5,6 +5,7 @@ mod constant_evaluator; mod emitter; pub mod index; +mod keyword_set; mod layouter; mod namer; mod overloads; @@ -17,6 +18,7 @@ pub use constant_evaluator::{ }; pub use emitter::Emitter; pub use index::{BoundsCheckPolicies, BoundsCheckPolicy, IndexableLength, IndexableLengthError}; +pub use keyword_set::{CaseInsensitiveKeywordSet, KeywordSet}; pub use layouter::{Alignment, LayoutError, LayoutErrorInner, Layouter, TypeLayout}; pub use namer::{EntryPointIndex, ExternalTextureNameKey, NameKey, Namer}; pub use overloads::{Conclusion, MissingSpecialType, OverloadSet, Rule}; diff --git a/naga/src/proc/namer.rs b/naga/src/proc/namer.rs index 0b812ff03..38c171356 100644 --- a/naga/src/proc/namer.rs +++ b/naga/src/proc/namer.rs @@ -1,16 +1,15 @@ use alloc::{ borrow::Cow, - boxed::Box, format, string::{String, ToString}, vec::Vec, }; -use core::hash::{Hash, Hasher}; -use hashbrown::HashSet; -use once_cell::race::OnceBox; - -use crate::{arena::Handle, FastHashMap, FastHashSet}; +use crate::{ + arena::Handle, + proc::{keyword_set::CaseInsensitiveKeywordSet, KeywordSet}, + FastHashMap, +}; pub type EntryPointIndex = u16; const SEPARATOR: char = '_'; @@ -86,27 +85,15 @@ pub enum NameKey { /// This processor assigns names to all the things in a module /// that may need identifiers in a textual backend. +#[derive(Default)] pub struct Namer { /// The last numeric suffix used for each base name. Zero means "no suffix". unique: FastHashMap, - keywords: &'static HashSet<&'static str>, - keywords_case_insensitive: FastHashSet>, + keywords: &'static KeywordSet, + keywords_case_insensitive: &'static CaseInsensitiveKeywordSet, reserved_prefixes: Vec<&'static str>, } -impl Default for Namer { - fn default() -> Self { - static DEFAULT_KEYWORDS: OnceBox> = OnceBox::new(); - - Self { - unique: Default::default(), - keywords: DEFAULT_KEYWORDS.get_or_init(|| Box::new(HashSet::default())), - keywords_case_insensitive: Default::default(), - reserved_prefixes: Default::default(), - } - } -} - impl Namer { /// Return a form of `string` suitable for use as the base of an identifier. /// @@ -191,13 +178,11 @@ impl Namer { let mut suffixed = base.to_string(); if base.ends_with(char::is_numeric) || self.keywords.contains(base.as_ref()) - || self - .keywords_case_insensitive - .contains(&AsciiUniCase(base.as_ref())) + || self.keywords_case_insensitive.contains(base.as_ref()) { suffixed.push(SEPARATOR); } - debug_assert!(!self.keywords.contains::(&suffixed)); + debug_assert!(!self.keywords.contains(&suffixed)); // `self.unique` wants to own its keys. This allocates only if we haven't // already done so earlier. self.unique.insert(base.into_owned(), 0); @@ -228,8 +213,8 @@ impl Namer { pub fn reset( &mut self, module: &crate::Module, - reserved_keywords: &'static HashSet<&'static str>, - reserved_keywords_case_insensitive: &[&'static str], + reserved_keywords: &'static KeywordSet, + reserved_keywords_case_insensitive: &'static CaseInsensitiveKeywordSet, reserved_prefixes: &[&'static str], output: &mut FastHashMap, ) { @@ -238,16 +223,7 @@ impl Namer { self.unique.clear(); self.keywords = reserved_keywords; - - debug_assert!(reserved_keywords_case_insensitive - .iter() - .all(|s| s.is_ascii())); - self.keywords_case_insensitive.clear(); - self.keywords_case_insensitive.extend( - reserved_keywords_case_insensitive - .iter() - .map(|string| AsciiUniCase(*string)), - ); + self.keywords_case_insensitive = reserved_keywords_case_insensitive; // Choose fallback names for anonymous entry point return types. let mut entrypoint_type_fallbacks = FastHashMap::default(); @@ -384,33 +360,6 @@ impl Namer { } } -/// A string wrapper type with an ascii case insensitive Eq and Hash impl -struct AsciiUniCase + ?Sized>(S); - -impl> PartialEq for AsciiUniCase { - #[inline] - fn eq(&self, other: &Self) -> bool { - self.0.as_ref().eq_ignore_ascii_case(other.0.as_ref()) - } -} - -impl> Eq for AsciiUniCase {} - -impl> Hash for AsciiUniCase { - #[inline] - fn hash(&self, hasher: &mut H) { - for byte in self - .0 - .as_ref() - .as_bytes() - .iter() - .map(|b| b.to_ascii_lowercase()) - { - hasher.write_u8(byte); - } - } -} - #[test] fn test() { let mut namer = Namer::default(); diff --git a/naga/src/racy_lock.rs b/naga/src/racy_lock.rs index 116808055..fcb70b792 100644 --- a/naga/src/racy_lock.rs +++ b/naga/src/racy_lock.rs @@ -1,11 +1,3 @@ -#![cfg_attr( - not(any(glsl_out, hlsl_out, msl_out, feature = "wgsl-in", wgsl_out)), - expect( - dead_code, - reason = "RacyLock is only required for the above configurations" - ) -)] - use alloc::boxed::Box; use once_cell::race::OnceBox; @@ -25,17 +17,13 @@ impl RacyLock { init, } } - - /// Loads the internal value, initializing it if required. - pub fn get(&self) -> &T { - self.inner.get_or_init(|| Box::new((self.init)())) - } } impl core::ops::Deref for RacyLock { type Target = T; + /// Loads the internal value, initializing it if required. fn deref(&self) -> &Self::Target { - self.get() + self.inner.get_or_init(|| Box::new((self.init)())) } }