Fix decompose_float subnormals, float format grouping/empty type; add HashSecret::from_keys (#8232)

* common: fix decompose_float for subnormal inputs

Subnormals carry a biased exponent of 0 and no implicit leading mantissa
bit, so the bit-twiddling frexp misread them: hash(5e-324) returned
8404992 instead of CPython's 16777216. Scale subnormals into the normal
range by 2**54 (exact) before decomposing and fold the shift back into
the returned exponent. Adds a frexp-contract unit test over normals,
subnormals, and boundary values.

Assisted-by: Claude

* common: add HashSecret::from_keys

HashSecret's fields are private and the only constructor derives k0/k1
from a u32 seed, so an embedder cannot reproduce a fixed keying (e.g. a
deterministic PYTHONHASHSEED=0-style run). Add a const constructor
taking explicit SipHash keys; the seeded path is unchanged.

Assisted-by: Claude

* common: fix float format grouping and empty presentation type

Thousands grouping split the magnitude only on '.', so exponent and
percent tails received separators (format(1e20, ',e') gave
'1e+,20'-class corruption). Group only the leading integer digits for
interval-3 output; hex/octal/binary keep whole-magnitude grouping.

Empty presentation type: with a precision, clamp it to at least 1 and
restore a trailing '.0' on integer-looking results (Py_DTSF_ADD_DOT_0);
without one, alternate form now inserts the forced decimal point before
an exponent instead of appending it.

Verified byte-identical to CPython 3.14 over a hash/format differential
battery; test_format, test_float, test_fstring, test_hash pass.

Assisted-by: Claude

* common: satisfy rustfmt and clippy in new hash/float tests

- float_ops: use core::f64::consts::PI in the frexp-contract battery
  (clippy::approx_constant)
- hash: rewrite the keyed_hash test via BuildHasher::hash_one
  (clippy::manual_hash_one)
- rustfmt the new float_ops/hash/format test modules

Assisted-by: Claude

* common: add DTSF to format.rs spell-checker ignores

Assisted-by: Claude

crates/common/src/float_ops.rs

@@ -5,13 +5,21 @@ use num_traits::{Signed, ToPrimitive};
 #[must_use]
 pub const fn decompose_float(value: f64) -> (f64, i32) {
     if value == 0.0 {
-        (0.0, 0)
-    } else {
-        let bits = value.to_bits();
-        let exponent: i32 = ((bits >> 52) & 0x7ff) as i32 - 1022;
-        let mantissa_bits = bits & (0x000f_ffff_ffff_ffff) | (1022 << 52);
-        (f64::from_bits(mantissa_bits), exponent)
+        return (0.0, 0);
     }
+    let bits = value.to_bits();
+    // Subnormals carry a biased exponent of 0 and no implicit leading mantissa
+    // bit, so the normal decomposition below would misread them. Scale them up
+    // into the normal range first (exact, since it is a power-of-two shift) and
+    // fold the scale back into the returned exponent.
+    let (bits, exponent_adjust) = if (bits >> 52) & 0x7ff == 0 {
+        ((value * (1u64 << 54) as f64).to_bits(), -54)
+    } else {
+        (bits, 0)
+    };
+    let exponent: i32 = ((bits >> 52) & 0x7ff) as i32 - 1022 + exponent_adjust;
+    let mantissa_bits = bits & (0x000f_ffff_ffff_ffff) | (1022 << 52);
+    (f64::from_bits(mantissa_bits), exponent)
 }
 
 /// Equate an integer to a float.
@@ -270,3 +278,119 @@ pub fn round_float_digits(x: f64, ndigits: i32) -> Option<f64> {
     }
     Some(result)
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use crate::hash::hash_float;
+
+    /// Exact `2**e` for `e` in `[-1074, 1023]`, built from bits so extreme
+    /// exponents don't overflow through an intermediate `2**|e|`.
+    fn pow2(e: i32) -> f64 {
+        if e >= -1022 {
+            f64::from_bits(((e + 1023) as u64) << 52)
+        } else {
+            f64::from_bits(1u64 << (e + 1074))
+        }
+    }
+
+    /// `decompose_float` is a frexp returning the *magnitude* mantissa: for a
+    /// nonzero `value`, `m` lies in `[0.5, 1)` and `m * 2**e == value.abs()`,
+    /// including for subnormals which have no implicit leading mantissa bit.
+    /// (Its sole caller reintroduces the sign via `value.signum()`.)
+    #[test]
+    fn decompose_float_frexp_contract() {
+        let mut values = alloc::vec![
+            0.0,
+            f64::from_bits(1), // smallest subnormal
+            f64::from_bits(2),
+            f64::from_bits(0x000f_ffff_ffff_ffff), // largest subnormal
+            f64::MIN_POSITIVE,                     // DBL_MIN, smallest normal
+            f64::from_bits(f64::MIN_POSITIVE.to_bits() - 1), // predecessor
+            1.0,
+            1.5,
+            0.1,
+            core::f64::consts::PI,
+        ];
+        for e in -1074..=1023 {
+            values.push(pow2(e));
+            values.push(-pow2(e));
+        }
+        for &v in &values {
+            let (m, e) = decompose_float(v);
+            if v == 0.0 {
+                assert_eq!((m, e), (0.0, 0));
+                continue;
+            }
+            assert!(
+                (0.5..1.0).contains(&m),
+                "mantissa {m} out of [0.5, 1) for value {v:e}"
+            );
+            // Reconstruct: m * 2**e must round-trip to the magnitude. Fold one
+            // power of two into the mantissa so `e` stays within `pow2`'s range
+            // (frexp yields e up to 1024 for 2**1023).
+            let reconstructed = (m * 2.0) * pow2(e - 1);
+            assert_eq!(
+                reconstructed.to_bits(),
+                v.abs().to_bits(),
+                "reconstruction failed for {v:e}: m={m}, e={e}"
+            );
+        }
+    }
+
+    /// Subnormal frexp regression: hash of the smallest positive subnormal.
+    #[test]
+    fn hash_float_smallest_subnormal() {
+        // hash(5e-324) == 16777216 (CPython 3.14 ground truth). The pre-fix
+        // bit-twiddling frexp returned 8404992 here.
+        assert_eq!(hash_float(f64::from_bits(1)), Some(16777216));
+    }
+
+    /// Differential float-hash table captured from CPython 3.14.5, spanning
+    /// subnormal boundaries, powers of two across the whole exponent range, and
+    /// a spread of normals.
+    #[test]
+    fn hash_float_matches_cpython() {
+        const HASH_CASES: &[(u64, i64)] = &[
+            (0x0000000000000001, 16777216),            // smallest subnormal 5e-324
+            (0x0000000000000002, 33554432),            // subnormal
+            (0x00000000deadbeef, 62678480394911744),   // subnormal midrange
+            (0x0008000000000000, 16384),               // subnormal high bit
+            (0x000fffffffffffff, 2305843009196949503), // largest subnormal
+            (0x0010000000000000, 32768),               // DBL_MIN smallest normal
+            (0x8000000000000001, -16777216),           // negative smallest subnormal
+            (0x0020000000000000, 65536),               // 2**-1021
+            (0x0170000000000000, 137438953472),        // 2**-1000
+            (0x39b0000000000000, 4194304),             // 2**-100
+            (0x3f50000000000000, 2251799813685248),    // 2**-10
+            (0x3fe0000000000000, 1152921504606846976), // 2**-1
+            (0x3ff0000000000000, 1),                   // 2**0
+            (0x4000000000000000, 2),                   // 2**1
+            (0x4090000000000000, 1024),                // 2**10
+            (0x4630000000000000, 549755813888),        // 2**100
+            (0x7e70000000000000, 16777216),            // 2**1000
+            (0x7fe0000000000000, 140737488355328),     // 2**1023
+            (0xffe0000000000000, -140737488355328),    // -2**1023
+            (0x3ff8000000000000, 1152921504606846977), // 1.5
+            (0x400921fb54442d18, 326490430436040707),  // 3.141592653589793
+            (0x7e37e43c8800759c, 1224995262755759164), // 1e+300
+            (0x01a56e1fc2f8f359, 482449582752280463),  // 1e-300
+            (0x40c81cd6c8b43958, 1563361560246628409), // 12345.678
+            (0x3fb999999999999a, 230584300921369408),  // 0.1
+            (0x4005666666666666, 1556444031219243010), // 2.675
+            (0x4132d68700000000, 1234567),             // 1234567.0
+            (0x44dfe154f457ea13, 1428027733287631914), // 6.022e+23
+            (0x3c07a42f549647fb, 851769299698974080),  // 1.602e-19
+            (0xbff0000000000000, -2),                  // -1.0
+            (0xbfb999999999999a, -230584300921369408), // -0.1
+        ];
+        for &(bits, expected) in HASH_CASES {
+            let v = f64::from_bits(bits);
+            assert_eq!(
+                hash_float(v),
+                Some(expected),
+                "hash mismatch for {v:e} (bits {bits:#018x})"
+            );
+        }
+    }
+}

