From 78a83b0d5fb90d3fa9418592282b55e9f2e496be Mon Sep 17 00:00:00 2001 From: Eduard-Mihai Burtescu Date: Fri, 27 May 2022 17:38:58 +0000 Subject: [PATCH] proc_macro: don't pass a client-side function pointer through the server. --- .../src/proc_macro_harness.rs | 71 ++++++++------- compiler/rustc_expand/src/proc_macro.rs | 6 +- library/proc_macro/src/bridge/client.rs | 81 +++++++++-------- library/proc_macro/src/bridge/mod.rs | 2 + .../proc_macro/src/bridge/selfless_reify.rs | 83 ++++++++++++++++++ library/proc_macro/src/bridge/server.rs | 87 +++++++------------ src/test/ui/proc-macro/signature.rs | 2 +- src/test/ui/proc-macro/signature.stderr | 19 ++-- 8 files changed, 217 insertions(+), 134 deletions(-) create mode 100644 library/proc_macro/src/bridge/selfless_reify.rs diff --git a/compiler/rustc_builtin_macros/src/proc_macro_harness.rs b/compiler/rustc_builtin_macros/src/proc_macro_harness.rs index 71e98bb447a7c..03159d4395066 100644 --- a/compiler/rustc_builtin_macros/src/proc_macro_harness.rs +++ b/compiler/rustc_builtin_macros/src/proc_macro_harness.rs @@ -294,50 +294,57 @@ fn mk_decls(cx: &mut ExtCtxt<'_>, macros: &[ProcMacro]) -> P { // that we generate expressions. The position of each NodeId // in the 'proc_macros' Vec corresponds to its position // in the static array that will be generated - let decls = { - let local_path = |cx: &ExtCtxt<'_>, sp: Span, name| { - cx.expr_path(cx.path(sp.with_ctxt(span.ctxt()), vec![name])) - }; - let proc_macro_ty_method_path = |cx: &ExtCtxt<'_>, method| { - cx.expr_path(cx.path(span, vec![proc_macro, bridge, client, proc_macro_ty, method])) - }; - let attr_or_bang = |cx: &mut ExtCtxt<'_>, ca: &ProcMacroDef, ident| { - cx.resolver.declare_proc_macro(ca.id); - cx.expr_call( - span, - proc_macro_ty_method_path(cx, ident), - vec![ - cx.expr_str(ca.span, ca.function_name.name), - local_path(cx, ca.span, ca.function_name), - ], - ) - }; - macros - .iter() - .map(|m| match m { + let decls = macros + .iter() + .map(|m| { + let harness_span = span; + let span = match m { + ProcMacro::Derive(m) => m.span, + ProcMacro::Attr(m) | ProcMacro::Bang(m) => m.span, + }; + let local_path = |cx: &ExtCtxt<'_>, name| cx.expr_path(cx.path(span, vec![name])); + let proc_macro_ty_method_path = |cx: &ExtCtxt<'_>, method| { + cx.expr_path(cx.path( + span.with_ctxt(harness_span.ctxt()), + vec![proc_macro, bridge, client, proc_macro_ty, method], + )) + }; + match m { ProcMacro::Derive(cd) => { cx.resolver.declare_proc_macro(cd.id); cx.expr_call( span, proc_macro_ty_method_path(cx, custom_derive), vec![ - cx.expr_str(cd.span, cd.trait_name), + cx.expr_str(span, cd.trait_name), cx.expr_vec_slice( span, - cd.attrs - .iter() - .map(|&s| cx.expr_str(cd.span, s)) - .collect::>(), + cd.attrs.iter().map(|&s| cx.expr_str(span, s)).collect::>(), ), - local_path(cx, cd.span, cd.function_name), + local_path(cx, cd.function_name), ], ) } - ProcMacro::Attr(ca) => attr_or_bang(cx, &ca, attr), - ProcMacro::Bang(ca) => attr_or_bang(cx, &ca, bang), - }) - .collect() - }; + ProcMacro::Attr(ca) | ProcMacro::Bang(ca) => { + cx.resolver.declare_proc_macro(ca.id); + let ident = match m { + ProcMacro::Attr(_) => attr, + ProcMacro::Bang(_) => bang, + ProcMacro::Derive(_) => unreachable!(), + }; + + cx.expr_call( + span, + proc_macro_ty_method_path(cx, ident), + vec![ + cx.expr_str(span, ca.function_name.name), + local_path(cx, ca.function_name), + ], + ) + } + } + }) + .collect(); let decls_static = cx .item_static( diff --git a/compiler/rustc_expand/src/proc_macro.rs b/compiler/rustc_expand/src/proc_macro.rs index eb59c5a185492..9e1cd299fd60f 100644 --- a/compiler/rustc_expand/src/proc_macro.rs +++ b/compiler/rustc_expand/src/proc_macro.rs @@ -14,7 +14,7 @@ use rustc_span::{Span, DUMMY_SP}; const EXEC_STRATEGY: pm::bridge::server::SameThread = pm::bridge::server::SameThread; pub struct BangProcMacro { - pub client: pm::bridge::client::Client pm::TokenStream>, + pub client: pm::bridge::client::Client, } impl base::BangProcMacro for BangProcMacro { @@ -42,7 +42,7 @@ impl base::BangProcMacro for BangProcMacro { } pub struct AttrProcMacro { - pub client: pm::bridge::client::Client pm::TokenStream>, + pub client: pm::bridge::client::Client<(pm::TokenStream, pm::TokenStream), pm::TokenStream>, } impl base::AttrProcMacro for AttrProcMacro { @@ -73,7 +73,7 @@ impl base::AttrProcMacro for AttrProcMacro { } pub struct DeriveProcMacro { - pub client: pm::bridge::client::Client pm::TokenStream>, + pub client: pm::bridge::client::Client, } impl MultiItemModifier for DeriveProcMacro { diff --git a/library/proc_macro/src/bridge/client.rs b/library/proc_macro/src/bridge/client.rs index 54d92ff5767f2..c38457ac6712d 100644 --- a/library/proc_macro/src/bridge/client.rs +++ b/library/proc_macro/src/bridge/client.rs @@ -357,22 +357,33 @@ impl Bridge<'_> { } } -/// A client-side "global object" (usually a function pointer), -/// which may be using a different `proc_macro` from the one -/// used by the server, but can be interacted with compatibly. +/// A client-side RPC entry-point, which may be using a different `proc_macro` +/// from the one used by the server, but can be invoked compatibly. /// -/// N.B., `F` must have FFI-friendly memory layout (e.g., a pointer). -/// The call ABI of function pointers used for `F` doesn't -/// need to match between server and client, since it's only -/// passed between them and (eventually) called by the client. +/// Note that the (phantom) `I` ("input") and `O` ("output") type parameters +/// decorate the `Client` with the RPC "interface" of the entry-point, but +/// do not themselves participate in ABI, at all, only facilitate type-checking. +/// +/// E.g. `Client` is the common proc macro interface, +/// used for `#[proc_macro] fn foo(input: TokenStream) -> TokenStream`, +/// indicating that the RPC input and output will be serialized token streams, +/// and forcing the use of APIs that take/return `S::TokenStream`, server-side. #[repr(C)] -#[derive(Copy, Clone)] -pub struct Client { +pub struct Client { // 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, - pub(super) f: F, + + pub(super) run: extern "C" fn(Bridge<'_>) -> Buffer, + + pub(super) _marker: PhantomData O>, +} + +impl Copy for Client {} +impl Clone for Client { + fn clone(&self) -> Self { + *self + } } /// Client-side helper for handling client panics, entering the bridge, @@ -419,31 +430,31 @@ fn run_client DecodeMut<'a, 's, ()>, R: Encode<()>>( buf } -impl Client crate::TokenStream> { - pub const fn expand1(f: fn(crate::TokenStream) -> crate::TokenStream) -> Self { - extern "C" fn run( - bridge: Bridge<'_>, - f: impl FnOnce(crate::TokenStream) -> crate::TokenStream, - ) -> Buffer { - run_client(bridge, |input| f(crate::TokenStream(input)).0) +impl Client { + pub const fn expand1(f: impl Fn(crate::TokenStream) -> crate::TokenStream + Copy) -> Self { + Client { + get_handle_counters: HandleCounters::get, + run: super::selfless_reify::reify_to_extern_c_fn_hrt_bridge(move |bridge| { + run_client(bridge, |input| f(crate::TokenStream(input)).0) + }), + _marker: PhantomData, } - Client { get_handle_counters: HandleCounters::get, run, f } } } -impl Client crate::TokenStream> { +impl Client<(crate::TokenStream, crate::TokenStream), crate::TokenStream> { pub const fn expand2( - f: fn(crate::TokenStream, crate::TokenStream) -> crate::TokenStream, + f: impl Fn(crate::TokenStream, crate::TokenStream) -> crate::TokenStream + Copy, ) -> Self { - extern "C" fn run( - bridge: Bridge<'_>, - f: impl FnOnce(crate::TokenStream, crate::TokenStream) -> crate::TokenStream, - ) -> Buffer { - run_client(bridge, |(input, input2)| { - f(crate::TokenStream(input), crate::TokenStream(input2)).0 - }) + Client { + get_handle_counters: HandleCounters::get, + run: super::selfless_reify::reify_to_extern_c_fn_hrt_bridge(move |bridge| { + run_client(bridge, |(input, input2)| { + f(crate::TokenStream(input), crate::TokenStream(input2)).0 + }) + }), + _marker: PhantomData, } - Client { get_handle_counters: HandleCounters::get, run, f } } } @@ -453,17 +464,17 @@ pub enum ProcMacro { CustomDerive { trait_name: &'static str, attributes: &'static [&'static str], - client: Client crate::TokenStream>, + client: Client, }, Attr { name: &'static str, - client: Client crate::TokenStream>, + client: Client<(crate::TokenStream, crate::TokenStream), crate::TokenStream>, }, Bang { name: &'static str, - client: Client crate::TokenStream>, + client: Client, }, } @@ -479,21 +490,21 @@ impl ProcMacro { pub const fn custom_derive( trait_name: &'static str, attributes: &'static [&'static str], - expand: fn(crate::TokenStream) -> crate::TokenStream, + expand: impl Fn(crate::TokenStream) -> crate::TokenStream + Copy, ) -> Self { ProcMacro::CustomDerive { trait_name, attributes, client: Client::expand1(expand) } } pub const fn attr( name: &'static str, - expand: fn(crate::TokenStream, crate::TokenStream) -> crate::TokenStream, + expand: impl Fn(crate::TokenStream, crate::TokenStream) -> crate::TokenStream + Copy, ) -> Self { ProcMacro::Attr { name, client: Client::expand2(expand) } } pub const fn bang( name: &'static str, - expand: fn(crate::TokenStream) -> crate::TokenStream, + expand: impl Fn(crate::TokenStream) -> crate::TokenStream + Copy, ) -> Self { ProcMacro::Bang { name, client: Client::expand1(expand) } } diff --git a/library/proc_macro/src/bridge/mod.rs b/library/proc_macro/src/bridge/mod.rs index babb1cbac70b5..4d3e89ba09356 100644 --- a/library/proc_macro/src/bridge/mod.rs +++ b/library/proc_macro/src/bridge/mod.rs @@ -208,6 +208,8 @@ mod handle; mod rpc; #[allow(unsafe_code)] mod scoped_cell; +#[allow(unsafe_code)] +mod selfless_reify; #[forbid(unsafe_code)] pub mod server; diff --git a/library/proc_macro/src/bridge/selfless_reify.rs b/library/proc_macro/src/bridge/selfless_reify.rs new file mode 100644 index 0000000000000..4ee4bb87c2bbd --- /dev/null +++ b/library/proc_macro/src/bridge/selfless_reify.rs @@ -0,0 +1,83 @@ +//! Abstraction for creating `fn` pointers from any callable that *effectively* +//! has the equivalent of implementing `Default`, even if the compiler neither +//! provides `Default` nor allows reifying closures (i.e. creating `fn` pointers) +//! other than those with absolutely no captures. +//! +//! More specifically, for a closure-like type to be "effectively `Default`": +//! * it must be a ZST (zero-sized type): no information contained within, so +//! that `Default`'s return value (if it were implemented) is unambiguous +//! * it must be `Copy`: no captured "unique ZST tokens" or any other similar +//! types that would make duplicating values at will unsound +//! * combined with the ZST requirement, this confers a kind of "telecopy" +//! ability: similar to `Copy`, but without keeping the value around, and +//! instead "reconstructing" it (a noop given it's a ZST) when needed +//! * it must be *provably* inhabited: no captured uninhabited types or any +//! other types that cannot be constructed by the user of this abstraction +//! * the proof is a value of the closure-like type itself, in a sense the +//! "seed" for the "telecopy" process made possible by ZST + `Copy` +//! * this requirement is the only reason an abstraction limited to a specific +//! usecase is required: ZST + `Copy` can be checked with *at worst* a panic +//! at the "attempted `::default()` call" time, but that doesn't guarantee +//! that the value can be soundly created, and attempting to use the typical +//! "proof ZST token" approach leads yet again to having a ZST + `Copy` type +//! that is not proof of anything without a value (i.e. isomorphic to a +//! newtype of the type it's trying to prove the inhabitation of) +//! +//! A more flexible (and safer) solution to the general problem could exist once +//! `const`-generic parameters can have type parameters in their types: +//! +//! ```rust,ignore (needs future const-generics) +//! extern "C" fn ffi_wrapper< +//! A, R, +//! F: Fn(A) -> R, +//! const f: F, // <-- this `const`-generic is not yet allowed +//! >(arg: A) -> R { +//! f(arg) +//! } +//! ``` + +use std::mem; + +// FIXME(eddyb) this could be `trait` impls except for the `const fn` requirement. +macro_rules! define_reify_functions { + ($( + fn $name:ident $(<$($param:ident),*>)? + for $(extern $abi:tt)? fn($($arg:ident: $arg_ty:ty),*) -> $ret_ty:ty; + )+) => { + $(pub const fn $name< + $($($param,)*)? + F: Fn($($arg_ty),*) -> $ret_ty + Copy + >(f: F) -> $(extern $abi)? fn($($arg_ty),*) -> $ret_ty { + // FIXME(eddyb) describe the `F` type (e.g. via `type_name::`) once panic + // formatting becomes possible in `const fn`. + assert!(mem::size_of::() == 0, "selfless_reify: closure must be zero-sized"); + + $(extern $abi)? fn wrapper< + $($($param,)*)? + F: Fn($($arg_ty),*) -> $ret_ty + Copy + >($($arg: $arg_ty),*) -> $ret_ty { + let f = unsafe { + // SAFETY: `F` satisfies all criteria for "out of thin air" + // reconstructability (see module-level doc comment). + mem::MaybeUninit::::uninit().assume_init() + }; + f($($arg),*) + } + let _f_proof = f; + wrapper::< + $($($param,)*)? + F + > + })+ + } +} + +define_reify_functions! { + fn _reify_to_extern_c_fn_unary for extern "C" fn(arg: A) -> R; + + // HACK(eddyb) this abstraction is used with `for<'a> fn(Bridge<'a>) -> T` + // but that doesn't work with just `reify_to_extern_c_fn_unary` because of + // the `fn` pointer type being "higher-ranked" (i.e. the `for<'a>` binder). + // FIXME(eddyb) try to remove the lifetime from `Bridge`, that'd help. + fn reify_to_extern_c_fn_hrt_bridge for extern "C" fn(bridge: super::Bridge<'_>) -> R; +} diff --git a/library/proc_macro/src/bridge/server.rs b/library/proc_macro/src/bridge/server.rs index 12c754682a0ef..cbddf39da44d2 100644 --- a/library/proc_macro/src/bridge/server.rs +++ b/library/proc_macro/src/bridge/server.rs @@ -118,12 +118,11 @@ macro_rules! define_dispatcher_impl { with_api!(Self, self_, define_dispatcher_impl); pub trait ExecutionStrategy { - fn run_bridge_and_client( + fn run_bridge_and_client( &self, dispatcher: &mut impl DispatcherTrait, input: Buffer, - run_client: extern "C" fn(Bridge<'_>, D) -> Buffer, - client_data: D, + run_client: extern "C" fn(Bridge<'_>) -> Buffer, force_show_panics: bool, ) -> Buffer; } @@ -131,25 +130,21 @@ pub trait ExecutionStrategy { pub struct SameThread; impl ExecutionStrategy for SameThread { - fn run_bridge_and_client( + fn run_bridge_and_client( &self, dispatcher: &mut impl DispatcherTrait, input: Buffer, - run_client: extern "C" fn(Bridge<'_>, D) -> Buffer, - client_data: D, + run_client: extern "C" fn(Bridge<'_>) -> Buffer, force_show_panics: bool, ) -> Buffer { let mut dispatch = |buf| dispatcher.dispatch(buf); - run_client( - Bridge { - cached_buffer: input, - dispatch: (&mut dispatch).into(), - force_show_panics, - _marker: marker::PhantomData, - }, - client_data, - ) + run_client(Bridge { + cached_buffer: input, + dispatch: (&mut dispatch).into(), + force_show_panics, + _marker: marker::PhantomData, + }) } } @@ -159,12 +154,11 @@ impl ExecutionStrategy for SameThread { pub struct CrossThread1; impl ExecutionStrategy for CrossThread1 { - fn run_bridge_and_client( + fn run_bridge_and_client( &self, dispatcher: &mut impl DispatcherTrait, input: Buffer, - run_client: extern "C" fn(Bridge<'_>, D) -> Buffer, - client_data: D, + run_client: extern "C" fn(Bridge<'_>) -> Buffer, force_show_panics: bool, ) -> Buffer { use std::sync::mpsc::channel; @@ -178,15 +172,12 @@ impl ExecutionStrategy for CrossThread1 { res_rx.recv().unwrap() }; - run_client( - Bridge { - cached_buffer: input, - dispatch: (&mut dispatch).into(), - force_show_panics, - _marker: marker::PhantomData, - }, - client_data, - ) + run_client(Bridge { + cached_buffer: input, + dispatch: (&mut dispatch).into(), + force_show_panics, + _marker: marker::PhantomData, + }) }); for b in req_rx { @@ -200,12 +191,11 @@ impl ExecutionStrategy for CrossThread1 { pub struct CrossThread2; impl ExecutionStrategy for CrossThread2 { - fn run_bridge_and_client( + fn run_bridge_and_client( &self, dispatcher: &mut impl DispatcherTrait, input: Buffer, - run_client: extern "C" fn(Bridge<'_>, D) -> Buffer, - client_data: D, + run_client: extern "C" fn(Bridge<'_>) -> Buffer, force_show_panics: bool, ) -> Buffer { use std::sync::{Arc, Mutex}; @@ -231,15 +221,12 @@ impl ExecutionStrategy for CrossThread2 { } }; - let r = run_client( - Bridge { - cached_buffer: input, - dispatch: (&mut dispatch).into(), - force_show_panics, - _marker: marker::PhantomData, - }, - client_data, - ); + let r = run_client(Bridge { + cached_buffer: input, + dispatch: (&mut dispatch).into(), + force_show_panics, + _marker: marker::PhantomData, + }); // Wake up the server so it can exit the dispatch loop. drop(state2); @@ -268,14 +255,12 @@ fn run_server< S: Server, I: Encode>>, O: for<'a, 's> DecodeMut<'a, 's, HandleStore>>, - D: Copy + Send + 'static, >( strategy: &impl ExecutionStrategy, handle_counters: &'static client::HandleCounters, server: S, input: I, - run_client: extern "C" fn(Bridge<'_>, D) -> Buffer, - client_data: D, + run_client: extern "C" fn(Bridge<'_>) -> Buffer, force_show_panics: bool, ) -> Result { let mut dispatcher = @@ -284,18 +269,12 @@ fn run_server< let mut buf = Buffer::new(); input.encode(&mut buf, &mut dispatcher.handle_store); - buf = strategy.run_bridge_and_client( - &mut dispatcher, - buf, - run_client, - client_data, - force_show_panics, - ); + buf = strategy.run_bridge_and_client(&mut dispatcher, buf, run_client, force_show_panics); Result::decode(&mut &buf[..], &mut dispatcher.handle_store) } -impl client::Client crate::TokenStream> { +impl client::Client { pub fn run( &self, strategy: &impl ExecutionStrategy, @@ -303,21 +282,20 @@ impl client::Client crate::TokenStream> { input: S::TokenStream, force_show_panics: bool, ) -> Result { - let client::Client { get_handle_counters, run, f } = *self; + let client::Client { get_handle_counters, run, _marker } = *self; run_server( strategy, get_handle_counters(), server, as Types>::TokenStream::mark(input), run, - f, force_show_panics, ) .map( as Types>::TokenStream::unmark) } } -impl client::Client crate::TokenStream> { +impl client::Client<(crate::TokenStream, crate::TokenStream), crate::TokenStream> { pub fn run( &self, strategy: &impl ExecutionStrategy, @@ -326,7 +304,7 @@ impl client::Client crate::TokenSt input2: S::TokenStream, force_show_panics: bool, ) -> Result { - let client::Client { get_handle_counters, run, f } = *self; + let client::Client { get_handle_counters, run, _marker } = *self; run_server( strategy, get_handle_counters(), @@ -336,7 +314,6 @@ impl client::Client crate::TokenSt as Types>::TokenStream::mark(input2), ), run, - f, force_show_panics, ) .map( as Types>::TokenStream::unmark) diff --git a/src/test/ui/proc-macro/signature.rs b/src/test/ui/proc-macro/signature.rs index e08928716b05b..2302238253e82 100644 --- a/src/test/ui/proc-macro/signature.rs +++ b/src/test/ui/proc-macro/signature.rs @@ -8,6 +8,6 @@ extern crate proc_macro; #[proc_macro_derive(A)] pub unsafe extern "C" fn foo(a: i32, b: u32) -> u32 { - //~^ ERROR: mismatched types + //~^ ERROR: expected a `Fn<(proc_macro::TokenStream,)>` closure, found `unsafe extern "C" fn loop {} } diff --git a/src/test/ui/proc-macro/signature.stderr b/src/test/ui/proc-macro/signature.stderr index 262a64acc544c..78b0beff0da39 100644 --- a/src/test/ui/proc-macro/signature.stderr +++ b/src/test/ui/proc-macro/signature.stderr @@ -1,20 +1,23 @@ -error[E0308]: mismatched types +error[E0277]: expected a `Fn<(proc_macro::TokenStream,)>` closure, found `unsafe extern "C" fn(i32, u32) -> u32 {foo}` --> $DIR/signature.rs:10:1 | LL | / pub unsafe extern "C" fn foo(a: i32, b: u32) -> u32 { LL | | LL | | loop {} LL | | } - | |_^ expected normal fn, found unsafe fn + | | ^ + | | | + | |_call the function in a closure: `|| unsafe { /* code */ }` + | required by a bound introduced by this call | - = note: expected fn pointer `fn(proc_macro::TokenStream) -> proc_macro::TokenStream` - found fn item `unsafe extern "C" fn(i32, u32) -> u32 {foo}` -note: associated function defined here + = help: the trait `Fn<(proc_macro::TokenStream,)>` is not implemented for `unsafe extern "C" fn(i32, u32) -> u32 {foo}` + = note: unsafe function cannot be called generically without an unsafe block +note: required by a bound in `ProcMacro::custom_derive` --> $SRC_DIR/proc_macro/src/bridge/client.rs:LL:COL | -LL | pub const fn custom_derive( - | ^^^^^^^^^^^^^ +LL | expand: impl Fn(crate::TokenStream) -> crate::TokenStream + Copy, + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `ProcMacro::custom_derive` error: aborting due to previous error -For more information about this error, try `rustc --explain E0308`. +For more information about this error, try `rustc --explain E0277`.