Auto merge of #109971 - WaffleLapkin:yeet_ownership, r=Nilstrieb

Yeet `owning_ref`

Based on the discussions from #109948

This replaces `owning_ref` with a far simpler & safer abstraction.

Fixes #109974

Author: bors

compiler/rustc_codegen_ssa/src/back/metadata.rs

@@ -13,8 +13,7 @@ use object::{
 use snap::write::FrameEncoder;
 
 use rustc_data_structures::memmap::Mmap;
-use rustc_data_structures::owning_ref::OwningRef;
-use rustc_data_structures::rustc_erase_owner;
+use rustc_data_structures::owned_slice::try_slice_owned;
 use rustc_data_structures::sync::MetadataRef;
 use rustc_metadata::fs::METADATA_FILENAME;
 use rustc_metadata::EncodedMetadata;
@@ -42,10 +41,10 @@ fn load_metadata_with(
 ) -> Result<MetadataRef, String> {
     let file =
         File::open(path).map_err(|e| format!("failed to open file '{}': {}", path.display(), e))?;
-    let data = unsafe { Mmap::map(file) }
-        .map_err(|e| format!("failed to mmap file '{}': {}", path.display(), e))?;
-    let metadata = OwningRef::new(data).try_map(f)?;
-    return Ok(rustc_erase_owner!(metadata.map_owner_box()));
+
+    unsafe { Mmap::map(file) }
+        .map_err(|e| format!("failed to mmap file '{}': {}", path.display(), e))
+        .and_then(|mmap| try_slice_owned(mmap, |mmap| f(mmap)))
 }
 
 impl MetadataLoader for DefaultMetadataLoader {

compiler/rustc_data_structures/src/lib.rs

@@ -1,5 +1,5 @@
 //! Various data structures used by the Rust compiler. The intention
-//! is that code in here should be not be *specific* to rustc, so that
+//! is that code in here should not be *specific* to rustc, so that
 //! it can be easily unit tested and so forth.
 //!
 //! # Note
@@ -27,6 +27,8 @@
 #![feature(thread_id_value)]
 #![feature(vec_into_raw_parts)]
 #![feature(get_mut_unchecked)]
+#![feature(lint_reasons)]
+#![feature(unwrap_infallible)]
 #![allow(rustc::default_hash_types)]
 #![allow(rustc::potential_query_instability)]
 #![deny(rustc::untranslatable_diagnostic)]
@@ -59,7 +61,6 @@ pub mod intern;
 pub mod jobserver;
 pub mod macros;
 pub mod obligation_forest;
-pub mod owning_ref;
 pub mod sip128;
 pub mod small_c_str;
 pub mod small_str;
@@ -82,6 +83,7 @@ pub mod vec_linked_list;
 pub mod work_queue;
 pub use atomic_ref::AtomicRef;
 pub mod frozen;
+pub mod owned_slice;
 pub mod sso;
 pub mod steal;
 pub mod tagged_ptr;

compiler/rustc_data_structures/src/memmap.rs

@@ -2,9 +2,7 @@ use std::fs::File;
 use std::io;
 use std::ops::{Deref, DerefMut};
 
-use crate::owning_ref::StableAddress;
-
-/// A trivial wrapper for [`memmap2::Mmap`] that implements [`StableAddress`].
+/// A trivial wrapper for [`memmap2::Mmap`] (or `Vec<u8>` on WASM).
 #[cfg(not(target_arch = "wasm32"))]
 pub struct Mmap(memmap2::Mmap);
 
@@ -46,12 +44,6 @@ impl AsRef<[u8]> for Mmap {
     }
 }
 
-// SAFETY: On architectures other than WASM, mmap is used as backing storage. The address of this
-// memory map is stable. On WASM, `Vec<u8>` is used as backing storage. The `Mmap` type doesn't
-// export any function that can cause the `Vec` to be re-allocated. As such the address of the
-// bytes inside this `Vec` is stable.
-unsafe impl StableAddress for Mmap {}
-
 #[cfg(not(target_arch = "wasm32"))]
 pub struct MmapMut(memmap2::MmapMut);
 

compiler/rustc_data_structures/src/owned_slice.rs

@@ -0,0 +1,118 @@
+use std::{borrow::Borrow, ops::Deref};
+
+// Use our fake Send/Sync traits when on not parallel compiler,
+// so that `OwnedSlice` only implements/requires Send/Sync
+// for parallel compiler builds.
+use crate::sync::{Send, Sync};
+
+/// An owned slice.
+///
+/// This is similar to `Box<[u8]>` but allows slicing and using anything as the
+/// backing buffer.
+///
+/// See [`slice_owned`] for `OwnedSlice` construction and examples.
+///
+/// ---------------------------------------------------------------------------
+///
+/// This is essentially a replacement for `owning_ref` which is a lot simpler
+/// and even sound! 🌸
+pub struct OwnedSlice {
+    /// This is conceptually a `&'self.owner [u8]`.
+    bytes: *const [u8],
+
+    // +---------------------------------------+
+    // | We expect `dead_code` lint here,      |
+    // | because we don't want to accidentally |
+    // | touch the owner — otherwise the owner |
+    // | could invalidate out `bytes` pointer  |
+    // |                                       |
+    // | so be quiet                           |
+    // +----+  +-------------------------------+
+    //       \/
+    //      ⊂(´･◡･⊂ )∘˚˳° (I am the phantom remnant of #97770)
+    #[expect(dead_code)]
+    owner: Box<dyn Send + Sync>,
+}
+
+/// Makes an [`OwnedSlice`] out of an `owner` and a `slicer` function.
+///
+/// ## Examples
+///
+/// ```rust
+/// # use rustc_data_structures::owned_slice::{OwnedSlice, slice_owned};
+/// let vec = vec![1, 2, 3, 4];
+///
+/// // Identical to slicing via `&v[1..3]` but produces an owned slice
+/// let slice: OwnedSlice = slice_owned(vec, |v| &v[1..3]);
+/// assert_eq!(&*slice, [2, 3]);
+/// ```
+///
+/// ```rust
+/// # use rustc_data_structures::owned_slice::{OwnedSlice, slice_owned};
+/// # use std::ops::Deref;
+/// let vec = vec![1, 2, 3, 4];
+///
+/// // Identical to slicing via `&v[..]` but produces an owned slice
+/// let slice: OwnedSlice = slice_owned(vec, Deref::deref);
+/// assert_eq!(&*slice, [1, 2, 3, 4]);
+/// ```
+pub fn slice_owned<O, F>(owner: O, slicer: F) -> OwnedSlice
+where
+    O: Send + Sync + 'static,
+    F: FnOnce(&O) -> &[u8],
+{
+    try_slice_owned(owner, |x| Ok::<_, !>(slicer(x))).into_ok()
+}
+
+/// Makes an [`OwnedSlice`] out of an `owner` and a `slicer` function that can fail.
+///
+/// See [`slice_owned`] for the infallible version.
+pub fn try_slice_owned<O, F, E>(owner: O, slicer: F) -> Result<OwnedSlice, E>
+where
+    O: Send + Sync + 'static,
+    F: FnOnce(&O) -> Result<&[u8], E>,
+{
+    // We box the owner of the bytes, so it doesn't move.
+    //
+    // Since the owner does not move and we don't access it in any way
+    // before drop, there is nothing that can invalidate the bytes pointer.
+    //
+    // Thus, "extending" the lifetime of the reference returned from `F` is fine.
+    // We pretend that we pass it a reference that lives as long as the returned slice.
+    //
+    // N.B. the HRTB on the `slicer` is important — without it the caller could provide
+    // a short lived slice, unrelated to the owner.
+
+    let owner = Box::new(owner);
+    let bytes = slicer(&*owner)?;
+
+    Ok(OwnedSlice { bytes, owner })
+}
+
+impl Deref for OwnedSlice {
+    type Target = [u8];
+
+    #[inline]
+    fn deref(&self) -> &[u8] {
+        // Safety:
+        // `self.bytes` is valid per the construction in `slice_owned`
+        // (which is the only constructor)
+        unsafe { &*self.bytes }
+    }
+}
+
+impl Borrow<[u8]> for OwnedSlice {
+    #[inline]
+    fn borrow(&self) -> &[u8] {
+        self
+    }
+}
+
+// Safety: `OwnedSlice` is conceptually `(&'self.1 [u8], Box<dyn Send + Sync>)`, which is `Send`
+unsafe impl Send for OwnedSlice {}
+
+// Safety: `OwnedSlice` is conceptually `(&'self.1 [u8], Box<dyn Send + Sync>)`, which is `Sync`
+unsafe impl Sync for OwnedSlice {}
+
+#[cfg(test)]
+mod tests;

compiler/rustc_data_structures/src/owned_slice/tests.rs

@@ -0,0 +1,74 @@
+use std::{
+    ops::Deref,
+    sync::{
+        atomic::{self, AtomicBool},
+        Arc,
+    },
+};
+
+use crate::{
+    owned_slice::{slice_owned, try_slice_owned, OwnedSlice},
+    OnDrop,
+};
+
+#[test]
+fn smoke() {
+    let slice = slice_owned(vec![1, 2, 3, 4, 5, 6], Vec::as_slice);
+
+    assert_eq!(&*slice, [1, 2, 3, 4, 5, 6]);
+}
+
+#[test]
+fn static_storage() {
+    let slice = slice_owned(Box::new(String::from("what")), |_| b"bytes boo");
+
+    assert_eq!(&*slice, b"bytes boo");
+}
+
+#[test]
+fn slice_the_slice() {
+    let slice = slice_owned(vec![1, 2, 3, 4, 5, 6], Vec::as_slice);
+    let slice = slice_owned(slice, |s| &s[1..][..4]);
+    let slice = slice_owned(slice, |s| s);
+    let slice = slice_owned(slice, |s| &s[1..]);
+
+    assert_eq!(&*slice, &[1, 2, 3, 4, 5, 6][1..][..4][1..]);
+}
+
+#[test]
+fn try_and_fail() {
+    let res = try_slice_owned(vec![0], |v| v.get(12..).ok_or(()));
+
+    assert!(res.is_err());
+}
+
+#[test]
+fn boxed() {
+    // It's important that we don't cause UB because of `Box`'es uniqueness
+
+    let boxed: Box<[u8]> = vec![1, 1, 2, 3, 5, 8, 13, 21].into_boxed_slice();
+    let slice = slice_owned(boxed, Deref::deref);
+
+    assert_eq!(&*slice, [1, 1, 2, 3, 5, 8, 13, 21]);
+}
+
+#[test]
+fn drop_drops() {
+    let flag = Arc::new(AtomicBool::new(false));
+    let flag_prime = Arc::clone(&flag);
+    let d = OnDrop(move || flag_prime.store(true, atomic::Ordering::Relaxed));
+
+    let slice = slice_owned(d, |_| &[]);
+
+    assert_eq!(flag.load(atomic::Ordering::Relaxed), false);
+
+    drop(slice);
+
+    assert_eq!(flag.load(atomic::Ordering::Relaxed), true);
+}
+
+#[test]
+fn send_sync() {
+    crate::sync::assert_send::<OwnedSlice>();
+    crate::sync::assert_sync::<OwnedSlice>();
+}

compiler/rustc_data_structures/src/owning_ref/LICENSE

@@ -7,9 +7,6 @@
 //! while the serial versions degenerate straightforwardly to serial execution.
 //! The operations include `join`, `parallel`, `par_iter`, and `par_for_each`.
 //!
-//! `rustc_erase_owner!` erases an `OwningRef` owner into `Erased` for the
-//! serial version and `Erased + Send + Sync` for the parallel version.
-//!
 //! Types
 //! -----
 //! The parallel versions of types provide various kinds of synchronization,
@@ -42,7 +39,7 @@
 //!
 //! [^2] `MTLockRef` is a typedef.
 
-use crate::owning_ref::{Erased, OwningRef};
+use crate::owned_slice::OwnedSlice;
 use std::collections::HashMap;
 use std::hash::{BuildHasher, Hash};
 use std::ops::{Deref, DerefMut};
@@ -57,18 +54,11 @@ mod vec;
 
 cfg_if! {
     if #[cfg(not(parallel_compiler))] {
-        pub auto trait Send {}
-        pub auto trait Sync {}
-
-        impl<T> Send for T {}
-        impl<T> Sync for T {}
+        pub unsafe auto trait Send {}
+        pub unsafe auto trait Sync {}
 
-        #[macro_export]
-        macro_rules! rustc_erase_owner {
-            ($v:expr) => {
-                $v.erase_owner()
-            }
-        }
+        unsafe impl<T> Send for T {}
+        unsafe impl<T> Sync for T {}
 
         use std::ops::Add;
 
@@ -197,7 +187,7 @@ cfg_if! {
             }
         }
 
-        pub type MetadataRef = OwningRef<Box<dyn Erased>, [u8]>;
+        pub type MetadataRef = OwnedSlice;
 
         pub use std::rc::Rc as Lrc;
         pub use std::rc::Weak as Weak;
@@ -380,20 +370,11 @@ cfg_if! {
             });
         }
 
