From 8cd532db71d0674ca4f78a40515ae3f5f1fb1cd6 Mon Sep 17 00:00:00 2001
From: Robert Bastian <4706271+robertbastian@users.noreply.github.com>
Date: Thu, 1 Feb 2024 18:38:05 +0100
Subject: [PATCH 1/8] hb

---
 components/normalizer/src/properties.rs |   2 +-
 components/properties/src/props.rs      |   6 +-
 experimental/harfbuzz/Cargo.toml        |   1 +
 experimental/harfbuzz/src/lib.rs        | 307 +++++++++++++++++-------
 4 files changed, 226 insertions(+), 90 deletions(-)

diff --git a/components/normalizer/src/properties.rs b/components/normalizer/src/properties.rs
index 631cb281fda..9794d5209cd 100644
--- a/components/normalizer/src/properties.rs
+++ b/components/normalizer/src/properties.rs
@@ -90,7 +90,7 @@ impl CanonicalComposition {
     ///
     /// [📚 Help choosing a constructor](icu_provider::constructors)
     #[cfg(feature = "compiled_data")]
-    pub fn new() -> Self {
+    pub const fn new() -> Self {
         Self {
             canonical_compositions: DataPayload::from_static_ref(
                 crate::provider::Baked::SINGLETON_NORMALIZER_COMP_V1,
diff --git a/components/properties/src/props.rs b/components/properties/src/props.rs
index fbae09531bb..1ac8378b2b8 100644
--- a/components/properties/src/props.rs
+++ b/components/properties/src/props.rs
@@ -585,7 +585,7 @@ macro_rules! impl_value_getter {
         impl $ty {
             $(#[$attr_n2e])*
             #[cfg(feature = "compiled_data")]
-            $vis_n2e fn $cname_n2e() -> PropertyValueNameToEnumMapperBorrowed<'static, $ty> {
+            $vis_n2e const fn $cname_n2e() -> PropertyValueNameToEnumMapperBorrowed<'static, $ty> {
                 PropertyValueNameToEnumMapperBorrowed {
                     map: crate::provider::Baked::$singleton_n2e,
                     markers: PhantomData,
@@ -604,7 +604,7 @@ macro_rules! impl_value_getter {
             $(
                 $(#[$attr_e2sn])*
                 #[cfg(feature = "compiled_data")]
-                $vis_e2sn fn $cname_e2sn() -> $mapper_e2snb<'static, $ty> {
+                $vis_e2sn const fn $cname_e2sn() -> $mapper_e2snb<'static, $ty> {
                     $mapper_e2snb {
                         map: crate::provider::Baked::$singleton_e2sn,
                         markers: PhantomData,
@@ -622,7 +622,7 @@ macro_rules! impl_value_getter {
 
                 $(#[$attr_e2ln])*
                 #[cfg(feature = "compiled_data")]
-                $vis_e2ln fn $cname_e2ln() -> $mapper_e2lnb<'static, $ty> {
+                $vis_e2ln const fn $cname_e2ln() -> $mapper_e2lnb<'static, $ty> {
                     $mapper_e2lnb {
                         map: crate::provider::Baked::$singleton_e2ln,
                         markers: PhantomData,
diff --git a/experimental/harfbuzz/Cargo.toml b/experimental/harfbuzz/Cargo.toml
index ba3b4f09f9c..f05f62b0a4a 100644
--- a/experimental/harfbuzz/Cargo.toml
+++ b/experimental/harfbuzz/Cargo.toml
@@ -44,5 +44,6 @@ harfbuzz = { version = "0.4.0" }
 [features]
 default = []
 compiled_data = ["icu_normalizer/compiled_data", "icu_properties/compiled_data"]
+alloc = []
 std = []
 serde = ["icu_properties/serde", "icu_normalizer/serde", "icu_provider/serde"]
diff --git a/experimental/harfbuzz/src/lib.rs b/experimental/harfbuzz/src/lib.rs
index 9a8054dfc59..b46e0e056ba 100644
--- a/experimental/harfbuzz/src/lib.rs
+++ b/experimental/harfbuzz/src/lib.rs
@@ -41,31 +41,43 @@
 //! assert_eq!(b.get_script(), sys::HB_SCRIPT_ARABIC);
 //! ```
 
+#[cfg(feature = "alloc")]
 extern crate alloc;
 mod error;
 
 use crate::error::HarfBuzzError;
-use alloc::boxed::Box;
 use core::ffi::{c_char, c_void};
 use harfbuzz_sys::*;
 use icu_normalizer::properties::CanonicalCombiningClassMap;
 use icu_normalizer::properties::CanonicalComposition;
 use icu_normalizer::properties::CanonicalDecomposition;
 use icu_normalizer::properties::Decomposed;
-use icu_normalizer::provider::CanonicalCompositionsV1Marker;
-use icu_normalizer::provider::CanonicalDecompositionDataV1Marker;
-use icu_normalizer::provider::CanonicalDecompositionTablesV1Marker;
-use icu_normalizer::provider::NonRecursiveDecompositionSupplementV1Marker;
+#[cfg(feature = "alloc")]
+use icu_normalizer::provider::{
+    CanonicalCompositionsV1Marker, CanonicalDecompositionDataV1Marker,
+    CanonicalDecompositionTablesV1Marker, NonRecursiveDecompositionSupplementV1Marker,
+};
+use icu_properties::bidi_data;
+#[cfg(feature = "alloc")]
 use icu_properties::bidi_data::BidiAuxiliaryProperties;
+use icu_properties::bidi_data::BidiAuxiliaryPropertiesBorrowed;
+use icu_properties::maps;
+#[cfg(feature = "alloc")]
 use icu_properties::maps::CodePointMapData;
+use icu_properties::maps::CodePointMapDataBorrowed;
+#[cfg(feature = "alloc")]
 use icu_properties::names::PropertyEnumToValueNameLinearTiny4Mapper;
+use icu_properties::names::PropertyEnumToValueNameLinearTiny4MapperBorrowed;
+#[cfg(feature = "alloc")]
 use icu_properties::provider::bidi_data::BidiAuxiliaryPropertiesV1Marker;
+#[cfg(feature = "alloc")]
 use icu_properties::provider::{
     GeneralCategoryV1Marker, ScriptV1Marker, ScriptValueToShortNameV1Marker,
 };
 use icu_properties::{GeneralCategory, Script};
+#[cfg(feature = "alloc")]
 use icu_provider::prelude::*;
-use tinystr::{tinystr, TinyStr4};
+use tinystr::tinystr;
 
 /// The total number of General Category values is fixed per
 /// https://www.unicode.org/policies/stability_policy.html :
@@ -116,35 +128,47 @@ static ICU4X_GENERAL_CATEGORY_TO_HARFBUZZ: [u8; 30] = [
 unsafe extern "C" fn icu4x_hb_unicode_combining_class(
     _ufuncs: *mut hb_unicode_funcs_t,
     unicode: hb_codepoint_t,
-    user_data: *mut c_void,
+    #[cfg_attr(not(feature = "alloc"), allow(unused_variables))] user_data: *mut c_void,
 ) -> hb_unicode_combining_class_t {
-    (*(user_data as *mut CanonicalCombiningClassMap))
-        .get32(unicode)
-        .0 as hb_unicode_combining_class_t
-}
+    #[cfg(feature = "compiled_data")]
+    const CCC_MAP: Option<CanonicalCombiningClassMap> = Some(CanonicalCombiningClassMap::new());
+    #[cfg(not(feature = "compiled_data"))]
+    const CCC_MAP: Option<CanonicalCombiningClassMap> = None;
+
+    #[cfg(feature = "alloc")]
+    let ccc_map = (user_data as *mut CanonicalCombiningClassMap).as_ref();
+    #[cfg(not(feature = "alloc"))]
+    let ccc_map = None;
 
-unsafe extern "C" fn icu4x_hb_unicode_combining_class_destroy(user_data: *mut c_void) {
-    let _ = Box::from_raw(user_data as *mut CanonicalCombiningClassMap);
+    ccc_map.or(CCC_MAP.as_ref()).unwrap_unchecked().get32(unicode).0 as hb_unicode_combining_class_t
 }
 
 #[allow(clippy::indexing_slicing)]
 unsafe extern "C" fn icu4x_hb_unicode_general_category(
     _ufuncs: *mut hb_unicode_funcs_t,
     unicode: hb_codepoint_t,
-    user_data: *mut c_void,
+    #[cfg_attr(not(feature = "alloc"), allow(unused_variables))] user_data: *mut c_void,
 ) -> hb_unicode_general_category_t {
+    #[cfg(feature = "compiled_data")]
+    const GC_MAP: Option<CodePointMapDataBorrowed<'static, GeneralCategory>> =
+        Some(maps::general_category());
+    #[cfg(not(feature = "compiled_data"))]
+    const GC_MAP: Option<CodePointMapDataBorrowed<'static, GeneralCategory>> = None;
+
+    #[cfg(feature = "alloc")]
+    let gc_map = (user_data as *mut CodePointMapData<GeneralCategory>)
+        .as_ref()
+        .map(CodePointMapData::as_borrowed);
+    #[cfg(not(feature = "alloc"))]
+    let gc_map = None;
+
     // Indexing is OK, because `GeneralCategory` data is validated upon
     // deserialization so that there can be no out-of-range `GeneralCategory`
     // values (which would be UB to materialize). `GeneralCategory` is a
     // stable exhaustive enum, and the length of `ICU4X_GENERAL_CATEGORY_TO_HARFBUZZ`
     // matches the number of enum items, so the index will always be in range here.
-    ICU4X_GENERAL_CATEGORY_TO_HARFBUZZ[(*(user_data as *mut CodePointMapData<GeneralCategory>))
-        .as_borrowed()
-        .get32(unicode) as usize] as hb_unicode_general_category_t
-}
-
-unsafe extern "C" fn icu4x_hb_unicode_general_category_destroy(user_data: *mut c_void) {
-    let _ = Box::from_raw(user_data as *mut CodePointMapData<GeneralCategory>);
+    ICU4X_GENERAL_CATEGORY_TO_HARFBUZZ[gc_map.or(GC_MAP).unwrap_unchecked().get32(unicode) as usize]
+        as hb_unicode_general_category_t
 }
 
 /// Returns the Bidi_Mirroring_Glyph, but adjusting the return value
@@ -158,20 +182,30 @@ unsafe extern "C" fn icu4x_hb_unicode_general_category_destroy(user_data: *mut c
 unsafe extern "C" fn icu4x_hb_unicode_mirroring(
     _ufuncs: *mut hb_unicode_funcs_t,
     unicode: hb_codepoint_t,
-    user_data: *mut c_void,
+    #[cfg_attr(not(feature = "alloc"), allow(unused_variables))] user_data: *mut c_void,
 ) -> hb_codepoint_t {
-    (*(user_data as *mut BidiAuxiliaryProperties))
-        .as_borrowed()
+    #[cfg(feature = "compiled_data")]
+    const BIDI: Option<BidiAuxiliaryPropertiesBorrowed<'static>> =
+        Some(bidi_data::bidi_auxiliary_properties());
+    #[cfg(not(feature = "compiled_data"))]
+    const BIDI: Option<BidiAuxiliaryPropertiesBorrowed<'static>> = None;
+
+    #[cfg(feature = "alloc")]
+    let bidi = (user_data as *mut BidiAuxiliaryProperties)
+        .as_ref()
+        .map(BidiAuxiliaryProperties::as_borrowed);
+    #[cfg(not(feature = "alloc"))]
+    let bidi = None;
+
+    bidi.or(BIDI)
+        .unwrap_unchecked()
         .get32_mirroring_props(unicode)
         .mirroring_glyph
         .map(u32::from)
         .unwrap_or(unicode) as hb_codepoint_t
 }
 
-unsafe extern "C" fn icu4x_hb_unicode_mirroring_destroy(user_data: *mut c_void) {
-    let _ = Box::from_raw(user_data as *mut BidiAuxiliaryProperties);
-}
-
+#[cfg(feature = "alloc")]
 struct ScriptDataForHarfBuzz {
     script_map: CodePointMapData<Script>,
     enum_to_name_mapper: PropertyEnumToValueNameLinearTiny4Mapper<Script>,
@@ -180,31 +214,62 @@ struct ScriptDataForHarfBuzz {
 unsafe extern "C" fn icu4x_hb_unicode_script(
     _ufuncs: *mut hb_unicode_funcs_t,
     unicode: hb_codepoint_t,
-    user_data: *mut c_void,
+    #[cfg_attr(not(feature = "alloc"), allow(unused_variables))] user_data: *mut c_void,
 ) -> hb_script_t {
-    let script_data: &ScriptDataForHarfBuzz = &*(user_data as *mut ScriptDataForHarfBuzz);
-    let script: Script = script_data.script_map.as_borrowed().get32(unicode);
-    let enum_to_name_mapper = script_data.enum_to_name_mapper.as_borrowed();
-    let name: TinyStr4 = enum_to_name_mapper
+    #[cfg(feature = "compiled_data")]
+    const SCRIPT_MAP: Option<CodePointMapDataBorrowed<'static, Script>> = Some(maps::script());
+    #[cfg(feature = "compiled_data")]
+    const ENUM_TO_NAME_MAPPER: Option<PropertyEnumToValueNameLinearTiny4MapperBorrowed<Script>> =
+        Some(Script::enum_to_short_name_mapper());
+    #[cfg(not(feature = "compiled_data"))]
+    const SCRIPT_MAP: Option<CodePointMapDataBorrowed<'static, Script>> = None;
+    #[cfg(not(feature = "compiled_data"))]
+    const ENUM_TO_NAME_MAPPER: Option<PropertyEnumToValueNameLinearTiny4MapperBorrowed<Script>> =
+        None;
+
+    #[cfg(feature = "alloc")]
+    let script_map = (user_data as *mut ScriptDataForHarfBuzz)
+        .as_ref()
+        .map(|d| d.script_map.as_borrowed());
+    #[cfg(feature = "alloc")]
+    let enum_to_name_mapper = (user_data as *mut ScriptDataForHarfBuzz)
+        .as_ref()
+        .map(|d| d.enum_to_name_mapper.as_borrowed());
+    #[cfg(not(feature = "alloc"))]
+    let script_map = None;
+    #[cfg(not(feature = "alloc"))]
+    let enum_to_name_mapper = None;
+
+    let script = script_map.or(SCRIPT_MAP).unwrap_unchecked().get32(unicode);
+    let name = enum_to_name_mapper
+        .or(ENUM_TO_NAME_MAPPER)
+        .unwrap_unchecked()
         .get(script)
         .unwrap_or(tinystr!(4, "Zzzz"));
+
     hb_script_from_string(
         name.as_ptr() as *const c_char,
         name.len().try_into().unwrap_or(0),
     )
 }
 
-unsafe extern "C" fn icu4x_hb_unicode_script_destroy(user_data: *mut c_void) {
-    let _ = Box::from_raw(user_data as *mut ScriptDataForHarfBuzz);
-}
-
 unsafe extern "C" fn icu4x_hb_unicode_compose(
     _ufuncs: *mut hb_unicode_funcs_t,
     a: hb_codepoint_t,
     b: hb_codepoint_t,
     ab: *mut hb_codepoint_t,
-    user_data: *mut c_void,
+    #[cfg_attr(not(feature = "alloc"), allow(unused_variables))] user_data: *mut c_void,
 ) -> hb_bool_t {
+    #[cfg(feature = "compiled_data")]
+    const CC: Option<CanonicalComposition> = Some(CanonicalComposition::new());
+    #[cfg(not(feature = "compiled_data"))]
+    const CC: Option<CanonicalComposition> = None;
+
+    #[cfg(feature = "alloc")]
+    let cc = (user_data as *mut CanonicalComposition).as_ref();
+    #[cfg(not(feature = "alloc"))]
+    let cc = None;
+
     // It appears that HarfBuzz will pass valid scalar values
     // unless the application violated the contract of
     // `hb_buffer_add_codepoints` and passed in non-scalar values.
@@ -225,7 +290,7 @@ unsafe extern "C" fn icu4x_hb_unicode_compose(
         debug_assert!(false);
         return false as hb_bool_t;
     };
-    if let Some(c) = (*(user_data as *mut CanonicalComposition)).compose(first, second) {
+    if let Some(c) = cc.or(CC.as_ref()).unwrap_unchecked().compose(first, second) {
         core::ptr::write(ab, c as hb_codepoint_t);
         true as hb_bool_t
     } else {
@@ -233,17 +298,23 @@ unsafe extern "C" fn icu4x_hb_unicode_compose(
     }
 }
 
-unsafe extern "C" fn icu4x_hb_unicode_compose_destroy(user_data: *mut c_void) {
-    let _ = Box::from_raw(user_data as *mut CanonicalComposition);
-}
-
 unsafe extern "C" fn icu4x_hb_unicode_decompose(
     _ufuncs: *mut hb_unicode_funcs_t,
     ab: hb_codepoint_t,
     a: *mut hb_codepoint_t,
     b: *mut hb_codepoint_t,
-    user_data: *mut c_void,
+    #[cfg_attr(not(feature = "alloc"), allow(unused_variables))] user_data: *mut c_void,
 ) -> hb_bool_t {
+    #[cfg(feature = "compiled_data")]
+    const DC: Option<CanonicalDecomposition> = Some(CanonicalDecomposition::new());
+    #[cfg(not(feature = "compiled_data"))]
+    const DC: Option<CanonicalDecomposition> = None;
+
+    #[cfg(feature = "alloc")]
+    let dc = (user_data as *mut CanonicalDecomposition).as_ref();
+    #[cfg(not(feature = "alloc"))]
+    let dc = None;
+
     // It appears that HarfBuzz will pass valid scalar values
     // unless the application violated the contract of
     // `hb_buffer_add_codepoints` and passed in non-scalar values.
@@ -257,7 +328,7 @@ unsafe extern "C" fn icu4x_hb_unicode_decompose(
         debug_assert!(false);
         return false as hb_bool_t;
     };
-    match (*(user_data as *mut CanonicalDecomposition)).decompose(composed) {
+    match dc.or(DC.as_ref()).unwrap_unchecked().decompose(composed) {
         Decomposed::Default => false as hb_bool_t,
         Decomposed::Expansion(first, second) => {
             core::ptr::write(a, first as hb_codepoint_t);
@@ -272,10 +343,6 @@ unsafe extern "C" fn icu4x_hb_unicode_decompose(
     }
 }
 
-unsafe extern "C" fn icu4x_hb_unicode_decompose_destroy(user_data: *mut c_void) {
-    let _ = Box::from_raw(user_data as *mut CanonicalDecomposition);
-}
-
 /// RAII holder for `*mut hb_unicode_funcs_t`.
 // TODO(#2838): Document the conditions under which multithreaded lookups via the
 // `*mut hb_unicode_funcs_t` are OK. HarfBuzz itself takes care of atomic
@@ -334,18 +401,64 @@ impl Drop for UnicodeFuncs {
 /// [📚 Help choosing a constructor](icu_provider::constructors)
 #[cfg(feature = "compiled_data")]
 pub fn new_hb_unicode_funcs() -> Result<UnicodeFuncs, HarfBuzzError> {
-    create_ufuncs(
-        CanonicalCombiningClassMap::new(),
-        icu_properties::maps::general_category().static_to_owned(),
-        icu_properties::bidi_data::bidi_auxiliary_properties().static_to_owned(),
-        icu_properties::maps::script().static_to_owned(),
-        Script::enum_to_short_name_mapper().static_to_owned(),
-        CanonicalComposition::new(),
-        CanonicalDecomposition::new(),
-    )
+    unsafe {
+        let empty = hb_unicode_funcs_get_empty();
+        // The HarfBuzz refcounting convention is that "create"
+        // sets refcount to one, not zero.
+        // https://harfbuzz.github.io/object-model-lifecycle.html
+        let ufuncs = hb_unicode_funcs_create(empty);
+        if ufuncs == empty {
+            return Err(HarfBuzzError::Alloc);
+        }
+        // Below this point, the only return is upon success.
+        hb_unicode_funcs_set_combining_class_func(
+            ufuncs,
+            Some(icu4x_hb_unicode_combining_class),
+            core::ptr::null_mut() as *mut c_void,
+            None,
+        );
+        hb_unicode_funcs_set_general_category_func(
+            ufuncs,
+            Some(icu4x_hb_unicode_general_category),
+            core::ptr::null_mut() as *mut c_void,
+            None,
+        );
+        hb_unicode_funcs_set_mirroring_func(
+            ufuncs,
+            Some(icu4x_hb_unicode_mirroring),
+            core::ptr::null_mut() as *mut c_void,
+            None,
+        );
+        hb_unicode_funcs_set_script_func(
+            ufuncs,
+            Some(icu4x_hb_unicode_script),
+            core::ptr::null_mut() as *mut c_void,
+            None,
+        );
+        hb_unicode_funcs_set_compose_func(
+            ufuncs,
+            Some(icu4x_hb_unicode_compose),
+            core::ptr::null_mut() as *mut c_void,
+            None,
+        );
+        hb_unicode_funcs_set_decompose_func(
+            ufuncs,
+            Some(icu4x_hb_unicode_decompose),
+            core::ptr::null_mut() as *mut c_void,
+            None,
+        );
+
+        // Compatibility decomposition and East Asian Width lookups
+        // are deprecated, and there's no need to set up the callbacks
+        // for those.
+
+        hb_unicode_funcs_make_immutable(ufuncs);
+        Ok(UnicodeFuncs::from_raw(ufuncs))
+    }
 }
 
 #[doc = icu_provider::gen_any_buffer_unstable_docs!(UNSTABLE, new_hb_unicode_funcs)]
+#[cfg(feature = "alloc")]
 pub fn new_hb_unicode_funcs_unstable<D>(provider: &D) -> Result<UnicodeFuncs, HarfBuzzError>
 where
     D: DataProvider<BidiAuxiliaryPropertiesV1Marker>
@@ -358,26 +471,51 @@ where
         + DataProvider<ScriptV1Marker>
         + ?Sized,
 {
-    create_ufuncs(
-        CanonicalCombiningClassMap::try_new_unstable(provider)?,
-        icu_properties::maps::load_general_category(provider)?,
-        icu_properties::bidi_data::load_bidi_auxiliary_properties_unstable(provider)?,
-        icu_properties::maps::load_script(provider)?,
-        Script::get_enum_to_short_name_mapper(provider)?,
-        CanonicalComposition::try_new_unstable(provider)?,
-        CanonicalDecomposition::try_new_unstable(provider)?,
-    )
-}
+    let canonical_combining_class_map = CanonicalCombiningClassMap::try_new_unstable(provider)?;
+    let general_category_map = maps::load_general_category(provider)?;
+    let bidi_auxiliary_props_map = bidi_data::load_bidi_auxiliary_properties_unstable(provider)?;
+    let script_map = maps::load_script(provider)?;
+    let script_enum_to_short_name_lookup = Script::get_enum_to_short_name_mapper(provider)?;
+    let canonical_composition = CanonicalComposition::try_new_unstable(provider)?;
+    let canonical_decomposition = CanonicalDecomposition::try_new_unstable(provider)?;
+
+    #[cfg(not(feature = "std"))]
+    use alloc::boxed::Box;
+
+    let combining_class = Box::into_raw(Box::new(canonical_combining_class_map));
+    let general_category = Box::into_raw(Box::new(general_category_map));
+    let mirroring = Box::into_raw(Box::new(bidi_auxiliary_props_map));
+    let script = Box::into_raw(Box::new(ScriptDataForHarfBuzz {
+        script_map,
+        enum_to_name_mapper: script_enum_to_short_name_lookup,
+    }));
+    let compose = Box::into_raw(Box::new(canonical_composition));
+    let decompose = Box::into_raw(Box::new(canonical_decomposition));
+
+    unsafe extern "C" fn icu4x_hb_unicode_combining_class_destroy(user_data: *mut c_void) {
+        let _ = Box::from_raw(user_data as *mut CanonicalCombiningClassMap);
+    }
+
+    unsafe extern "C" fn icu4x_hb_unicode_general_category_destroy(user_data: *mut c_void) {
+        let _ = Box::from_raw(user_data as *mut CodePointMapData<GeneralCategory>);
+    }
+
+    unsafe extern "C" fn icu4x_hb_unicode_mirroring_destroy(user_data: *mut c_void) {
+        let _ = Box::from_raw(user_data as *mut BidiAuxiliaryProperties);
+    }
+
+    unsafe extern "C" fn icu4x_hb_unicode_script_destroy(user_data: *mut c_void) {
+        let _ = Box::from_raw(user_data as *mut ScriptDataForHarfBuzz);
+    }
+
+    unsafe extern "C" fn icu4x_hb_unicode_compose_destroy(user_data: *mut c_void) {
+        let _ = Box::from_raw(user_data as *mut CanonicalComposition);
+    }
+
+    unsafe extern "C" fn icu4x_hb_unicode_decompose_destroy(user_data: *mut c_void) {
+        let _ = Box::from_raw(user_data as *mut CanonicalDecomposition);
+    }
 
-fn create_ufuncs(
-    canonical_combining_class_map: CanonicalCombiningClassMap,
-    general_category_map: CodePointMapData<GeneralCategory>,
-    bidi_auxiliary_props_map: BidiAuxiliaryProperties,
-    script_map: CodePointMapData<Script>,
-    script_enum_to_short_name_lookup: PropertyEnumToValueNameLinearTiny4Mapper<Script>,
-    canonical_composition: CanonicalComposition,
-    canonical_decomposition: CanonicalDecomposition,
-) -> Result<UnicodeFuncs, HarfBuzzError> {
     unsafe {
         let empty = hb_unicode_funcs_get_empty();
         // The HarfBuzz refcounting convention is that "create"
@@ -391,40 +529,37 @@ fn create_ufuncs(
         hb_unicode_funcs_set_combining_class_func(
             ufuncs,
             Some(icu4x_hb_unicode_combining_class),
-            Box::into_raw(Box::new(canonical_combining_class_map)) as *mut c_void,
+            combining_class as *mut c_void,
             Some(icu4x_hb_unicode_combining_class_destroy),
         );
         hb_unicode_funcs_set_general_category_func(
             ufuncs,
             Some(icu4x_hb_unicode_general_category),
-            Box::into_raw(Box::new(general_category_map)) as *mut c_void,
+            general_category as *mut c_void,
             Some(icu4x_hb_unicode_general_category_destroy),
         );
         hb_unicode_funcs_set_mirroring_func(
             ufuncs,
             Some(icu4x_hb_unicode_mirroring),
-            Box::into_raw(Box::new(bidi_auxiliary_props_map)) as *mut c_void,
+            mirroring as *mut c_void,
             Some(icu4x_hb_unicode_mirroring_destroy),
         );
         hb_unicode_funcs_set_script_func(
             ufuncs,
             Some(icu4x_hb_unicode_script),
-            Box::into_raw(Box::new(ScriptDataForHarfBuzz {
-                script_map,
-                enum_to_name_mapper: script_enum_to_short_name_lookup,
-            })) as *mut c_void,
+            script as *mut c_void,
             Some(icu4x_hb_unicode_script_destroy),
         );
         hb_unicode_funcs_set_compose_func(
             ufuncs,
             Some(icu4x_hb_unicode_compose),
-            Box::into_raw(Box::new(canonical_composition)) as *mut c_void,
+            compose as *mut c_void,
             Some(icu4x_hb_unicode_compose_destroy),
         );
         hb_unicode_funcs_set_decompose_func(
             ufuncs,
             Some(icu4x_hb_unicode_decompose),
-            Box::into_raw(Box::new(canonical_decomposition)) as *mut c_void,
+            decompose as *mut c_void,
             Some(icu4x_hb_unicode_decompose_destroy),
         );
 

From 845d6c8940093a531e14a4a3e0c4d1b76551f737 Mon Sep 17 00:00:00 2001
From: Robert Bastian <4706271+robertbastian@users.noreply.github.com>
Date: Thu, 1 Feb 2024 19:19:45 +0100
Subject: [PATCH 2/8] clippay

---
 experimental/harfbuzz/src/lib.rs | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/experimental/harfbuzz/src/lib.rs b/experimental/harfbuzz/src/lib.rs
index b46e0e056ba..112d8e398fa 100644
--- a/experimental/harfbuzz/src/lib.rs
+++ b/experimental/harfbuzz/src/lib.rs
@@ -140,7 +140,11 @@ unsafe extern "C" fn icu4x_hb_unicode_combining_class(
     #[cfg(not(feature = "alloc"))]
     let ccc_map = None;
 
-    ccc_map.or(CCC_MAP.as_ref()).unwrap_unchecked().get32(unicode).0 as hb_unicode_combining_class_t
+    ccc_map
+        .or(CCC_MAP.as_ref())
+        .unwrap_unchecked()
+        .get32(unicode)
+        .0 as hb_unicode_combining_class_t
 }
 
 #[allow(clippy::indexing_slicing)]
@@ -414,37 +418,37 @@ pub fn new_hb_unicode_funcs() -> Result<UnicodeFuncs, HarfBuzzError> {
         hb_unicode_funcs_set_combining_class_func(
             ufuncs,
             Some(icu4x_hb_unicode_combining_class),
-            core::ptr::null_mut() as *mut c_void,
+            core::ptr::null_mut(),
             None,
         );
         hb_unicode_funcs_set_general_category_func(
             ufuncs,
             Some(icu4x_hb_unicode_general_category),
-            core::ptr::null_mut() as *mut c_void,
+            core::ptr::null_mut(),
             None,
         );
         hb_unicode_funcs_set_mirroring_func(
             ufuncs,
             Some(icu4x_hb_unicode_mirroring),
-            core::ptr::null_mut() as *mut c_void,
+            core::ptr::null_mut(),
             None,
         );
         hb_unicode_funcs_set_script_func(
             ufuncs,
             Some(icu4x_hb_unicode_script),
-            core::ptr::null_mut() as *mut c_void,
+            core::ptr::null_mut(),
             None,
         );
         hb_unicode_funcs_set_compose_func(
             ufuncs,
             Some(icu4x_hb_unicode_compose),
-            core::ptr::null_mut() as *mut c_void,
+            core::ptr::null_mut(),
             None,
         );
         hb_unicode_funcs_set_decompose_func(
             ufuncs,
             Some(icu4x_hb_unicode_decompose),
-            core::ptr::null_mut() as *mut c_void,
+            core::ptr::null_mut(),
             None,
         );
 

From 9f0384514995ca1b897a0a058ac2d00dcf413bd9 Mon Sep 17 00:00:00 2001
From: Robert Bastian <4706271+robertbastian@users.noreply.github.com>
Date: Tue, 27 Feb 2024 14:18:16 +0100
Subject: [PATCH 3/8] split

---
 experimental/harfbuzz/Cargo.toml |   1 -
 experimental/harfbuzz/src/lib.rs | 905 +++++++++++++++++--------------
 2 files changed, 508 insertions(+), 398 deletions(-)

diff --git a/experimental/harfbuzz/Cargo.toml b/experimental/harfbuzz/Cargo.toml
index f05f62b0a4a..ba3b4f09f9c 100644
--- a/experimental/harfbuzz/Cargo.toml
+++ b/experimental/harfbuzz/Cargo.toml
@@ -44,6 +44,5 @@ harfbuzz = { version = "0.4.0" }
 [features]
 default = []
 compiled_data = ["icu_normalizer/compiled_data", "icu_properties/compiled_data"]
-alloc = []
 std = []
 serde = ["icu_properties/serde", "icu_normalizer/serde", "icu_provider/serde"]
diff --git a/experimental/harfbuzz/src/lib.rs b/experimental/harfbuzz/src/lib.rs
index 112d8e398fa..6bcd221d0f4 100644
--- a/experimental/harfbuzz/src/lib.rs
+++ b/experimental/harfbuzz/src/lib.rs
@@ -28,7 +28,7 @@
 //!
 //! let mut b = Buffer::with("مساء الخير");
 //!
-//! let unicode_funcs = icu_harfbuzz::new_hb_unicode_funcs().unwrap();
+//! let unicode_funcs = icu_harfbuzz::UnicodeFuncs::new().unwrap();
 //!
 //! // NOTE: This currently requires `unsafe` code. For progress toward a safe abstraction, see:
 //! // <https://github.com/servo/rust-harfbuzz/pull/197>
@@ -41,41 +41,31 @@
 //! assert_eq!(b.get_script(), sys::HB_SCRIPT_ARABIC);
 //! ```
 
-#[cfg(feature = "alloc")]
 extern crate alloc;
 mod error;
 
 use crate::error::HarfBuzzError;
+use alloc::boxed::Box;
 use core::ffi::{c_char, c_void};
 use harfbuzz_sys::*;
 use icu_normalizer::properties::CanonicalCombiningClassMap;
 use icu_normalizer::properties::CanonicalComposition;
 use icu_normalizer::properties::CanonicalDecomposition;
 use icu_normalizer::properties::Decomposed;
-#[cfg(feature = "alloc")]
 use icu_normalizer::provider::{
     CanonicalCompositionsV1Marker, CanonicalDecompositionDataV1Marker,
     CanonicalDecompositionTablesV1Marker, NonRecursiveDecompositionSupplementV1Marker,
 };
 use icu_properties::bidi_data;
-#[cfg(feature = "alloc")]
 use icu_properties::bidi_data::BidiAuxiliaryProperties;
-use icu_properties::bidi_data::BidiAuxiliaryPropertiesBorrowed;
 use icu_properties::maps;
-#[cfg(feature = "alloc")]
 use icu_properties::maps::CodePointMapData;
-use icu_properties::maps::CodePointMapDataBorrowed;
-#[cfg(feature = "alloc")]
 use icu_properties::names::PropertyEnumToValueNameLinearTiny4Mapper;
-use icu_properties::names::PropertyEnumToValueNameLinearTiny4MapperBorrowed;
-#[cfg(feature = "alloc")]
 use icu_properties::provider::bidi_data::BidiAuxiliaryPropertiesV1Marker;
-#[cfg(feature = "alloc")]
 use icu_properties::provider::{
     GeneralCategoryV1Marker, ScriptV1Marker, ScriptValueToShortNameV1Marker,
 };
 use icu_properties::{GeneralCategory, Script};
-#[cfg(feature = "alloc")]
 use icu_provider::prelude::*;
 use tinystr::tinystr;
 
@@ -125,228 +115,6 @@ static ICU4X_GENERAL_CATEGORY_TO_HARFBUZZ: [u8; 30] = [
     HB_UNICODE_GENERAL_CATEGORY_FINAL_PUNCTUATION as u8, // FinalPunctuation = 29,
 ];
 
-unsafe extern "C" fn icu4x_hb_unicode_combining_class(
-    _ufuncs: *mut hb_unicode_funcs_t,
-    unicode: hb_codepoint_t,
-    #[cfg_attr(not(feature = "alloc"), allow(unused_variables))] user_data: *mut c_void,
-) -> hb_unicode_combining_class_t {
-    #[cfg(feature = "compiled_data")]
-    const CCC_MAP: Option<CanonicalCombiningClassMap> = Some(CanonicalCombiningClassMap::new());
-    #[cfg(not(feature = "compiled_data"))]
-    const CCC_MAP: Option<CanonicalCombiningClassMap> = None;
-
-    #[cfg(feature = "alloc")]
-    let ccc_map = (user_data as *mut CanonicalCombiningClassMap).as_ref();
-    #[cfg(not(feature = "alloc"))]
-    let ccc_map = None;
-
-    ccc_map
-        .or(CCC_MAP.as_ref())
-        .unwrap_unchecked()
-        .get32(unicode)
-        .0 as hb_unicode_combining_class_t
-}
-
-#[allow(clippy::indexing_slicing)]
-unsafe extern "C" fn icu4x_hb_unicode_general_category(
-    _ufuncs: *mut hb_unicode_funcs_t,
-    unicode: hb_codepoint_t,
-    #[cfg_attr(not(feature = "alloc"), allow(unused_variables))] user_data: *mut c_void,
-) -> hb_unicode_general_category_t {
-    #[cfg(feature = "compiled_data")]
-    const GC_MAP: Option<CodePointMapDataBorrowed<'static, GeneralCategory>> =
-        Some(maps::general_category());
-    #[cfg(not(feature = "compiled_data"))]
-    const GC_MAP: Option<CodePointMapDataBorrowed<'static, GeneralCategory>> = None;
-
-    #[cfg(feature = "alloc")]
-    let gc_map = (user_data as *mut CodePointMapData<GeneralCategory>)
-        .as_ref()
-        .map(CodePointMapData::as_borrowed);
-    #[cfg(not(feature = "alloc"))]
-    let gc_map = None;
-
-    // Indexing is OK, because `GeneralCategory` data is validated upon
-    // deserialization so that there can be no out-of-range `GeneralCategory`
-    // values (which would be UB to materialize). `GeneralCategory` is a
-    // stable exhaustive enum, and the length of `ICU4X_GENERAL_CATEGORY_TO_HARFBUZZ`
-    // matches the number of enum items, so the index will always be in range here.
-    ICU4X_GENERAL_CATEGORY_TO_HARFBUZZ[gc_map.or(GC_MAP).unwrap_unchecked().get32(unicode) as usize]
-        as hb_unicode_general_category_t
-}
-
-/// Returns the Bidi_Mirroring_Glyph, but adjusting the return value
-/// to fix HarfBuzz expected behavior for code points whose property value
-/// for Bidi_Mirroring_Glyph is the undefined value.
-///
-/// From HarfBuzz docs on `hb_unicode_mirroring_func_t`:
-/// <note>Note: If a code point does not have a specified
-/// Bi-Directional Mirroring Glyph defined, the method should
-/// return the original code point.</note>
-unsafe extern "C" fn icu4x_hb_unicode_mirroring(
-    _ufuncs: *mut hb_unicode_funcs_t,
-    unicode: hb_codepoint_t,
-    #[cfg_attr(not(feature = "alloc"), allow(unused_variables))] user_data: *mut c_void,
-) -> hb_codepoint_t {
-    #[cfg(feature = "compiled_data")]
-    const BIDI: Option<BidiAuxiliaryPropertiesBorrowed<'static>> =
-        Some(bidi_data::bidi_auxiliary_properties());
-    #[cfg(not(feature = "compiled_data"))]
-    const BIDI: Option<BidiAuxiliaryPropertiesBorrowed<'static>> = None;
-
-    #[cfg(feature = "alloc")]
-    let bidi = (user_data as *mut BidiAuxiliaryProperties)
-        .as_ref()
-        .map(BidiAuxiliaryProperties::as_borrowed);
-    #[cfg(not(feature = "alloc"))]
-    let bidi = None;
-
-    bidi.or(BIDI)
-        .unwrap_unchecked()
-        .get32_mirroring_props(unicode)
-        .mirroring_glyph
-        .map(u32::from)
-        .unwrap_or(unicode) as hb_codepoint_t
-}
-
-#[cfg(feature = "alloc")]
-struct ScriptDataForHarfBuzz {
-    script_map: CodePointMapData<Script>,
-    enum_to_name_mapper: PropertyEnumToValueNameLinearTiny4Mapper<Script>,
-}
-
-unsafe extern "C" fn icu4x_hb_unicode_script(
-    _ufuncs: *mut hb_unicode_funcs_t,
-    unicode: hb_codepoint_t,
-    #[cfg_attr(not(feature = "alloc"), allow(unused_variables))] user_data: *mut c_void,
-) -> hb_script_t {
-    #[cfg(feature = "compiled_data")]
-    const SCRIPT_MAP: Option<CodePointMapDataBorrowed<'static, Script>> = Some(maps::script());
-    #[cfg(feature = "compiled_data")]
-    const ENUM_TO_NAME_MAPPER: Option<PropertyEnumToValueNameLinearTiny4MapperBorrowed<Script>> =
-        Some(Script::enum_to_short_name_mapper());
-    #[cfg(not(feature = "compiled_data"))]
-    const SCRIPT_MAP: Option<CodePointMapDataBorrowed<'static, Script>> = None;
-    #[cfg(not(feature = "compiled_data"))]
-    const ENUM_TO_NAME_MAPPER: Option<PropertyEnumToValueNameLinearTiny4MapperBorrowed<Script>> =
-        None;
-
-    #[cfg(feature = "alloc")]
-    let script_map = (user_data as *mut ScriptDataForHarfBuzz)
-        .as_ref()
-        .map(|d| d.script_map.as_borrowed());
-    #[cfg(feature = "alloc")]
-    let enum_to_name_mapper = (user_data as *mut ScriptDataForHarfBuzz)
-        .as_ref()
-        .map(|d| d.enum_to_name_mapper.as_borrowed());
-    #[cfg(not(feature = "alloc"))]
-    let script_map = None;
-    #[cfg(not(feature = "alloc"))]
-    let enum_to_name_mapper = None;
-
-    let script = script_map.or(SCRIPT_MAP).unwrap_unchecked().get32(unicode);
-    let name = enum_to_name_mapper
-        .or(ENUM_TO_NAME_MAPPER)
-        .unwrap_unchecked()
-        .get(script)
-        .unwrap_or(tinystr!(4, "Zzzz"));
-
-    hb_script_from_string(
-        name.as_ptr() as *const c_char,
-        name.len().try_into().unwrap_or(0),
-    )
-}
-
-unsafe extern "C" fn icu4x_hb_unicode_compose(
-    _ufuncs: *mut hb_unicode_funcs_t,
-    a: hb_codepoint_t,
-    b: hb_codepoint_t,
-    ab: *mut hb_codepoint_t,
-    #[cfg_attr(not(feature = "alloc"), allow(unused_variables))] user_data: *mut c_void,
-) -> hb_bool_t {
-    #[cfg(feature = "compiled_data")]
-    const CC: Option<CanonicalComposition> = Some(CanonicalComposition::new());
-    #[cfg(not(feature = "compiled_data"))]
-    const CC: Option<CanonicalComposition> = None;
-
-    #[cfg(feature = "alloc")]
-    let cc = (user_data as *mut CanonicalComposition).as_ref();
-    #[cfg(not(feature = "alloc"))]
-    let cc = None;
-
-    // It appears that HarfBuzz will pass valid scalar values
-    // unless the application violated the contract of
-    // `hb_buffer_add_codepoints` and passed in non-scalar values.
-    // If we treated `hb_buffer_add_codepoints` as conceptually
-    // `unsafe`, it would be appropriate not to do scalar value
-    // validation here.
-    let first = if let Some(first) = core::char::from_u32(a) {
-        first
-    } else {
-        // GIGO case
-        debug_assert!(false);
-        return false as hb_bool_t;
-    };
-    let second = if let Some(second) = core::char::from_u32(b) {
-        second
-    } else {
-        // GIGO case
-        debug_assert!(false);
-        return false as hb_bool_t;
-    };
-    if let Some(c) = cc.or(CC.as_ref()).unwrap_unchecked().compose(first, second) {
-        core::ptr::write(ab, c as hb_codepoint_t);
-        true as hb_bool_t
-    } else {
-        false as hb_bool_t
-    }
-}
-
-unsafe extern "C" fn icu4x_hb_unicode_decompose(
-    _ufuncs: *mut hb_unicode_funcs_t,
-    ab: hb_codepoint_t,
-    a: *mut hb_codepoint_t,
-    b: *mut hb_codepoint_t,
-    #[cfg_attr(not(feature = "alloc"), allow(unused_variables))] user_data: *mut c_void,
-) -> hb_bool_t {
-    #[cfg(feature = "compiled_data")]
-    const DC: Option<CanonicalDecomposition> = Some(CanonicalDecomposition::new());
-    #[cfg(not(feature = "compiled_data"))]
-    const DC: Option<CanonicalDecomposition> = None;
-
-    #[cfg(feature = "alloc")]
-    let dc = (user_data as *mut CanonicalDecomposition).as_ref();
-    #[cfg(not(feature = "alloc"))]
-    let dc = None;
-
-    // It appears that HarfBuzz will pass valid scalar values
-    // unless the application violated the contract of
-    // `hb_buffer_add_codepoints` and passed in non-scalar values.
-    // If we treated `hb_buffer_add_codepoints` as conceptually
-    // `unsafe`, it would be appropriate not to do scalar value
-    // validation here.
-    let composed = if let Some(composed) = core::char::from_u32(ab) {
-        composed
-    } else {
-        // GIGO case
-        debug_assert!(false);
-        return false as hb_bool_t;
-    };
-    match dc.or(DC.as_ref()).unwrap_unchecked().decompose(composed) {
-        Decomposed::Default => false as hb_bool_t,
-        Decomposed::Expansion(first, second) => {
-            core::ptr::write(a, first as hb_codepoint_t);
-            core::ptr::write(b, second as hb_codepoint_t);
-            true as hb_bool_t
-        }
-        Decomposed::Singleton(single) => {
-            core::ptr::write(a, single as hb_codepoint_t);
-            core::ptr::write(b, 0);
-            true as hb_bool_t
-        }
-    }
-}
-
 /// RAII holder for `*mut hb_unicode_funcs_t`.
 // TODO(#2838): Document the conditions under which multithreaded lookups via the
 // `*mut hb_unicode_funcs_t` are OK. HarfBuzz itself takes care of atomic
@@ -386,192 +154,535 @@ impl UnicodeFuncs {
     pub fn as_ptr(&self) -> *mut hb_unicode_funcs_t {
         self.raw
     }
-}
-
-impl Drop for UnicodeFuncs {
-    fn drop(&mut self) {
-        unsafe {
-            hb_unicode_funcs_destroy(self.raw);
-        }
-    }
-}
 
-/// Sets up a `hb_unicode_funcs_t` with ICU4X compiled data as the back end as the Unicode
-/// Database operations that HarfBuzz needs. The `hb_unicode_funcs_t` held
-/// by the returned `UnicodeFuncs` is marked immutable.
-///
-/// ✨ *Enabled with the `compiled_data` Cargo feature.*
-///
-/// [📚 Help choosing a constructor](icu_provider::constructors)
-#[cfg(feature = "compiled_data")]
-pub fn new_hb_unicode_funcs() -> Result<UnicodeFuncs, HarfBuzzError> {
-    unsafe {
-        let empty = hb_unicode_funcs_get_empty();
+    fn empty() -> Result<Self, HarfBuzzError> {
+        let empty = unsafe { hb_unicode_funcs_get_empty() };
         // The HarfBuzz refcounting convention is that "create"
         // sets refcount to one, not zero.
         // https://harfbuzz.github.io/object-model-lifecycle.html
-        let ufuncs = hb_unicode_funcs_create(empty);
-        if ufuncs == empty {
-            return Err(HarfBuzzError::Alloc);
+        let raw = unsafe { hb_unicode_funcs_create(empty) };
+        if raw == empty {
+            Err(HarfBuzzError::Alloc)
+        } else {
+            Ok(Self { raw })
+        }
+    }
+
+    /// Sets up a `hb_unicode_funcs_t` with ICU4X compiled data as the back end as the Unicode
+    /// Database operations that HarfBuzz needs. The `hb_unicode_funcs_t` held
+    /// by the returned `UnicodeFuncs` is marked immutable.
+    ///
+    /// ✨ *Enabled with the `compiled_data` Cargo feature.*
+    ///
+    /// [📚 Help choosing a constructor](icu_provider::constructors)
+    #[cfg(feature = "compiled_data")]
+    pub fn new() -> Result<Self, HarfBuzzError> {
+        let ufuncs = Self::empty()?;
+
+        unsafe {
+            hb_unicode_funcs_set_combining_class_func(
+                ufuncs.raw,
+                Some({
+                    extern "C" fn cb(
+                        _: *mut hb_unicode_funcs_t,
+                        unicode: hb_codepoint_t,
+                        _: *mut c_void,
+                    ) -> hb_unicode_combining_class_t {
+                        CanonicalCombiningClassMap::new().get32(unicode).0
+                            as hb_unicode_combining_class_t
+                    }
+                    cb
+                }),
+                core::ptr::null_mut(),
+                None,
+            );
+        }
+
+        unsafe {
+            hb_unicode_funcs_set_general_category_func(
+                ufuncs.raw,
+                Some({
+                    #[allow(clippy::indexing_slicing)]
+                    extern "C" fn cb(
+                        _: *mut hb_unicode_funcs_t,
+                        unicode: hb_codepoint_t,
+                        _: *mut c_void,
+                    ) -> hb_unicode_general_category_t {
+                        // Indexing is OK, because `GeneralCategory` data is validated upon
+                        // deserialization so that there can be no out-of-range `GeneralCategory`
+                        // values (which would be UB to materialize). `GeneralCategory` is a
+                        // stable exhaustive enum, and the length of `ICU4X_GENERAL_CATEGORY_TO_HARFBUZZ`
+                        // matches the number of enum items, so the index will always be in range here.
+                        ICU4X_GENERAL_CATEGORY_TO_HARFBUZZ
+                            [maps::general_category().get32(unicode) as usize]
+                            as hb_unicode_general_category_t
+                    }
+                    cb
+                }),
+                core::ptr::null_mut(),
+                None,
+            );
+        }
+
+        // Returns the Bidi_Mirroring_Glyph, but adjusting the return value
+        // to fix HarfBuzz expected behavior for code points whose property value
+        // for Bidi_Mirroring_Glyph is the undefined value.
+        //
+        // From HarfBuzz docs on `hb_unicode_mirroring_func_t`:
+        // <note>Note: If a code point does not have a specified
+        // Bi-Directional Mirroring Glyph defined, the method should
+        // return the original code point.</note>
+        unsafe {
+            hb_unicode_funcs_set_mirroring_func(
+                ufuncs.raw,
+                Some({
+                    extern "C" fn cb(
+                        _: *mut hb_unicode_funcs_t,
+                        unicode: hb_codepoint_t,
+                        _: *mut c_void,
+                    ) -> hb_codepoint_t {
+                        bidi_data::bidi_auxiliary_properties()
+                            .get32_mirroring_props(unicode)
+                            .mirroring_glyph
+                            .map(u32::from)
+                            .unwrap_or(unicode) as hb_codepoint_t
+                    }
+                    cb
+                }),
+                core::ptr::null_mut(),
+                None,
+            );
+        }
+
+        unsafe {
+            hb_unicode_funcs_set_script_func(
+                ufuncs.raw,
+                Some({
+                    extern "C" fn cb(
+                        _: *mut hb_unicode_funcs_t,
+                        unicode: hb_codepoint_t,
+                        _: *mut c_void,
+                    ) -> hb_script_t {
+                        let script = maps::script().get32(unicode);
+                        let name = Script::enum_to_short_name_mapper()
+                            .get(script)
+                            .unwrap_or(tinystr!(4, "Zzzz"));
+
+                        unsafe {
+                            hb_script_from_string(
+                                name.as_ptr() as *const c_char,
+                                name.len().try_into().unwrap_or(0),
+                            )
+                        }
+                    }
+                    cb
+                }),
+                core::ptr::null_mut(),
+                None,
+            );
+        }
+
+        unsafe {
+            hb_unicode_funcs_set_compose_func(
+                ufuncs.raw,
+                Some({
+                    extern "C" fn cb(
+                        _: *mut hb_unicode_funcs_t,
+                        a: hb_codepoint_t,
+                        b: hb_codepoint_t,
+                        ab: *mut hb_codepoint_t,
+                        _: *mut c_void,
+                    ) -> hb_bool_t {
+                        // It appears that HarfBuzz will pass valid scalar values
+                        // unless the application violated the contract of
+                        // `hb_buffer_add_codepoints` and passed in non-scalar values.
+                        // If we treated `hb_buffer_add_codepoints` as conceptually
+                        // `unsafe`, it would be appropriate not to do scalar value
+                        // validation here.
+                        let first = if let Some(first) = core::char::from_u32(a) {
+                            first
+                        } else {
+                            // GIGO case
+                            debug_assert!(false);
+                            return false as hb_bool_t;
+                        };
+                        let second = if let Some(second) = core::char::from_u32(b) {
+                            second
+                        } else {
+                            // GIGO case
+                            debug_assert!(false);
+                            return false as hb_bool_t;
+                        };
+                        if let Some(c) = CanonicalComposition::new().compose(first, second) {
+                            unsafe {
+                                core::ptr::write(ab, c as hb_codepoint_t);
+                            }
+                            true as hb_bool_t
+                        } else {
+                            false as hb_bool_t
+                        }
+                    }
+                    cb
+                }),
+                core::ptr::null_mut(),
+                None,
+            );
+        }
+
+        unsafe {
+            hb_unicode_funcs_set_decompose_func(
+                ufuncs.raw,
+                Some({
+                    extern "C" fn cb(
+                        _: *mut hb_unicode_funcs_t,
+                        ab: hb_codepoint_t,
+                        a: *mut hb_codepoint_t,
+                        b: *mut hb_codepoint_t,
+                        _: *mut c_void,
+                    ) -> hb_bool_t {
+                        // It appears that HarfBuzz will pass valid scalar values
+                        // unless the application violated the contract of
+                        // `hb_buffer_add_codepoints` and passed in non-scalar values.
+                        // If we treated `hb_buffer_add_codepoints` as conceptually
+                        // `unsafe`, it would be appropriate not to do scalar value
+                        // validation here.
+                        let composed = if let Some(composed) = core::char::from_u32(ab) {
+                            composed
+                        } else {
+                            // GIGO case
+                            debug_assert!(false);
+                            return false as hb_bool_t;
+                        };
+                        match CanonicalDecomposition::new().decompose(composed) {
+                            Decomposed::Default => false as hb_bool_t,
+                            Decomposed::Expansion(first, second) => {
+                                unsafe {
+                                    core::ptr::write(a, first as hb_codepoint_t);
+                                }
+                                unsafe {
+                                    core::ptr::write(b, second as hb_codepoint_t);
+                                }
+                                true as hb_bool_t
+                            }
+                            Decomposed::Singleton(single) => {
+                                unsafe {
+                                    core::ptr::write(a, single as hb_codepoint_t);
+                                }
+                                unsafe {
+                                    core::ptr::write(b, 0);
+                                }
+                                true as hb_bool_t
+                            }
+                        }
+                    }
+                    cb
+                }),
+                core::ptr::null_mut(),
+                None,
+            );
         }
-        // Below this point, the only return is upon success.
-        hb_unicode_funcs_set_combining_class_func(
-            ufuncs,
-            Some(icu4x_hb_unicode_combining_class),
-            core::ptr::null_mut(),
-            None,
-        );
-        hb_unicode_funcs_set_general_category_func(
-            ufuncs,
-            Some(icu4x_hb_unicode_general_category),
-            core::ptr::null_mut(),
-            None,
-        );
-        hb_unicode_funcs_set_mirroring_func(
-            ufuncs,
-            Some(icu4x_hb_unicode_mirroring),
-            core::ptr::null_mut(),
-            None,
-        );
-        hb_unicode_funcs_set_script_func(
-            ufuncs,
-            Some(icu4x_hb_unicode_script),
-            core::ptr::null_mut(),
-            None,
-        );
-        hb_unicode_funcs_set_compose_func(
-            ufuncs,
-            Some(icu4x_hb_unicode_compose),
-            core::ptr::null_mut(),
-            None,
-        );
-        hb_unicode_funcs_set_decompose_func(
-            ufuncs,
-            Some(icu4x_hb_unicode_decompose),
-            core::ptr::null_mut(),
-            None,
-        );
 
         // Compatibility decomposition and East Asian Width lookups
         // are deprecated, and there's no need to set up the callbacks
         // for those.
 
-        hb_unicode_funcs_make_immutable(ufuncs);
-        Ok(UnicodeFuncs::from_raw(ufuncs))
+        unsafe {
+            hb_unicode_funcs_make_immutable(ufuncs.raw);
+        }
+        Ok(ufuncs)
     }
-}
 
-#[doc = icu_provider::gen_any_buffer_unstable_docs!(UNSTABLE, new_hb_unicode_funcs)]
-#[cfg(feature = "alloc")]
-pub fn new_hb_unicode_funcs_unstable<D>(provider: &D) -> Result<UnicodeFuncs, HarfBuzzError>
-where
-    D: DataProvider<BidiAuxiliaryPropertiesV1Marker>
-        + DataProvider<CanonicalCompositionsV1Marker>
-        + DataProvider<CanonicalDecompositionDataV1Marker>
-        + DataProvider<CanonicalDecompositionTablesV1Marker>
-        + DataProvider<NonRecursiveDecompositionSupplementV1Marker>
-        + DataProvider<GeneralCategoryV1Marker>
-        + DataProvider<ScriptValueToShortNameV1Marker>
-        + DataProvider<ScriptV1Marker>
-        + ?Sized,
-{
-    let canonical_combining_class_map = CanonicalCombiningClassMap::try_new_unstable(provider)?;
-    let general_category_map = maps::load_general_category(provider)?;
-    let bidi_auxiliary_props_map = bidi_data::load_bidi_auxiliary_properties_unstable(provider)?;
-    let script_map = maps::load_script(provider)?;
-    let script_enum_to_short_name_lookup = Script::get_enum_to_short_name_mapper(provider)?;
-    let canonical_composition = CanonicalComposition::try_new_unstable(provider)?;
-    let canonical_decomposition = CanonicalDecomposition::try_new_unstable(provider)?;
-
-    #[cfg(not(feature = "std"))]
-    use alloc::boxed::Box;
-
-    let combining_class = Box::into_raw(Box::new(canonical_combining_class_map));
-    let general_category = Box::into_raw(Box::new(general_category_map));
-    let mirroring = Box::into_raw(Box::new(bidi_auxiliary_props_map));
-    let script = Box::into_raw(Box::new(ScriptDataForHarfBuzz {
-        script_map,
-        enum_to_name_mapper: script_enum_to_short_name_lookup,
-    }));
-    let compose = Box::into_raw(Box::new(canonical_composition));
-    let decompose = Box::into_raw(Box::new(canonical_decomposition));
-
-    unsafe extern "C" fn icu4x_hb_unicode_combining_class_destroy(user_data: *mut c_void) {
-        let _ = Box::from_raw(user_data as *mut CanonicalCombiningClassMap);
-    }
+    #[doc = icu_provider::gen_any_buffer_unstable_docs!(UNSTABLE, new_hb_unicode_funcs)]
+    pub fn new_unstable<D>(provider: &D) -> Result<UnicodeFuncs, HarfBuzzError>
+    where
+        D: DataProvider<BidiAuxiliaryPropertiesV1Marker>
+            + DataProvider<CanonicalCompositionsV1Marker>
+            + DataProvider<CanonicalDecompositionDataV1Marker>
+            + DataProvider<CanonicalDecompositionTablesV1Marker>
+            + DataProvider<NonRecursiveDecompositionSupplementV1Marker>
+            + DataProvider<GeneralCategoryV1Marker>
+            + DataProvider<ScriptValueToShortNameV1Marker>
+            + DataProvider<ScriptV1Marker>
+            + ?Sized,
+    {
+        let ufuncs = Self::empty()?;
+
+        let canonical_combining_class_map = CanonicalCombiningClassMap::try_new_unstable(provider)?;
+        let general_category_map = maps::load_general_category(provider)?;
+        let bidi_auxiliary_props_map =
+            bidi_data::load_bidi_auxiliary_properties_unstable(provider)?;
+        let script_map = maps::load_script(provider)?;
+        let script_enum_to_short_name_lookup = Script::get_enum_to_short_name_mapper(provider)?;
+        let canonical_composition = CanonicalComposition::try_new_unstable(provider)?;
+        let canonical_decomposition = CanonicalDecomposition::try_new_unstable(provider)?;
 
-    unsafe extern "C" fn icu4x_hb_unicode_general_category_destroy(user_data: *mut c_void) {
-        let _ = Box::from_raw(user_data as *mut CodePointMapData<GeneralCategory>);
-    }
+        unsafe {
+            hb_unicode_funcs_set_combining_class_func(
+                ufuncs.raw,
+                Some({
+                    extern "C" fn cb(
+                        _: *mut hb_unicode_funcs_t,
+                        unicode: hb_codepoint_t,
+                        user_data: *mut c_void,
+                    ) -> hb_unicode_combining_class_t {
+                        unsafe { &*(user_data as *mut CanonicalCombiningClassMap) }
+                            .get32(unicode)
+                            .0 as hb_unicode_combining_class_t
+                    }
+                    cb
+                }),
+                Box::into_raw(Box::new(canonical_combining_class_map)) as *mut c_void,
+                Some({
+                    extern "C" fn cb(user_data: *mut c_void) {
+                        drop(unsafe {
+                            Box::from_raw(user_data as *mut CanonicalCombiningClassMap)
+                        });
+                    }
+                    cb
+                }),
+            );
+        }
 
-    unsafe extern "C" fn icu4x_hb_unicode_mirroring_destroy(user_data: *mut c_void) {
-        let _ = Box::from_raw(user_data as *mut BidiAuxiliaryProperties);
-    }
+        unsafe {
+            hb_unicode_funcs_set_general_category_func(
+                ufuncs.raw,
+                Some({
+                    #[allow(clippy::indexing_slicing)]
+                    extern "C" fn cb(
+                        _: *mut hb_unicode_funcs_t,
+                        unicode: hb_codepoint_t,
+                        user_data: *mut c_void,
+                    ) -> hb_unicode_general_category_t {
+                        // Indexing is OK, because `GeneralCategory` data is validated upon
+                        // deserialization so that there can be no out-of-range `GeneralCategory`
+                        // values (which would be UB to materialize). `GeneralCategory` is a
+                        // stable exhaustive enum, and the length of `ICU4X_GENERAL_CATEGORY_TO_HARFBUZZ`
+                        // matches the number of enum items, so the index will always be in range here.
+                        ICU4X_GENERAL_CATEGORY_TO_HARFBUZZ[unsafe {
+                            &*(user_data as *mut CodePointMapData<GeneralCategory>)
+                        }
+                        .as_borrowed()
+                        .get32(unicode)
+                            as usize] as hb_unicode_general_category_t
+                    }
+                    cb
+                }),
+                Box::into_raw(Box::new(general_category_map)) as *mut c_void,
+                Some({
+                    extern "C" fn cb(user_data: *mut c_void) {
+                        drop(unsafe {
+                            Box::from_raw(user_data as *mut CodePointMapData<GeneralCategory>)
+                        });
+                    }
+                    cb
+                }),
+            );
+        }
 
-    unsafe extern "C" fn icu4x_hb_unicode_script_destroy(user_data: *mut c_void) {
-        let _ = Box::from_raw(user_data as *mut ScriptDataForHarfBuzz);
-    }
+        // Returns the Bidi_Mirroring_Glyph, but adjusting the return value
+        // to fix HarfBuzz expected behavior for code points whose property value
+        // for Bidi_Mirroring_Glyph is the undefined value.
+        //
+        // From HarfBuzz docs on `hb_unicode_mirroring_func_t`:
+        // <note>Note: If a code point does not have a specified
+        // Bi-Directional Mirroring Glyph defined, the method should
+        // return the original code point.</note>
+        unsafe {
+            hb_unicode_funcs_set_mirroring_func(
+                ufuncs.raw,
+                Some({
+                    extern "C" fn cb(
+                        _: *mut hb_unicode_funcs_t,
+                        unicode: hb_codepoint_t,
+                        user_data: *mut c_void,
+                    ) -> hb_codepoint_t {
+                        unsafe { &*(user_data as *mut BidiAuxiliaryProperties) }
+                            .as_borrowed()
+                            .get32_mirroring_props(unicode)
+                            .mirroring_glyph
+                            .map(u32::from)
+                            .unwrap_or(unicode) as hb_codepoint_t
+                    }
+                    cb
+                }),
+                Box::into_raw(Box::new(bidi_auxiliary_props_map)) as *mut c_void,
+                Some({
+                    extern "C" fn cb(user_data: *mut c_void) {
+                        drop(unsafe { Box::from_raw(user_data as *mut BidiAuxiliaryProperties) });
+                    }
+                    cb
+                }),
+            );
+        }
 
-    unsafe extern "C" fn icu4x_hb_unicode_compose_destroy(user_data: *mut c_void) {
-        let _ = Box::from_raw(user_data as *mut CanonicalComposition);
-    }
+        struct ScriptDataForHarfBuzz {
+            script_map: CodePointMapData<Script>,
+            enum_to_name_mapper: PropertyEnumToValueNameLinearTiny4Mapper<Script>,
+        }
 
-    unsafe extern "C" fn icu4x_hb_unicode_decompose_destroy(user_data: *mut c_void) {
-        let _ = Box::from_raw(user_data as *mut CanonicalDecomposition);
-    }
+        unsafe {
+            hb_unicode_funcs_set_script_func(
+                ufuncs.raw,
+                Some({
+                    extern "C" fn cb(
+                        _: *mut hb_unicode_funcs_t,
+                        unicode: hb_codepoint_t,
+                        user_data: *mut c_void,
+                    ) -> hb_script_t {
+                        let script = unsafe { &*(user_data as *mut ScriptDataForHarfBuzz) }
+                            .script_map
+                            .as_borrowed()
+                            .get32(unicode);
+                        let name = unsafe { &*(user_data as *mut ScriptDataForHarfBuzz) }
+                            .enum_to_name_mapper
+                            .as_borrowed()
+                            .get(script)
+                            .unwrap_or(tinystr!(4, "Zzzz"));
+
+                        unsafe {
+                            hb_script_from_string(
+                                name.as_ptr() as *const c_char,
+                                name.len().try_into().unwrap_or(0),
+                            )
+                        }
+                    }
+                    cb
+                }),
+                Box::into_raw(Box::new(ScriptDataForHarfBuzz {
+                    script_map,
+                    enum_to_name_mapper: script_enum_to_short_name_lookup,
+                })) as *mut c_void,
+                Some({
+                    extern "C" fn cb(user_data: *mut c_void) {
+                        drop(unsafe { Box::from_raw(user_data as *mut ScriptDataForHarfBuzz) });
+                    }
+                    cb
+                }),
+            );
+        }
 
-    unsafe {
-        let empty = hb_unicode_funcs_get_empty();
-        // The HarfBuzz refcounting convention is that "create"
-        // sets refcount to one, not zero.
-        // https://harfbuzz.github.io/object-model-lifecycle.html
-        let ufuncs = hb_unicode_funcs_create(empty);
-        if ufuncs == empty {
-            return Err(HarfBuzzError::Alloc);
+        unsafe {
+            hb_unicode_funcs_set_compose_func(
+                ufuncs.raw,
+                Some({
+                    extern "C" fn cb(
+                        _: *mut hb_unicode_funcs_t,
+                        a: hb_codepoint_t,
+                        b: hb_codepoint_t,
+                        ab: *mut hb_codepoint_t,
+                        user_data: *mut c_void,
+                    ) -> hb_bool_t {
+                        // It appears that HarfBuzz will pass valid scalar values
+                        // unless the application violated the contract of
+                        // `hb_buffer_add_codepoints` and passed in non-scalar values.
+                        // If we treated `hb_buffer_add_codepoints` as conceptually
+                        // `unsafe`, it would be appropriate not to do scalar value
+                        // validation here.
+                        let first = if let Some(first) = core::char::from_u32(a) {
+                            first
+                        } else {
+                            // GIGO case
+                            debug_assert!(false);
+                            return false as hb_bool_t;
+                        };
+                        let second = if let Some(second) = core::char::from_u32(b) {
+                            second
+                        } else {
+                            // GIGO case
+                            debug_assert!(false);
+                            return false as hb_bool_t;
+                        };
+                        if let Some(c) = unsafe { &*(user_data as *mut CanonicalComposition) }
+                            .compose(first, second)
+                        {
+                            unsafe {
+                                core::ptr::write(ab, c as hb_codepoint_t);
+                            }
+                            true as hb_bool_t
+                        } else {
+                            false as hb_bool_t
+                        }
+                    }
+                    cb
+                }),
+                Box::into_raw(Box::new(canonical_composition)) as *mut c_void,
+                Some({
+                    extern "C" fn cb(user_data: *mut c_void) {
+                        drop(unsafe { Box::from_raw(user_data as *mut CanonicalComposition) });
+                    }
+                    cb
+                }),
+            );
+        }
+
+        unsafe {
+            hb_unicode_funcs_set_decompose_func(
+                ufuncs.raw,
+                Some({
+                    extern "C" fn cb(
+                        _: *mut hb_unicode_funcs_t,
+                        ab: hb_codepoint_t,
+                        a: *mut hb_codepoint_t,
+                        b: *mut hb_codepoint_t,
+                        user_data: *mut c_void,
+                    ) -> hb_bool_t {
+                        // It appears that HarfBuzz will pass valid scalar values
+                        // unless the application violated the contract of
+                        // `hb_buffer_add_codepoints` and passed in non-scalar values.
+                        // If we treated `hb_buffer_add_codepoints` as conceptually
+                        // `unsafe`, it would be appropriate not to do scalar value
+                        // validation here.
+                        let composed = if let Some(composed) = core::char::from_u32(ab) {
+                            composed
+                        } else {
+                            // GIGO case
+                            debug_assert!(false);
+                            return false as hb_bool_t;
+                        };
+                        match unsafe { &*(user_data as *mut CanonicalDecomposition) }
+                            .decompose(composed)
+                        {
+                            Decomposed::Default => false as hb_bool_t,
+                            Decomposed::Expansion(first, second) => {
+                                unsafe { core::ptr::write(a, first as hb_codepoint_t) };
+                                unsafe { core::ptr::write(b, second as hb_codepoint_t) };
+                                true as hb_bool_t
+                            }
+                            Decomposed::Singleton(single) => {
+                                unsafe { core::ptr::write(a, single as hb_codepoint_t) };
+                                unsafe { core::ptr::write(b, 0) };
+                                true as hb_bool_t
+                            }
+                        }
+                    }
+                    cb
+                }),
+                Box::into_raw(Box::new(canonical_decomposition)) as *mut c_void,
+                Some({
+                    extern "C" fn cb(user_data: *mut c_void) {
+                        drop(unsafe { Box::from_raw(user_data as *mut CanonicalDecomposition) });
+                    }
+                    cb
+                }),
+            );
         }
-        // Below this point, the only return is upon success.
-        hb_unicode_funcs_set_combining_class_func(
-            ufuncs,
-            Some(icu4x_hb_unicode_combining_class),
-            combining_class as *mut c_void,
-            Some(icu4x_hb_unicode_combining_class_destroy),
-        );
-        hb_unicode_funcs_set_general_category_func(
-            ufuncs,
-            Some(icu4x_hb_unicode_general_category),
-            general_category as *mut c_void,
-            Some(icu4x_hb_unicode_general_category_destroy),
-        );
-        hb_unicode_funcs_set_mirroring_func(
-            ufuncs,
-            Some(icu4x_hb_unicode_mirroring),
-            mirroring as *mut c_void,
-            Some(icu4x_hb_unicode_mirroring_destroy),
-        );
-        hb_unicode_funcs_set_script_func(
-            ufuncs,
-            Some(icu4x_hb_unicode_script),
-            script as *mut c_void,
-            Some(icu4x_hb_unicode_script_destroy),
-        );
-        hb_unicode_funcs_set_compose_func(
-            ufuncs,
-            Some(icu4x_hb_unicode_compose),
-            compose as *mut c_void,
-            Some(icu4x_hb_unicode_compose_destroy),
-        );
-        hb_unicode_funcs_set_decompose_func(
-            ufuncs,
-            Some(icu4x_hb_unicode_decompose),
-            decompose as *mut c_void,
-            Some(icu4x_hb_unicode_decompose_destroy),
-        );
 
         // Compatibility decomposition and East Asian Width lookups
         // are deprecated, and there's no need to set up the callbacks
         // for those.
 
-        hb_unicode_funcs_make_immutable(ufuncs);
-        Ok(UnicodeFuncs::from_raw(ufuncs))
+        unsafe {
+            hb_unicode_funcs_make_immutable(ufuncs.raw);
+        }
+
+        Ok(ufuncs)
+    }
+}
+
+impl Drop for UnicodeFuncs {
+    fn drop(&mut self) {
+        unsafe {
+            hb_unicode_funcs_destroy(self.raw);
+        }
     }
 }

From a2c845c545b178030f7ba8fc9a5ee8947b52e93a Mon Sep 17 00:00:00 2001
From: Robert Bastian <4706271+robertbastian@users.noreply.github.com>
Date: Tue, 27 Feb 2024 14:22:36 +0100
Subject: [PATCH 4/8] clippy

---
 experimental/harfbuzz/src/lib.rs | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/experimental/harfbuzz/src/lib.rs b/experimental/harfbuzz/src/lib.rs
index 6bcd221d0f4..d81fbbd3220 100644
--- a/experimental/harfbuzz/src/lib.rs
+++ b/experimental/harfbuzz/src/lib.rs
@@ -202,7 +202,6 @@ impl UnicodeFuncs {
             hb_unicode_funcs_set_general_category_func(
                 ufuncs.raw,
                 Some({
-                    #[allow(clippy::indexing_slicing)]
                     extern "C" fn cb(
                         _: *mut hb_unicode_funcs_t,
                         unicode: hb_codepoint_t,
@@ -213,6 +212,7 @@ impl UnicodeFuncs {
                         // values (which would be UB to materialize). `GeneralCategory` is a
                         // stable exhaustive enum, and the length of `ICU4X_GENERAL_CATEGORY_TO_HARFBUZZ`
                         // matches the number of enum items, so the index will always be in range here.
+                        #![allow(clippy::indexing_slicing)]
                         ICU4X_GENERAL_CATEGORY_TO_HARFBUZZ
                             [maps::general_category().get32(unicode) as usize]
                             as hb_unicode_general_category_t
@@ -447,7 +447,6 @@ impl UnicodeFuncs {
             hb_unicode_funcs_set_general_category_func(
                 ufuncs.raw,
                 Some({
-                    #[allow(clippy::indexing_slicing)]
                     extern "C" fn cb(
                         _: *mut hb_unicode_funcs_t,
                         unicode: hb_codepoint_t,
@@ -458,6 +457,7 @@ impl UnicodeFuncs {
                         // values (which would be UB to materialize). `GeneralCategory` is a
                         // stable exhaustive enum, and the length of `ICU4X_GENERAL_CATEGORY_TO_HARFBUZZ`
                         // matches the number of enum items, so the index will always be in range here.
+                        #![allow(clippy::indexing_slicing)]
                         ICU4X_GENERAL_CATEGORY_TO_HARFBUZZ[unsafe {
                             &*(user_data as *mut CodePointMapData<GeneralCategory>)
                         }

From 58c05c75e3da768e7f4e40c68794f6311970bd64 Mon Sep 17 00:00:00 2001
From: Robert Bastian <4706271+robertbastian@users.noreply.github.com>
Date: Tue, 27 Feb 2024 15:20:45 +0100
Subject: [PATCH 5/8] j

---
 experimental/harfbuzz/src/lib.rs | 58 ++++++++++++--------------------
 1 file changed, 22 insertions(+), 36 deletions(-)

diff --git a/experimental/harfbuzz/src/lib.rs b/experimental/harfbuzz/src/lib.rs
index d81fbbd3220..a93359b5612 100644
--- a/experimental/harfbuzz/src/lib.rs
+++ b/experimental/harfbuzz/src/lib.rs
@@ -299,28 +299,23 @@ impl UnicodeFuncs {
                         // If we treated `hb_buffer_add_codepoints` as conceptually
                         // `unsafe`, it would be appropriate not to do scalar value
                         // validation here.
-                        let first = if let Some(first) = core::char::from_u32(a) {
-                            first
-                        } else {
+                        let Some(first) = char::from_u32(a) else {
                             // GIGO case
                             debug_assert!(false);
                             return false as hb_bool_t;
                         };
-                        let second = if let Some(second) = core::char::from_u32(b) {
-                            second
-                        } else {
+                        let Some(second) = char::from_u32(b) else {
                             // GIGO case
                             debug_assert!(false);
                             return false as hb_bool_t;
                         };
-                        if let Some(c) = CanonicalComposition::new().compose(first, second) {
-                            unsafe {
-                                core::ptr::write(ab, c as hb_codepoint_t);
-                            }
-                            true as hb_bool_t
-                        } else {
-                            false as hb_bool_t
+                        let Some(c) = CanonicalComposition::new().compose(first, second) else {
+                            return false as hb_bool_t;
+                        };
+                        unsafe {
+                            core::ptr::write(ab, c as hb_codepoint_t);
                         }
+                        true as hb_bool_t
                     }
                     cb
                 }),
@@ -346,9 +341,7 @@ impl UnicodeFuncs {
                         // If we treated `hb_buffer_add_codepoints` as conceptually
                         // `unsafe`, it would be appropriate not to do scalar value
                         // validation here.
-                        let composed = if let Some(composed) = core::char::from_u32(ab) {
-                            composed
-                        } else {
+                        let Some(composed) = char::from_u32(ab) else {
                             // GIGO case
                             debug_assert!(false);
                             return false as hb_bool_t;
@@ -392,7 +385,7 @@ impl UnicodeFuncs {
         Ok(ufuncs)
     }
 
-    #[doc = icu_provider::gen_any_buffer_unstable_docs!(UNSTABLE, new_hb_unicode_funcs)]
+    #[doc = icu_provider::gen_any_buffer_unstable_docs!(UNSTABLE, Self::new)]
     pub fn new_unstable<D>(provider: &D) -> Result<UnicodeFuncs, HarfBuzzError>
     where
         D: DataProvider<BidiAuxiliaryPropertiesV1Marker>
@@ -405,8 +398,6 @@ impl UnicodeFuncs {
             + DataProvider<ScriptV1Marker>
             + ?Sized,
     {
-        let ufuncs = Self::empty()?;
-
         let canonical_combining_class_map = CanonicalCombiningClassMap::try_new_unstable(provider)?;
         let general_category_map = maps::load_general_category(provider)?;
         let bidi_auxiliary_props_map =
@@ -416,6 +407,8 @@ impl UnicodeFuncs {
         let canonical_composition = CanonicalComposition::try_new_unstable(provider)?;
         let canonical_decomposition = CanonicalDecomposition::try_new_unstable(provider)?;
 
+        let ufuncs = Self::empty()?;
+
         unsafe {
             hb_unicode_funcs_set_combining_class_func(
                 ufuncs.raw,
@@ -578,30 +571,25 @@ impl UnicodeFuncs {
                         // If we treated `hb_buffer_add_codepoints` as conceptually
                         // `unsafe`, it would be appropriate not to do scalar value
                         // validation here.
-                        let first = if let Some(first) = core::char::from_u32(a) {
-                            first
-                        } else {
+                        let Some(first) = char::from_u32(a) else {
                             // GIGO case
                             debug_assert!(false);
                             return false as hb_bool_t;
                         };
-                        let second = if let Some(second) = core::char::from_u32(b) {
-                            second
-                        } else {
+                        let Some(second) = char::from_u32(b) else {
                             // GIGO case
                             debug_assert!(false);
                             return false as hb_bool_t;
                         };
-                        if let Some(c) = unsafe { &*(user_data as *mut CanonicalComposition) }
+                        let Some(c) = unsafe { &*(user_data as *mut CanonicalComposition) }
                             .compose(first, second)
-                        {
-                            unsafe {
-                                core::ptr::write(ab, c as hb_codepoint_t);
-                            }
-                            true as hb_bool_t
-                        } else {
-                            false as hb_bool_t
+                        else {
+                            return false as hb_bool_t;
+                        };
+                        unsafe {
+                            core::ptr::write(ab, c as hb_codepoint_t);
                         }
+                        true as hb_bool_t
                     }
                     cb
                 }),
@@ -632,9 +620,7 @@ impl UnicodeFuncs {
                         // If we treated `hb_buffer_add_codepoints` as conceptually
                         // `unsafe`, it would be appropriate not to do scalar value
                         // validation here.
-                        let composed = if let Some(composed) = core::char::from_u32(ab) {
-                            composed
-                        } else {
+                        let Some(composed) = char::from_u32(ab) else {
                             // GIGO case
                             debug_assert!(false);
                             return false as hb_bool_t;

From af3611ecf85d1100d06e8374705f8f2dc2b4ee9d Mon Sep 17 00:00:00 2001
From: Robert Bastian <4706271+robertbastian@users.noreply.github.com>
Date: Tue, 27 Feb 2024 15:25:03 +0100
Subject: [PATCH 6/8] tidy

---
 experimental/harfbuzz/README.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/experimental/harfbuzz/README.md b/experimental/harfbuzz/README.md
index 8cdab6e5369..f005f1209a2 100644
--- a/experimental/harfbuzz/README.md
+++ b/experimental/harfbuzz/README.md
@@ -11,7 +11,7 @@ use harfbuzz::{Buffer, Direction, sys};
 
 let mut b = Buffer::with("مساء الخير");
 
-let unicode_funcs = icu_harfbuzz::new_hb_unicode_funcs().unwrap();
+let unicode_funcs = icu_harfbuzz::UnicodeFuncs::new().unwrap();
 
 // NOTE: This currently requires `unsafe` code. For progress toward a safe abstraction, see:
 // <https://github.com/servo/rust-harfbuzz/pull/197>

From 088376739d9f7b1cf78e65707542c994d47a2a45 Mon Sep 17 00:00:00 2001
From: Robert Bastian <4706271+robertbastian@users.noreply.github.com>
Date: Tue, 27 Feb 2024 17:04:39 +0100
Subject: [PATCH 7/8] pointer types

---
 experimental/harfbuzz/src/lib.rs | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/experimental/harfbuzz/src/lib.rs b/experimental/harfbuzz/src/lib.rs
index a93359b5612..2202fbeba98 100644
--- a/experimental/harfbuzz/src/lib.rs
+++ b/experimental/harfbuzz/src/lib.rs
@@ -418,7 +418,7 @@ impl UnicodeFuncs {
                         unicode: hb_codepoint_t,
                         user_data: *mut c_void,
                     ) -> hb_unicode_combining_class_t {
-                        unsafe { &*(user_data as *mut CanonicalCombiningClassMap) }
+                        unsafe { &*(user_data as *const CanonicalCombiningClassMap) }
                             .get32(unicode)
                             .0 as hb_unicode_combining_class_t
                     }
@@ -452,7 +452,7 @@ impl UnicodeFuncs {
                         // matches the number of enum items, so the index will always be in range here.
                         #![allow(clippy::indexing_slicing)]
                         ICU4X_GENERAL_CATEGORY_TO_HARFBUZZ[unsafe {
-                            &*(user_data as *mut CodePointMapData<GeneralCategory>)
+                            &*(user_data as *const CodePointMapData<GeneralCategory>)
                         }
                         .as_borrowed()
                         .get32(unicode)
@@ -489,7 +489,7 @@ impl UnicodeFuncs {
                         unicode: hb_codepoint_t,
                         user_data: *mut c_void,
                     ) -> hb_codepoint_t {
-                        unsafe { &*(user_data as *mut BidiAuxiliaryProperties) }
+                        unsafe { &*(user_data as *const BidiAuxiliaryProperties) }
                             .as_borrowed()
                             .get32_mirroring_props(unicode)
                             .mirroring_glyph
@@ -522,11 +522,11 @@ impl UnicodeFuncs {
                         unicode: hb_codepoint_t,
                         user_data: *mut c_void,
                     ) -> hb_script_t {
-                        let script = unsafe { &*(user_data as *mut ScriptDataForHarfBuzz) }
+                        let script = unsafe { &*(user_data as *const ScriptDataForHarfBuzz) }
                             .script_map
                             .as_borrowed()
                             .get32(unicode);
-                        let name = unsafe { &*(user_data as *mut ScriptDataForHarfBuzz) }
+                        let name = unsafe { &*(user_data as *const ScriptDataForHarfBuzz) }
                             .enum_to_name_mapper
                             .as_borrowed()
                             .get(script)
@@ -581,7 +581,7 @@ impl UnicodeFuncs {
                             debug_assert!(false);
                             return false as hb_bool_t;
                         };
-                        let Some(c) = unsafe { &*(user_data as *mut CanonicalComposition) }
+                        let Some(c) = unsafe { &*(user_data as *const CanonicalComposition) }
                             .compose(first, second)
                         else {
                             return false as hb_bool_t;
@@ -625,7 +625,7 @@ impl UnicodeFuncs {
                             debug_assert!(false);
                             return false as hb_bool_t;
                         };
-                        match unsafe { &*(user_data as *mut CanonicalDecomposition) }
+                        match unsafe { &*(user_data as *const CanonicalDecomposition) }
                             .decompose(composed)
                         {
                             Decomposed::Default => false as hb_bool_t,

From bbe3736960452608b35ebd7dbd947b02924ccec3 Mon Sep 17 00:00:00 2001
From: Robert Bastian <4706271+robertbastian@users.noreply.github.com>
Date: Tue, 27 Feb 2024 19:25:02 +0100
Subject: [PATCH 8/8] Update CHANGELOG.md

---
 CHANGELOG.md | 1 +
 1 file changed, 1 insertion(+)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 5bf9d475f1c..a5c10fff2e1 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -11,6 +11,7 @@
     - `FormattedDateTime` and `FormattedZonedDateTime` now implement `Clone` and `Copy` (https://github.com/unicode-org/icu4x/pull/4476)
   - `icu_properties`
     - Add `Aran` script code (https://github.com/unicode-org/icu4x/pull/4426)
+    - Made additional constructors `const` (https://github.com/unicode-org/icu4x/pull/4574)
   - `icu_segmenter`
     - Fix Unicode 15.0 line breaking (https://github.com/unicode-org/icu4x/pull/4389)
 - Data model and providers