crates/common/src/format.rs

@@ -1,4 +1,4 @@
-// spell-checker:ignore ddfe
+// spell-checker:ignore ddfe DTSF
 use core::ops::Deref;
 use core::{cmp, str::FromStr};
 use itertools::{Itertools, PeekingNext};
@@ -380,15 +380,31 @@ impl FormatSpec {
         sep: char,
         disp_digit_cnt: i32,
     ) -> String {
-        // Don't add separators to the floating decimal point of numbers
-        let mut parts = magnitude_str.splitn(2, '.');
-        let magnitude_int_str = parts.next().unwrap().to_string();
+        // Group only the leading integer digits; the trailing remainder must
+        // never receive separators. For decimal and float output (interval 3)
+        // that remainder is the decimal point and fraction, an exponent
+        // (`e+NN`), or a trailing percent sign. Hex/octal/binary output
+        // (interval 4) has no such tail and its `a`-`f`/`e`/`E` are digits, so
+        // the whole magnitude is groupable.
+        let int_len = if inter == 4 {
+            magnitude_str.len()
+        } else {
+            magnitude_str
+                .bytes()
+                .position(|b| !b.is_ascii_digit())
+                .unwrap_or(magnitude_str.len())
+        };
+        // No leading integer digits (e.g. "inf"/"nan") means nothing to group;
+        // leave any width padding to the fill/align step.
+        if int_len == 0 {
+            return magnitude_str;
+        }
+        let magnitude_int_str = magnitude_str[..int_len].to_string();
+        let remainder = &magnitude_str[int_len..];
         let dec_digit_cnt = magnitude_str.len() as i32 - magnitude_int_str.len() as i32;
         let int_digit_cnt = disp_digit_cnt - dec_digit_cnt;
         let mut result = Self::separate_integer(magnitude_int_str, inter, sep, int_digit_cnt);
-        if let Some(part) = parts.next() {
-            result.push_str(&format!(".{part}"))
-        }
+        result.push_str(remainder);
         result
     }
 
