Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Yeet owning_ref #109971

Merged
merged 9 commits into from
Apr 8, 2023
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 5 additions & 6 deletions compiler/rustc_codegen_ssa/src/back/metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

|mmap| f(mmap)

nit: Can this be replaced with f?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not with &f either?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No.

}

impl MetadataLoader for DefaultMetadataLoader {
Expand Down
6 changes: 4 additions & 2 deletions compiler/rustc_data_structures/src/lib.rs
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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)]
Expand Down Expand Up @@ -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;
Expand All @@ -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;
Expand Down
10 changes: 1 addition & 9 deletions compiler/rustc_data_structures/src/memmap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -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);

Expand Down
116 changes: 116 additions & 0 deletions compiler/rustc_data_structures/src/owned_slice.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,116 @@
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];

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 {
fn borrow(&self) -> &[u8] {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It will be nice to check, that this (and other short fns around) call will be inlined, as rust sometimes forget about that.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great call, these two functions should all have #[inline] attributes. I'm not entirely sure where they are all used but given that it's used in metadata loading which is very hot this could be really bad on platforms that aren't compiled with LTO (like our neutral perf run was).

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;
68 changes: 68 additions & 0 deletions compiler/rustc_data_structures/src/owned_slice/tests.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
use std::{cell::Cell, ops::Deref, rc::Rc};

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() {
let flag = Rc::new(Cell::new(false));
let flag_prime = Rc::clone(&flag);
let d = OnDrop(move || flag_prime.set(true));

let slice = slice_owned(d, |_| &[]);

assert_eq!(flag.get(), false);

std::mem::drop(slice);
WaffleLapkin marked this conversation as resolved.
Show resolved Hide resolved

assert_eq!(flag.get(), true);
}

#[test]
fn send_sync() {
crate::sync::assert_send::<OwnedSlice>();
crate::sync::assert_sync::<OwnedSlice>();
}
21 changes: 0 additions & 21 deletions compiler/rustc_data_structures/src/owning_ref/LICENSE

This file was deleted.

Loading