Merge pull request pop-os#188 from EHfive/use_self_cell

Use self_cell for creating self-referential struct

Author: jackpot51

Cargo.toml

@@ -13,7 +13,6 @@ rust-version = "1.65"
 fontdb = { version = "0.14.1", default-features = false }
 libm = "0.2.7"
 log = "0.4.20"
-aliasable = "0.1.3"
 rustybuzz = { version = "0.10.0", default-features = false, features = ["libm"] }
 swash = { version = "0.1.8", optional = true }
 syntect = { version = "5.1.0", optional = true }
@@ -24,6 +23,7 @@ unicode-segmentation = "1.10.1"
 rangemap = "1.3.0"
 hashbrown = { version = "0.14.0", optional = true, default-features = false }
 rustc-hash = { version = "1.1.0", default-features = false }
+self_cell = "1.0.1"
 
 [dependencies.unicode-bidi]
 version = "0.3.13"

src/font/mod.rs

@@ -1,116 +1,68 @@
 // SPDX-License-Identifier: MIT OR Apache-2.0
 pub(crate) mod fallback;
 
-use alloc::boxed::Box;
+use core::fmt;
+
 use alloc::sync::Arc;
 
+use rustybuzz::Face as RustybuzzFace;
+use self_cell::self_cell;
+
 pub use self::system::*;
 mod system;
 
-pub use font_inner::Font;
-
-/// Encapsulates the self-referencing `Font` struct to ensure all field accesses have to go through
-/// safe methods.
-mod font_inner {
-    use super::*;
-    use aliasable::boxed::AliasableBox;
-    use core::fmt;
+self_cell!(
+    struct OwnedFace {
+        owner: Arc<dyn AsRef<[u8]> + Send + Sync>,
 
-    /// A font
-    //
-    // # Safety invariant
-    //
-    // `data` must never have a mutable reference taken, nor be modified during the lifetime of
-    // this `Font`.
-    pub struct Font {
-        #[cfg(feature = "swash")]
-        swash: (u32, swash::CacheKey),
-        rustybuzz: rustybuzz::Face<'static>,
-        // Note: This field must be after rustybuzz to ensure that it is dropped later. Otherwise
-        // there would be a dangling reference when dropping rustybuzz.
-        data: aliasable::boxed::AliasableBox<Arc<dyn AsRef<[u8]> + Send + Sync>>,
-        id: fontdb::ID,
+        #[covariant]
+        dependent: RustybuzzFace,
     }
+);
 
-    impl fmt::Debug for Font {
-        fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-            f.debug_struct("Font")
-                .field("id", &self.id)
-                .finish_non_exhaustive()
-        }
-    }
+/// A font
+pub struct Font {
+    #[cfg(feature = "swash")]
+    swash: (u32, swash::CacheKey),
+    rustybuzz: OwnedFace,
+    data: Arc<dyn AsRef<[u8]> + Send + Sync>,
+    id: fontdb::ID,
+}
 
-    pub(super) struct FontTryBuilder<
-        RustybuzzBuilder: for<'this> FnOnce(
-            &'this Arc<dyn AsRef<[u8]> + Send + Sync>,
-        ) -> Option<rustybuzz::Face<'this>>,
-    > {
-        pub(super) id: fontdb::ID,
-        pub(super) data: Arc<dyn AsRef<[u8]> + Send + Sync>,
-        pub(super) rustybuzz_builder: RustybuzzBuilder,
-        #[cfg(feature = "swash")]
-        pub(super) swash: (u32, swash::CacheKey),
+impl fmt::Debug for Font {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        f.debug_struct("Font")
+            .field("id", &self.id)
+            .finish_non_exhaustive()
     }
-    impl<
-            RustybuzzBuilder: for<'this> FnOnce(
-                &'this Arc<dyn AsRef<[u8]> + Send + Sync>,
-            ) -> Option<rustybuzz::Face<'this>>,
-        > FontTryBuilder<RustybuzzBuilder>
-    {
-        pub(super) fn try_build(self) -> Option<Font> {
-            unsafe fn change_lifetime<'old, 'new: 'old, T: 'new>(data: &'old T) -> &'new T {
-                &*(data as *const _)
-            }
-
-            let data: AliasableBox<Arc<dyn AsRef<[u8]> + Send + Sync>> =
-                AliasableBox::from_unique(Box::new(self.data));
-
-            // Safety: We use AliasableBox to allow the references in rustybuzz::Face to alias with
-            // the data stored behind the AliasableBox. In addition the entire public interface of
-            // Font ensures that no mutable reference is given to data. And finally we use
-            // for<'this> for the rustybuzz_builder to ensure it can't leak a reference. Combined
-            // this ensures that it is sound to produce a self-referential type.
-            let rustybuzz = (self.rustybuzz_builder)(unsafe { change_lifetime(&*data) })?;
+}
 
-            Some(Font {
-                id: self.id,
-                data,
-                rustybuzz,
-                #[cfg(feature = "swash")]
-                swash: self.swash,
-            })
-        }
+impl Font {
+    pub fn id(&self) -> fontdb::ID {
+        self.id
     }
 
-    impl Font {
-        pub fn id(&self) -> fontdb::ID {
-            self.id
-        }
-
-        pub fn data(&self) -> &[u8] {
-            // Safety: This only gives an immutable access to `data`.
-            (**self.data).as_ref()
-        }
+    pub fn data(&self) -> &[u8] {
+        (*self.data).as_ref()
+    }
 
-        pub fn rustybuzz(&self) -> &rustybuzz::Face<'_> {
-            &self.rustybuzz
-        }
+    pub fn rustybuzz(&self) -> &RustybuzzFace<'_> {
+        self.rustybuzz.borrow_dependent()
+    }
 
-        #[cfg(feature = "swash")]
-        pub fn as_swash(&self) -> swash::FontRef<'_> {
-            let swash = &self.swash;
-            swash::FontRef {
-                data: self.data(),
-                offset: swash.0,
-                key: swash.1,
-            }
+    #[cfg(feature = "swash")]
+    pub fn as_swash(&self) -> swash::FontRef<'_> {
+        let swash = &self.swash;
+        swash::FontRef {
+            data: self.data(),
+            offset: swash.0,
+            key: swash.1,
         }
     }
 }
 
 impl Font {
     pub fn new(info: &fontdb::FaceInfo) -> Option<Self> {
-        #[allow(unused_variables)]
         let data = match &info.source {
             fontdb::Source::Binary(data) => Arc::clone(data),
             #[cfg(feature = "std")]
@@ -121,16 +73,19 @@ impl Font {
             #[cfg(feature = "std")]
             fontdb::Source::SharedFile(_path, data) => Arc::clone(data),
         };
-        font_inner::FontTryBuilder {
+
+        Some(Self {
             id: info.id,
             #[cfg(feature = "swash")]
             swash: {
                 let swash = swash::FontRef::from_index((*data).as_ref(), info.index as usize)?;
                 (swash.offset, swash.key)
             },
+            rustybuzz: OwnedFace::try_new(Arc::clone(&data), |data| {
+                RustybuzzFace::from_slice((**data).as_ref(), info.index).ok_or(())
+            })
+            .ok()?,
             data,
-            rustybuzz_builder: |data| rustybuzz::Face::from_slice((**data).as_ref(), info.index),
-        }
-        .try_build()
+        })
     }
 }