-        pub type MetadataRef = OwningRef<Box<dyn Erased + Send + Sync>, [u8]>;
+        pub type MetadataRef = OwnedSlice;
 
         /// This makes locks panic if they are already held.
         /// It is only useful when you are running in a single thread
         const ERROR_CHECKING: bool = false;
-
-        #[macro_export]
-        macro_rules! rustc_erase_owner {
-            ($v:expr) => {{
-                let v = $v;
-                ::rustc_data_structures::sync::assert_send_val(&v);
-                v.erase_send_sync_owner()
-            }}
-        }
     }
 }
 

compiler/rustc_data_structures/src/owning_ref/mod.rs

@@ -22,8 +22,6 @@ extern crate proc_macro;
 extern crate rustc_macros;
 #[macro_use]
 extern crate rustc_middle;
-#[macro_use]
-extern crate rustc_data_structures;
 
 #[macro_use]
 extern crate tracing;

compiler/rustc_data_structures/src/owning_ref/tests.rs

@@ -218,7 +218,7 @@ use crate::rmeta::{rustc_version, MetadataBlob, METADATA_HEADER};
 
 use rustc_data_structures::fx::{FxHashMap, FxHashSet};
 use rustc_data_structures::memmap::Mmap;
-use rustc_data_structures::owning_ref::OwningRef;
+use rustc_data_structures::owned_slice::slice_owned;
 use rustc_data_structures::svh::Svh;
 use rustc_data_structures::sync::MetadataRef;
 use rustc_errors::{DiagnosticArgValue, FatalError, IntoDiagnosticArg};
