From 5e98d921683119780e2e79df7b8e114b0afff315 Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Tue, 25 Aug 2026 08:36:32 +0200 Subject: [PATCH] Fix division by zero with empty static set This happens for example with [`web_atoms::Prefix`](https://docs.rs/web_atoms/latest/web_atoms/type.Prefix.html): all statically-known strings are short enough to be represented inline, so phf is effectively unused. Signed-off-by: Simon Sapin --- integration-tests/build.rs | 7 +++++++ integration-tests/src/lib.rs | 29 +++++++++++++++++++++++++---- string-cache-codegen/Cargo.toml | 2 +- string-cache-codegen/lib.rs | 17 +++++++++++++---- 4 files changed, 46 insertions(+), 9 deletions(-) diff --git a/integration-tests/build.rs b/integration-tests/build.rs index 0acbb73..b9755fd 100644 --- a/integration-tests/build.rs +++ b/integration-tests/build.rs @@ -20,5 +20,12 @@ fn main() { "❤💯❤💯", ]) .write_to_file(&Path::new(&env::var("OUT_DIR").unwrap()).join("test_atom.rs")) + .unwrap(); + + // All statically-known atoms are short enough to be represented inline, + // so the static set is empty. Ensure phf doesn’t divide by zero. + string_cache_codegen::AtomType::new("TestAtom2", "test_atom2!") + .atoms(&["a"]) + .write_to_file(&Path::new(&env::var("OUT_DIR").unwrap()).join("test_atom2.rs")) .unwrap() } diff --git a/integration-tests/src/lib.rs b/integration-tests/src/lib.rs index ff17e30..cfb1bfa 100644 --- a/integration-tests/src/lib.rs +++ b/integration-tests/src/lib.rs @@ -19,6 +19,7 @@ use std::thread; use string_cache::StaticAtomSet; include!(concat!(env!("OUT_DIR"), "/test_atom.rs")); +include!(concat!(env!("OUT_DIR"), "/test_atom2.rs")); pub type Atom = TestAtom; #[test] @@ -358,14 +359,18 @@ fn test_eq_ignore_ascii_case() { assert!(Atom::from("").eq_ignore_ascii_case(&Atom::from(""))); assert!(Atom::from("aZ9").eq_ignore_ascii_case(&Atom::from("aZ9"))); assert!(Atom::from("aZ9").eq_ignore_ascii_case(&Atom::from("Az9"))); - assert!(Atom::from("The Quick Brown Fox!") - .eq_ignore_ascii_case(&Atom::from("THE quick BROWN fox!"))); + assert!( + Atom::from("The Quick Brown Fox!") + .eq_ignore_ascii_case(&Atom::from("THE quick BROWN fox!")) + ); assert!(Atom::from("Je vais à Paris").eq_ignore_ascii_case(&Atom::from("je VAIS à PARIS"))); assert!(!Atom::from("").eq_ignore_ascii_case(&Atom::from("az9"))); assert!(!Atom::from("aZ9").eq_ignore_ascii_case(&Atom::from(""))); assert!(!Atom::from("aZ9").eq_ignore_ascii_case(&Atom::from("9Za"))); - assert!(!Atom::from("The Quick Brown Fox!") - .eq_ignore_ascii_case(&Atom::from("THE quick BROWN fox!!"))); + assert!( + !Atom::from("The Quick Brown Fox!") + .eq_ignore_ascii_case(&Atom::from("THE quick BROWN fox!!")) + ); assert!(!Atom::from("Je vais à Paris").eq_ignore_ascii_case(&Atom::from("JE vais À paris"))); } @@ -382,6 +387,22 @@ fn test_try_static() { assert!(Atom::try_static("not in the static table").is_none()); } +#[test] +fn test_with_empty_static_set() { + assert_eq!(TestAtom2::from("a").as_str(), "a"); + assert_eq!( + TestAtom2::from("longer-than-inline").as_str(), + "longer-than-inline" + ); + + // the dummy string used in `string_cache_codegen::AtomType::to_tokens` + // to make the static set non-empty + let dummy = " ".repeat(8); + let atom = TestAtom2::from(dummy); + assert!(atom.is_static()); + assert_eq!(atom, test_atom2!(" ")); +} + #[cfg(test)] #[path = "common-usage.rs"] mod common_usage; diff --git a/string-cache-codegen/Cargo.toml b/string-cache-codegen/Cargo.toml index f12daab..11a5b52 100644 --- a/string-cache-codegen/Cargo.toml +++ b/string-cache-codegen/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "string_cache_codegen" -version = "0.11.0" # Also update ../README.md when making a semver-breaking change +version = "0.11.1" # Also update ../README.md when making a semver-breaking change authors = [ "The Servo Project Developers" ] description = "A codegen library for string-cache, developed as part of the Servo project." license = "MIT OR Apache-2.0" diff --git a/string-cache-codegen/lib.rs b/string-cache-codegen/lib.rs index 861dd76..6f01343 100644 --- a/string-cache-codegen/lib.rs +++ b/string-cache-codegen/lib.rs @@ -199,16 +199,25 @@ impl AtomType { #[expect(clippy::wrong_self_convention)] // Doesn’t matter on a private method fn to_tokens(&mut self) -> proc_macro2::TokenStream { - // Make `atom!("")` always work, and ensure the set is non-empty - // to avoid divisions by zero in rust-phf. + // Make `atom!("")` always work self.atoms.insert(String::new()); + const MAX_INLINE_LEN: usize = 7; + // Strings over 7 bytes added to static set, otherwise stored inline. - let (static_strs, inline_strs): (Vec<_>, Vec<_>) = self + let (mut static_strs, inline_strs): (Vec<_>, Vec<_>) = self .atoms .iter() .map(String::as_str) - .partition(|s| s.len() > 7); + .partition(|s| s.len() > MAX_INLINE_LEN); + + let dummy; + if static_strs.is_empty() { + // Some arbitrary string that `Atom::from(&str)` won’t represent inline + dummy = " ".repeat(MAX_INLINE_LEN + 1); + // Make the static set non-empty to avoid divisions by zero in rust-phf + static_strs.push(&dummy); + } // Static strings let hash_state = phf_generator::generate_hash(&static_strs);