@@ -786,14 +802,40 @@ impl FormatSpec {
                 magnitude if magnitude.is_nan() => Ok("nan".to_owned()),
                 magnitude if magnitude.is_infinite() => Ok("inf".to_owned()),
                 _ => match self.precision {
-                    Some(precision) => Ok(float::format_general(
-                        precision,
-                        magnitude,
-                        Case::Lower,
-                        self.alternate_form,
-                        true,
-                    )),
-                    None => Ok(float::to_string(magnitude)),
+                    Some(precision) => {
+                        // Empty presentation type with a precision behaves like
+                        // `g` but repr-like: precision is clamped to at least 1,
+                        // and an integer-looking result keeps a trailing `.0`
+                        // (Py_DTSF_ADD_DOT_0).
+                        let precision = if precision == 0 { 1 } else { precision };
+                        let s = float::format_general(
+                            precision,
+                            magnitude,
+                            Case::Lower,
+                            self.alternate_form,
+                            true,
+                        );
+                        Ok(if s.bytes().any(|b| matches!(b, b'.' | b'e' | b'E')) {
+                            s
+                        } else {
+                            format!("{s}.0")
+                        })
+                    }
+                    None => {
+                        let s = float::to_string(magnitude);
+                        // Alternate form forces a decimal point into the
+                        // repr-like output. Only exponent-form values lack one
+                        // (`1e+16` -> `1.e+16`); fixed-form repr already carries
+                        // a `.`.
+                        Ok(if self.alternate_form && !s.contains('.') {
+                            match s.find(['e', 'E']) {
+                                Some(pos) => format!("{}.{}", &s[..pos], &s[pos..]),
+                                None => format!("{s}."),
+                            }
+                        } else {
+                            s
+                        })
+                    }
                 },
             },
         };