@@ -236,6 +236,7 @@ use rustc_target::spec::{Target, TargetTriple};
 use snap::read::FrameDecoder;
 use std::borrow::Cow;
 use std::io::{Read, Result as IoResult, Write};
+use std::ops::Deref;
 use std::path::{Path, PathBuf};
 use std::{cmp, fmt};
 
@@ -814,15 +815,14 @@ fn get_metadata_section<'p>(
             // Assume the decompressed data will be at least the size of the compressed data, so we
             // don't have to grow the buffer as much.
             let mut inflated = Vec::with_capacity(compressed_bytes.len());
-            match FrameDecoder::new(compressed_bytes).read_to_end(&mut inflated) {
-                Ok(_) => rustc_erase_owner!(OwningRef::new(inflated).map_owner_box()),
-                Err(_) => {
-                    return Err(MetadataError::LoadFailure(format!(
-                        "failed to decompress metadata: {}",
-                        filename.display()
-                    )));
-                }
-            }
+            FrameDecoder::new(compressed_bytes).read_to_end(&mut inflated).map_err(|_| {
+                MetadataError::LoadFailure(format!(
+                    "failed to decompress metadata: {}",
+                    filename.display()
+                ))
+            })?;
+
+            slice_owned(inflated, Deref::deref)
         }
         CrateFlavor::Rmeta => {
             // mmap the file, because only a small fraction of it is read.
@@ -840,7 +840,7 @@ fn get_metadata_section<'p>(
                 ))
             })?;
 
