# HG changeset patch # User Raphaël Gomès # Date 1686606088 -7200 # Node ID 2cc5de261d765345b2320712b1aa2649c1bbbe50 # Parent 47b44d80d836e43c6810d92164286a8499435582 rust-hg-core: move from `ouroboros` to `self_cell` `ouroboros` has a fundamental soundness problem that, while not applicable today, could become applicable given new compiler optimizations.¹ `self_cell` is a crate that accomplishes a lot of the same things that `ouroboros` did while remaining sound (that is, unless a new soundness issue is discovered) by not assuming as much about the memory layout of the program. `self_cell` has been scrutinized heavily in the past few months by very competent people, some from the compiler team and has shown no weaknesses for a while, with a 1.0 stable release coming out a couple months ago. Our internal API is exactly the same, this is just an implementation detail. To reiterate, no actual soundness issue was found with our use of `ouroboros`, but there might be evolutions of `rustc` (or even a future separate compiler) that could generate unsound code. [1] https://github.com/joshua-maros/ouroboros/issues/88 diff -r 47b44d80d836 -r 2cc5de261d76 rust/Cargo.lock --- a/rust/Cargo.lock Fri Jun 02 15:12:05 2023 +0200 +++ b/rust/Cargo.lock Mon Jun 12 23:41:28 2023 +0200 @@ -3,12 +3,6 @@ version = 3 [[package]] -name = "Inflector" -version = "0.11.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "fe438c63458706e03479442743baae6c88256498e6431708f6dfc520a26515d3" - -[[package]] name = "adler" version = "1.0.2" source = "registry+https://github.com/rust-lang/crates.io-index" @@ -35,12 +29,6 @@ ] [[package]] -name = "aliasable" -version = "0.1.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "250f629c0161ad8107cf89319e990051fae62832fd343083bea452d93e2205fd" - -[[package]] name = "android_system_properties" version = "0.1.5" source = "registry+https://github.com/rust-lang/crates.io-index" @@ -539,7 +527,6 @@ "logging_timer", "memmap2", "once_cell", - "ouroboros", "pretty_assertions", "rand 0.8.5", "rand_distr", @@ -547,6 +534,7 @@ "rayon", "regex", "same-file", + "self_cell", "sha-1 0.10.0", "tempfile", "thread_local", @@ -809,29 +797,6 @@ checksum = "7b5bf27447411e9ee3ff51186bf7a08e16c341efdde93f4d823e8844429bed7e" [[package]] -name = "ouroboros" -version = "0.15.5" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "dfbb50b356159620db6ac971c6d5c9ab788c9cc38a6f49619fca2a27acb062ca" -dependencies = [ - "aliasable", - "ouroboros_macro", -] - -[[package]] -name = "ouroboros_macro" -version = "0.15.5" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4a0d9d1a6191c4f391f87219d1ea42b23f09ee84d64763cd05ee6ea88d9f384d" -dependencies = [ - "Inflector", - "proc-macro-error", - "proc-macro2", - "quote", - "syn", -] - -[[package]] name = "output_vt100" version = "0.1.3" source = "registry+https://github.com/rust-lang/crates.io-index" @@ -1130,6 +1095,12 @@ checksum = "9c8132065adcfd6e02db789d9285a0deb2f3fcb04002865ab67d5fb103533898" [[package]] +name = "self_cell" +version = "1.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4a3926e239738d36060909ffe6f511502f92149a45a1fade7fe031cb2d33e88b" + +[[package]] name = "semver" version = "1.0.14" source = "registry+https://github.com/rust-lang/crates.io-index" diff -r 47b44d80d836 -r 2cc5de261d76 rust/hg-core/Cargo.toml --- a/rust/hg-core/Cargo.toml Fri Jun 02 15:12:05 2023 +0200 +++ b/rust/hg-core/Cargo.toml Mon Jun 12 23:41:28 2023 +0200 @@ -20,12 +20,12 @@ lazy_static = "1.4.0" libc = "0.2.137" logging_timer = "1.1.0" -ouroboros = "0.15.5" rand = "0.8.5" rand_pcg = "0.3.1" rand_distr = "0.4.3" rayon = "1.7.0" regex = "1.7.0" +self_cell = "1.0" sha-1 = "0.10.0" twox-hash = "1.6.3" same-file = "1.0.6" diff -r 47b44d80d836 -r 2cc5de261d76 rust/hg-core/src/dirstate_tree/owning.rs --- a/rust/hg-core/src/dirstate_tree/owning.rs Fri Jun 02 15:12:05 2023 +0200 +++ b/rust/hg-core/src/dirstate_tree/owning.rs Mon Jun 12 23:41:28 2023 +0200 @@ -1,19 +1,18 @@ use crate::{DirstateError, DirstateParents}; use super::dirstate_map::DirstateMap; +use self_cell::self_cell; use std::ops::Deref; -use ouroboros::self_referencing; - -/// Keep a `DirstateMap<'on_disk>` next to the `on_disk` buffer that it -/// borrows. -#[self_referencing] -pub struct OwningDirstateMap { - on_disk: Box + Send>, - #[borrows(on_disk)] - #[covariant] - map: DirstateMap<'this>, -} +self_cell!( + /// Keep a `DirstateMap<'owner>` next to the `owner` buffer that it + /// borrows. + pub struct OwningDirstateMap { + owner: Box + Send>, + #[covariant] + dependent: DirstateMap, + } +); impl OwningDirstateMap { pub fn new_empty(on_disk: OnDisk) -> Self @@ -22,11 +21,7 @@ { let on_disk = Box::new(on_disk); - OwningDirstateMapBuilder { - on_disk, - map_builder: |bytes| DirstateMap::empty(bytes), - } - .build() + OwningDirstateMap::new(on_disk, |bytes| DirstateMap::empty(bytes)) } pub fn new_v1( @@ -40,16 +35,12 @@ let mut parents = DirstateParents::NULL; Ok(( - OwningDirstateMapTryBuilder { - on_disk, - map_builder: |bytes| { - DirstateMap::new_v1(bytes, identity).map(|(dmap, p)| { - parents = p.unwrap_or(DirstateParents::NULL); - dmap - }) - }, - } - .try_build()?, + OwningDirstateMap::try_new(on_disk, |bytes| { + DirstateMap::new_v1(bytes, identity).map(|(dmap, p)| { + parents = p.unwrap_or(DirstateParents::NULL); + dmap + }) + })?, parents, )) } @@ -66,28 +57,24 @@ { let on_disk = Box::new(on_disk); - OwningDirstateMapTryBuilder { - on_disk, - map_builder: |bytes| { - DirstateMap::new_v2(bytes, data_size, metadata, uuid, identity) - }, - } - .try_build() + OwningDirstateMap::try_new(on_disk, |bytes| { + DirstateMap::new_v2(bytes, data_size, metadata, uuid, identity) + }) } pub fn with_dmap_mut( &mut self, f: impl FnOnce(&mut DirstateMap) -> R, ) -> R { - self.with_map_mut(f) + self.with_dependent_mut(|_owner, dmap| f(dmap)) } pub fn get_map(&self) -> &DirstateMap { - self.borrow_map() + self.borrow_dependent() } pub fn on_disk(&self) -> &[u8] { - self.borrow_on_disk() + self.borrow_owner() } pub fn old_uuid(&self) -> Option<&[u8]> {