@@ -857,10 +899,11 @@ impl FormatSpec {
                 Err(FormatSpecError::UnknownFormatCode('N', "int"))
             }
             Some(FormatType::String) => Err(FormatSpecError::UnknownFormatCode('s', "int")),
-            Some(FormatType::Character) => match (self.sign, self.alternate_form) {
-                (Some(_), _) => Err(FormatSpecError::NotAllowed("Sign")),
-                (_, true) => Err(FormatSpecError::NotAllowed("Alternate form (#)")),
-                (_, _) => match num.to_u32() {
+            Some(FormatType::Character) => match (self.precision, self.sign, self.alternate_form) {
+                (Some(_), _, _) => Err(FormatSpecError::PrecisionNotAllowed),
+                (_, Some(_), _) => Err(FormatSpecError::NotAllowed("Sign")),
+                (_, _, true) => Err(FormatSpecError::NotAllowed("Alternate form (#)")),
+                (_, _, _) => match num.to_u32() {
                     Some(n) if n <= 0x10ffff => Ok(core::char::from_u32(n).unwrap().to_string()),
                     Some(_) | None => Err(FormatSpecError::CodeNotInRange),
                 },
@@ -1664,6 +1707,105 @@ mod tests {
         assert_eq!(result, "000001,234");
     }
 
+    fn fmt_float(spec: &str, value: f64) -> String {
+        FormatSpec::parse(spec)
+            .unwrap()
+            .format_float(value)
+            .unwrap()
+    }
+
+    #[test]
+    fn format_float_grouping_never_touches_exponent() {
+        // Grouping must group only the mantissa's integer digits, never the
+        // exponent digits (was "1e,+20") or a trailing percent sign.
+        assert_eq!(fmt_float(",g", 1e20), "1e+20");
+        assert_eq!(fmt_float("_g", 1e-10), "1e-10");
+        assert_eq!(fmt_float(",e", 1e20), "1.000000e+20");
+        assert_eq!(fmt_float(",", 1e16), "1e+16");
+        assert_eq!(fmt_float(",.0%", 1.0), "100%");
+        assert_eq!(fmt_float(",.2%", 12345.0), "1,234,500.00%");
+        // Fixed-form grouping still groups the integer part.
+        assert_eq!(fmt_float(",", 1234567.0), "1,234,567.0");
+    }
+
+    #[test]
+    fn format_float_grouping_inf_nan() {
+        // No integer digits to group; width padding is left to fill/align, so
+        // separators never land inside "inf"/"nan".
+        assert_eq!(fmt_float(",", f64::INFINITY), "inf");
+        assert_eq!(fmt_float("06,", f64::INFINITY), "000inf");
+        assert_eq!(fmt_float("06,", f64::NAN), "000nan");
+        assert_eq!(fmt_float("06,%", f64::INFINITY), "00inf%");
+    }
+
+    #[test]
+    fn format_float_empty_type_with_precision() {
+        // Empty presentation type with a precision is repr-like: precision is
+        // clamped to at least 1 and integer-looking output keeps a `.0`.
+        assert_eq!(fmt_float(".2", 1.0), "1.0");
+        assert_eq!(fmt_float(".6", 100.0), "100.0");
+        assert_eq!(fmt_float(".17", 1234567.0), "1234567.0");
+        assert_eq!(fmt_float(".0", 0.5), "0.5");
+        assert_eq!(fmt_float(".0", 0.0001), "0.0001");
+        assert_eq!(fmt_float(".2", 0.0), "0.0");
+        assert_eq!(fmt_float(".0", 0.0), "0e+00");
+        assert_eq!(fmt_float(".2", 100.0), "1e+02");
+    }
+
+    #[test]
+    fn format_float_alternate_form_forces_point() {
+        // Alternate form injects a decimal point into exponent-form repr.
+        assert_eq!(fmt_float("#", 1e16), "1.e+16");
+        assert_eq!(fmt_float("#", 1e-5), "1.e-05");
+        // Fixed-form repr already has a point, so it is unchanged.
+        assert_eq!(fmt_float("#", 100.0), "100.0");
+        assert_eq!(fmt_float("#", 1.5), "1.5");
+    }
+
+    #[test]
+    fn format_int_hex_grouping_preserved() {
+        // Underscore grouping of hex/octal groups every 4 digits, including the
+        // `a`-`f` letters.
+        assert_eq!(
+            FormatSpec::parse("_x")
+                .unwrap()
+                .format_int(&BigInt::from(1000000))
+                .unwrap(),
+            "f_4240"
+        );
+        assert_eq!(
+            FormatSpec::parse("_X")
+                .unwrap()
+                .format_int(&BigInt::from(0xABCDEFu32))
+                .unwrap(),
+            "AB_CDEF"
+        );
+    }
+
+    #[test]
+    fn format_int_character_rejects_precision() {
+        // 'c' rejects precision, and precision is checked before sign/alt form.
+        assert_eq!(
+            FormatSpec::parse(".2c")
+                .unwrap()
+                .format_int(&BigInt::from(65)),
+            Err(FormatSpecError::PrecisionNotAllowed)
+        );
+        assert_eq!(
+            FormatSpec::parse("+.2c")
+                .unwrap()
+                .format_int(&BigInt::from(65)),
+            Err(FormatSpecError::PrecisionNotAllowed)
+        );
+        // Without precision, 'c' still renders the code point.
+        assert_eq!(
+            FormatSpec::parse("c")
+                .unwrap()
+                .format_int(&BigInt::from(65)),
+            Ok("A".to_owned())
+        );
+    }
+
     #[test]
     fn format_parse() {
         let expected = Ok(FormatString {

crates/common/src/hash.rs

@@ -50,6 +50,14 @@ impl HashSecret {
         Self { k0, k1 }
     }
 
+    /// Build a secret from explicit SipHash keys, bypassing seed derivation.
+    /// Lets an embedder reproduce a fixed keying (e.g. a deterministic run) that
+    /// [`new`](Self::new) cannot express through its `u32` seed.
+    #[must_use]
+    pub const fn from_keys(k0: u64, k1: u64) -> Self {
+        Self { k0, k1 }
+    }
+
     pub fn hash_value<T: Hash + ?Sized>(&self, data: &T) -> PyHash {
         fix_sentinel(mod_int(self.hash_one(data) as _))
     }
@@ -296,3 +304,34 @@ impl FrozenSetHash {
         hash as PyHash
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn from_keys_is_stable_and_seed_independent() {
+        const K0: u64 = 0x0706_0504_0302_0100;
+        const K1: u64 = 0x0f0e_0d0c_0b0a_0908;
+        const LOCKED_DIGEST: PyHash = -1862661396243998188;
+
+        // Two secrets built from the same explicit keys hash identically, and
+        // the digest does not depend on the seed-derivation path.
+        let a = HashSecret::from_keys(K0, K1);
+        let b = HashSecret::from_keys(K0, K1);
+        assert_eq!(a.hash_str("hello"), b.hash_str("hello"));
+        assert_eq!(
+            a.hash_bytes(b"a fixed message"),
+            b.hash_bytes(b"a fixed message")
+        );
+
+        // Explicit keys drive the SipHasher-2-4 directly. `keyed_hash` pins
+        // k1 = 0, so a secret built with the same k0 and k1 = 0 must reproduce
+        // its raw digest.
+        let zero_k1 = HashSecret::from_keys(K0, 0);
+        assert_eq!(keyed_hash(K0, b"payload"), zero_k1.hash_one(b"payload"));
+
+        // Locked digest so an accidental keying change is caught.
+        assert_eq!(a.hash_str("determinism"), LOCKED_DIGEST);
+    }
+}