-            rustc_erase_owner!(OwningRef::new(mmap).map_owner_box())
+            slice_owned(mmap, Deref::deref)
         }
     };
     let blob = MetadataBlob::new(raw_bytes);

compiler/rustc_data_structures/src/sync.rs

@@ -51,12 +51,6 @@ mod cstore_impl;
 #[derive(Clone)]
 pub(crate) struct MetadataBlob(Lrc<MetadataRef>);
 
-// This is needed so we can create an OwningRef into the blob.
-// The data behind a `MetadataBlob` has a stable address because it is
-// contained within an Rc/Arc.
-unsafe impl rustc_data_structures::owning_ref::StableAddress for MetadataBlob {}
-
-// This is needed so we can create an OwningRef into the blob.
 impl std::ops::Deref for MetadataBlob {
     type Target = [u8];
 

compiler/rustc_metadata/src/lib.rs

@@ -1,14 +1,14 @@
 use crate::rmeta::DecodeContext;
 use crate::rmeta::EncodeContext;
-use crate::rmeta::MetadataBlob;
-use rustc_data_structures::owning_ref::OwningRef;
+use rustc_data_structures::owned_slice::slice_owned;
+use rustc_data_structures::owned_slice::OwnedSlice;
 use rustc_hir::def_path_hash_map::{Config as HashMapConfig, DefPathHashMap};
 use rustc_middle::parameterized_over_tcx;
 use rustc_serialize::{Decodable, Decoder, Encodable, Encoder};
 use rustc_span::def_id::{DefIndex, DefPathHash};
 
 pub(crate) enum DefPathHashMapRef<'tcx> {
-    OwnedFromMetadata(odht::HashTable<HashMapConfig, OwningRef<MetadataBlob, [u8]>>),
+    OwnedFromMetadata(odht::HashTable<HashMapConfig, OwnedSlice>),
     BorrowedFromTcx(&'tcx DefPathHashMap),
 }
 
@@ -50,11 +50,11 @@ impl<'a, 'tcx> Decodable<DecodeContext<'a, 'tcx>> for DefPathHashMapRef<'static>
 
         let len = d.read_usize();
         let pos = d.position();
-        let o = OwningRef::new(d.blob().clone()).map(|x| &x[pos..pos + len]);
+        let o = slice_owned(d.blob().clone(), |blob| &blob[pos..pos + len]);
 
-        // Although we already have the data we need via the OwningRef, we still need
-        // to advance the DecodeContext's position so it's in a valid state after
-        // the method. We use read_raw_bytes() for that.
+        // Although we already have the data we need via the `OwnedSlice`, we still need
+        // to advance the `DecodeContext`'s position so it's in a valid state after
+        // the method. We use `read_raw_bytes()` for that.
         let _ = d.read_raw_bytes(len);
 
         let inner = odht::HashTable::from_raw_bytes(o).unwrap_or_else(|e| {