rust-hg-core: move from `ouroboros` to `self_cell`
authorRaphaël Gomès <rgomes@octobus.net>
Mon, 12 Jun 2023 23:41:28 +0200
changeset 50682 2cc5de261d76
parent 50681 47b44d80d836
child 50683 d39ac3468ad4
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
rust/Cargo.lock
rust/hg-core/Cargo.toml
rust/hg-core/src/dirstate_tree/owning.rs
--- 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"
--- 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"
--- 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<dyn Deref<Target = [u8]> + 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<dyn Deref<Target = [u8]> + Send>,
+        #[covariant]
+        dependent: DirstateMap,
+    }
+);
 
 impl OwningDirstateMap {
     pub fn new_empty<OnDisk>(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<OnDisk>(
@@ -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<R>(
         &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]> {