Skip to content

proc_macro/bridge: remove Closure. #97045

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

Closed
wants to merge 4 commits into from
Closed
Changes from all 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
69 changes: 43 additions & 26 deletions library/proc_macro/src/bridge/client.rs
Original file line number Diff line number Diff line change
@@ -2,6 +2,7 @@
use super::*;

use std::cell::Cell;
use std::marker::PhantomData;

macro_rules! define_handles {
@@ -256,7 +257,7 @@ macro_rules! define_client_side {
api_tags::Method::$name(api_tags::$name::$method).encode(&mut b, &mut ());
reverse_encode!(b; $($arg),*);

b = bridge.dispatch.call(b);
b = (bridge.dispatch)(b);

let r = Result::<_, PanicMessage>::decode(&mut &b[..], &mut ());

@@ -270,48 +271,64 @@ macro_rules! define_client_side {
}
with_api!(self, self, define_client_side);

enum BridgeState<'a> {
enum BridgeState {
/// No server is currently connected to this client.
NotConnected,

/// A server is connected and available for requests.
Connected(Bridge<'a>),
Connected(Bridge),

/// Access to the bridge is being exclusively acquired
/// (e.g., during `BridgeState::with`).
InUse,
}

enum BridgeStateL {}
impl BridgeState {
/// Sets the thread-local `BridgeState` to `replacement` while
/// running `f`, which gets the old `BridgeState`, mutably.
///
/// The state will be restored after `f` exits, even
/// by panic, including modifications made to it by `f`.
fn replace_during<R>(replacement: Self, f: impl FnOnce(&mut Self) -> R) -> R {
/// Wrapper that ensures that a cell always gets filled
/// (with the original state, optionally changed by `f`),
/// even if `f` had panicked.
struct PutBackOnDrop<'a, T> {
cell: &'a Cell<T>,
value: Option<T>,
}

impl<'a> scoped_cell::ApplyL<'a> for BridgeStateL {
type Out = BridgeState<'a>;
}
impl<'a, T> Drop for PutBackOnDrop<'a, T> {
fn drop(&mut self) {
self.cell.set(self.value.take().unwrap());
}
}

thread_local! {
static BRIDGE_STATE: scoped_cell::ScopedCell<BridgeStateL> =
scoped_cell::ScopedCell::new(BridgeState::NotConnected);
}
thread_local! {
static BRIDGE_STATE: Cell<BridgeState> = Cell::new(BridgeState::NotConnected);
}
BRIDGE_STATE.with(|state| {
let mut put_back_on_drop =
PutBackOnDrop { cell: state, value: Some(state.replace(replacement)) };

f(put_back_on_drop.value.as_mut().unwrap())
})
}

impl BridgeState<'_> {
/// Take exclusive control of the thread-local
/// `BridgeState`, and pass it to `f`, mutably.
///
/// The state will be restored after `f` exits, even
/// by panic, including modifications made to it by `f`.
///
/// N.B., while `f` is running, the thread-local state
/// is `BridgeState::InUse`.
fn with<R>(f: impl FnOnce(&mut BridgeState<'_>) -> R) -> R {
BRIDGE_STATE.with(|state| {
state.replace(BridgeState::InUse, |mut state| {
// FIXME(#52812) pass `f` directly to `replace` when `RefMutL` is gone
f(&mut *state)
})
})
fn with<R>(f: impl FnOnce(&mut Self) -> R) -> R {
Self::replace_during(BridgeState::InUse, f)
}
}

impl Bridge<'_> {
impl Bridge {
pub(crate) fn is_available() -> bool {
BridgeState::with(|state| match state {
BridgeState::Connected(_) | BridgeState::InUse => true,
@@ -337,10 +354,10 @@ impl Bridge<'_> {
}));
});

BRIDGE_STATE.with(|state| state.set(BridgeState::Connected(self), f))
BridgeState::replace_during(BridgeState::Connected(self), |_| f())
}

fn with<R>(f: impl FnOnce(&mut Bridge<'_>) -> R) -> R {
fn with<R>(f: impl FnOnce(&mut Self) -> R) -> R {
BridgeState::with(|state| match state {
BridgeState::NotConnected => {
panic!("procedural macro API is used outside of a procedural macro");
@@ -367,15 +384,15 @@ pub struct Client<F> {
// FIXME(eddyb) use a reference to the `static COUNTERS`, instead of
// a wrapper `fn` pointer, once `const fn` can reference `static`s.
pub(super) get_handle_counters: extern "C" fn() -> &'static HandleCounters,
pub(super) run: extern "C" fn(Bridge<'_>, F) -> Buffer<u8>,
pub(super) run: extern "C" fn(Bridge, F) -> Buffer<u8>,
pub(super) f: F,
}

/// Client-side helper for handling client panics, entering the bridge,
/// deserializing input and serializing output.
// FIXME(eddyb) maybe replace `Bridge::enter` with this?
fn run_client<A: for<'a, 's> DecodeMut<'a, 's, ()>, R: Encode<()>>(
mut bridge: Bridge<'_>,
mut bridge: Bridge,
f: impl FnOnce(A) -> R,
) -> Buffer<u8> {
// The initial `cached_buffer` contains the input.
@@ -418,7 +435,7 @@ fn run_client<A: for<'a, 's> DecodeMut<'a, 's, ()>, R: Encode<()>>(
impl Client<fn(crate::TokenStream) -> crate::TokenStream> {
pub const fn expand1(f: fn(crate::TokenStream) -> crate::TokenStream) -> Self {
extern "C" fn run(
bridge: Bridge<'_>,
bridge: Bridge,
f: impl FnOnce(crate::TokenStream) -> crate::TokenStream,
) -> Buffer<u8> {
run_client(bridge, |input| f(crate::TokenStream(input)).0)
@@ -432,7 +449,7 @@ impl Client<fn(crate::TokenStream, crate::TokenStream) -> crate::TokenStream> {
f: fn(crate::TokenStream, crate::TokenStream) -> crate::TokenStream,
) -> Self {
extern "C" fn run(
bridge: Bridge<'_>,
bridge: Bridge,
f: impl FnOnce(crate::TokenStream, crate::TokenStream) -> crate::TokenStream,
) -> Buffer<u8> {
run_client(bridge, |(input, input2)| {
28 changes: 0 additions & 28 deletions library/proc_macro/src/bridge/closure.rs

This file was deleted.

6 changes: 2 additions & 4 deletions library/proc_macro/src/bridge/mod.rs
Original file line number Diff line number Diff line change
@@ -199,8 +199,6 @@ macro_rules! reverse_decode {
mod buffer;
#[forbid(unsafe_code)]
pub mod client;
#[allow(unsafe_code)]
mod closure;
#[forbid(unsafe_code)]
mod handle;
#[macro_use]
@@ -221,13 +219,13 @@ use rpc::{Decode, DecodeMut, Encode, Reader, Writer};
/// field of `client::Client`. The client holds its copy of the `Bridge`
/// in TLS during its execution (`Bridge::{enter, with}` in `client.rs`).
#[repr(C)]
pub struct Bridge<'a> {
pub struct Bridge {
/// Reusable buffer (only `clear`-ed, never shrunk), primarily
/// used for making requests, but also for passing input to client.
cached_buffer: Buffer<u8>,

/// Server-side function that the client uses to make requests.
dispatch: closure::Closure<'a, Buffer<u8>, Buffer<u8>>,
dispatch: extern "C" fn(Buffer<u8>) -> Buffer<u8>,

/// If 'true', always invoke the default panic hook
force_show_panics: bool,
5 changes: 3 additions & 2 deletions library/proc_macro/src/bridge/scoped_cell.rs
Original file line number Diff line number Diff line change
@@ -41,9 +41,10 @@ impl<T: LambdaL> ScopedCell<T> {

/// Sets the value in `self` to `replacement` while
/// running `f`, which gets the old value, mutably.
///
/// The old value will be restored after `f` exits, even
/// by panic, including modifications made to it by `f`.
pub fn replace<'a, R>(
pub fn replace_during<'a, R>(
&self,
replacement: <T as ApplyL<'a>>::Out,
f: impl for<'b, 'c> FnOnce(RefMutL<'b, 'c, T>) -> R,
@@ -76,6 +77,6 @@ impl<T: LambdaL> ScopedCell<T> {

/// Sets the value in `self` to `value` while running `f`.
pub fn set<R>(&self, value: <T as ApplyL<'_>>::Out, f: impl FnOnce() -> R) -> R {
self.replace(value, |_| f())
self.replace_during(value, |_| f())
}
}
155 changes: 87 additions & 68 deletions library/proc_macro/src/bridge/server.rs
Original file line number Diff line number Diff line change
@@ -85,14 +85,16 @@ macro_rules! define_dispatcher_impl {
($($name:ident {
$(fn $method:ident($($arg:ident: $arg_ty:ty),* $(,)?) $(-> $ret_ty:ty)?;)*
}),* $(,)?) => {
// FIXME(eddyb) `pub` only for `ExecutionStrategy` below.
pub trait DispatcherTrait {
// HACK(eddyb) these are here to allow `Self::$name` to work below.
// HACK(eddyb) only a (private) trait because inherent impls can't have
// associated types (and `Self::AssocType` is used within `$arg_ty`).
// While `with_api!` allows customizing `Self`, it would have to be
// extended to allow `Self::` to become `<MarkedTypes<S> as Types>::`.
trait DispatcherPrivateHelperTrait {
$(type $name;)*
fn dispatch(&mut self, b: Buffer<u8>) -> Buffer<u8>;
}

impl<S: Server> DispatcherTrait for Dispatcher<MarkedTypes<S>> {
impl<S: Server> DispatcherPrivateHelperTrait for Dispatcher<MarkedTypes<S>> {
$(type $name = <MarkedTypes<S> as Types>::$name;)*
fn dispatch(&mut self, mut b: Buffer<u8>) -> Buffer<u8> {
let Dispatcher { handle_store, server } = self;
@@ -128,39 +130,27 @@ macro_rules! define_dispatcher_impl {
}
with_api!(Self, self_, define_dispatcher_impl);

// FIXME(eddyb) a trait alias of `FnMut(Buffer<u8>) -> Buffer<u8>` would allow
// replacing the non-dynamic mentions of that trait, as well.
pub type DynDispatch<'a> = &'a mut dyn FnMut(Buffer<u8>) -> Buffer<u8>;

pub trait ExecutionStrategy {
fn run_bridge_and_client<D: Copy + Send + 'static>(
fn cross_thread_dispatch(
&self,
dispatcher: &mut impl DispatcherTrait,
input: Buffer<u8>,
run_client: extern "C" fn(Bridge<'_>, D) -> Buffer<u8>,
client_data: D,
force_show_panics: bool,
server_thread_dispatch: impl FnMut(Buffer<u8>) -> Buffer<u8>,
with_client_thread_dispatch: impl FnOnce(DynDispatch<'_>) -> Buffer<u8> + Send + 'static,
) -> Buffer<u8>;
}

pub struct SameThread;

impl ExecutionStrategy for SameThread {
fn run_bridge_and_client<D: Copy + Send + 'static>(
fn cross_thread_dispatch(
&self,
dispatcher: &mut impl DispatcherTrait,
input: Buffer<u8>,
run_client: extern "C" fn(Bridge<'_>, D) -> Buffer<u8>,
client_data: D,
force_show_panics: bool,
mut server_thread_dispatch: impl FnMut(Buffer<u8>) -> Buffer<u8>,
with_client_thread_dispatch: impl FnOnce(DynDispatch<'_>) -> Buffer<u8> + Send + 'static,
) -> Buffer<u8> {
let mut dispatch = |b| dispatcher.dispatch(b);

run_client(
Bridge {
cached_buffer: input,
dispatch: (&mut dispatch).into(),
force_show_panics,
_marker: marker::PhantomData,
},
client_data,
)
with_client_thread_dispatch(&mut server_thread_dispatch)
}
}

@@ -170,38 +160,25 @@ impl ExecutionStrategy for SameThread {
pub struct CrossThread1;

impl ExecutionStrategy for CrossThread1 {
fn run_bridge_and_client<D: Copy + Send + 'static>(
fn cross_thread_dispatch(
&self,
dispatcher: &mut impl DispatcherTrait,
input: Buffer<u8>,
run_client: extern "C" fn(Bridge<'_>, D) -> Buffer<u8>,
client_data: D,
force_show_panics: bool,
mut server_thread_dispatch: impl FnMut(Buffer<u8>) -> Buffer<u8>,
with_client_thread_dispatch: impl FnOnce(DynDispatch<'_>) -> Buffer<u8> + Send + 'static,
) -> Buffer<u8> {
use std::sync::mpsc::channel;

let (req_tx, req_rx) = channel();
let (res_tx, res_rx) = channel();

let join_handle = thread::spawn(move || {
let mut dispatch = |b| {
with_client_thread_dispatch(&mut |b| {
req_tx.send(b).unwrap();
res_rx.recv().unwrap()
};

run_client(
Bridge {
cached_buffer: input,
dispatch: (&mut dispatch).into(),
force_show_panics,
_marker: marker::PhantomData,
},
client_data,
)
})
});

for b in req_rx {
res_tx.send(dispatcher.dispatch(b)).unwrap();
res_tx.send(server_thread_dispatch(b)).unwrap();
}

join_handle.join().unwrap()
@@ -211,13 +188,10 @@ impl ExecutionStrategy for CrossThread1 {
pub struct CrossThread2;

impl ExecutionStrategy for CrossThread2 {
fn run_bridge_and_client<D: Copy + Send + 'static>(
fn cross_thread_dispatch(
&self,
dispatcher: &mut impl DispatcherTrait,
input: Buffer<u8>,
run_client: extern "C" fn(Bridge<'_>, D) -> Buffer<u8>,
client_data: D,
force_show_panics: bool,
mut server_thread_dispatch: impl FnMut(Buffer<u8>) -> Buffer<u8>,
with_client_thread_dispatch: impl FnOnce(DynDispatch<'_>) -> Buffer<u8> + Send + 'static,
) -> Buffer<u8> {
use std::sync::{Arc, Mutex};

@@ -231,7 +205,7 @@ impl ExecutionStrategy for CrossThread2 {
let server_thread = thread::current();
let state2 = state.clone();
let join_handle = thread::spawn(move || {
let mut dispatch = |b| {
let r = with_client_thread_dispatch(&mut |b| {
*state2.lock().unwrap() = State::Req(b);
server_thread.unpark();
loop {
@@ -240,17 +214,7 @@ impl ExecutionStrategy for CrossThread2 {
break b.take();
}
}
};

let r = run_client(
Bridge {
cached_buffer: input,
dispatch: (&mut dispatch).into(),
force_show_panics,
_marker: marker::PhantomData,
},
client_data,
);
});

// Wake up the server so it can exit the dispatch loop.
drop(state2);
@@ -266,7 +230,7 @@ impl ExecutionStrategy for CrossThread2 {
State::Req(b) => b.take(),
_ => continue,
};
b = dispatcher.dispatch(b.take());
b = server_thread_dispatch(b.take());
*state.lock().unwrap() = State::Res(b);
join_handle.thread().unpark();
}
@@ -275,6 +239,60 @@ impl ExecutionStrategy for CrossThread2 {
}
}

fn run_bridge_and_client<D: Copy + Send + 'static>(
strategy: &impl ExecutionStrategy,
server_thread_dispatch: impl FnMut(Buffer<u8>) -> Buffer<u8>,
input: Buffer<u8>,
run_client: extern "C" fn(Bridge, D) -> Buffer<u8>,
client_data: D,
force_show_panics: bool,
) -> Buffer<u8> {
enum OptionDynDispatchL {}

impl<'a> scoped_cell::ApplyL<'a> for OptionDynDispatchL {
type Out = Option<DynDispatch<'a>>;
}

thread_local! {
/// Dispatch callback held in server TLS, and using server ABI, but
/// on the client thread (which can differ from the server thread).
//
// FIXME(eddyb) how redundant is this with the (also) thread-local
// client-side `BridgeState`? Some of that indirection can probably
// be removed, as long as concerns around further isolation methods
// (IPC and/or wasm) are considered.
static CLIENT_THREAD_DISPATCH: scoped_cell::ScopedCell<OptionDynDispatchL> =
scoped_cell::ScopedCell::new(None);
}

strategy.cross_thread_dispatch(server_thread_dispatch, move |client_thread_dispatch| {
CLIENT_THREAD_DISPATCH.with(|dispatch_slot| {
dispatch_slot.set(Some(client_thread_dispatch), || {
extern "C" fn dispatch(b: Buffer<u8>) -> Buffer<u8> {
CLIENT_THREAD_DISPATCH.with(|dispatch_slot| {
dispatch_slot.replace_during(None, |mut client_thread_dispatch| {
client_thread_dispatch.as_mut().expect(
"proc_macro::bridge::server: dispatch used on \
thread lacking an active server-side dispatcher",
)(b)
})
})
}

run_client(
Bridge {
cached_buffer: input,
dispatch,
force_show_panics,
_marker: marker::PhantomData,
},
client_data,
)
})
})
})
}

fn run_server<
S: Server,
I: Encode<HandleStore<MarkedTypes<S>>>,
@@ -285,7 +303,7 @@ fn run_server<
handle_counters: &'static client::HandleCounters,
server: S,
input: I,
run_client: extern "C" fn(Bridge<'_>, D) -> Buffer<u8>,
run_client: extern "C" fn(Bridge, D) -> Buffer<u8>,
client_data: D,
force_show_panics: bool,
) -> Result<O, PanicMessage> {
@@ -295,8 +313,9 @@ fn run_server<
let mut b = Buffer::new();
input.encode(&mut b, &mut dispatcher.handle_store);

b = strategy.run_bridge_and_client(
&mut dispatcher,
b = run_bridge_and_client(
strategy,
|b| dispatcher.dispatch(b),
b,
run_client,
client_data,
4 changes: 2 additions & 2 deletions src/bootstrap/native.rs
Original file line number Diff line number Diff line change
@@ -156,7 +156,7 @@ pub(crate) fn maybe_download_ci_llvm(builder: &Builder<'_>) {
let llvm_lib = llvm_root.join("lib");
for entry in t!(fs::read_dir(&llvm_lib)) {
let lib = t!(entry).path();
if lib.ends_with(".so") {
if lib.extension().map_or(false, |ext| ext == "so") {
fix_bin_or_dylib(builder, &lib);
}
}
@@ -284,7 +284,7 @@ fn fix_bin_or_dylib(builder: &Builder<'_>, fname: &Path) {
entries
};
patchelf.args(&[OsString::from("--set-rpath"), rpath_entries]);
if !fname.ends_with(".so") {
if !fname.extension().map_or(false, |ext| ext == "so") {
// Finally, set the corret .interp for binaries
let dynamic_linker_path = nix_deps_dir.join("nix-support/dynamic-linker");
// FIXME: can we support utf8 here? `args` doesn't accept Vec<u8>, only OsString ...