From b3f64946f9d89833c1f05bd010c464f54c3fb6b5 Mon Sep 17 00:00:00 2001 From: Eduard-Mihai Burtescu Date: Mon, 28 Oct 2019 19:55:25 +0200 Subject: [PATCH 1/3] proc_macro: remove now-unnecessary ICE workarounds from bridge::client. --- src/libproc_macro/bridge/client.rs | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/src/libproc_macro/bridge/client.rs b/src/libproc_macro/bridge/client.rs index 5c543165bc2b1..cd33c4bc67e76 100644 --- a/src/libproc_macro/bridge/client.rs +++ b/src/libproc_macro/bridge/client.rs @@ -15,8 +15,9 @@ macro_rules! define_handles { } impl HandleCounters { - // FIXME(#53451) public to work around `Cannot create local mono-item` ICE. - pub extern "C" fn get() -> &'static Self { + // FIXME(eddyb) use a reference to the `static COUNTERS`, intead of + // a wrapper `fn` pointer, once `const fn` can reference `static`s. + extern "C" fn get() -> &'static Self { static COUNTERS: HandleCounters = HandleCounters { $($oty: AtomicUsize::new(1),)* $($ity: AtomicUsize::new(1),)* @@ -333,14 +334,14 @@ impl Bridge<'_> { #[repr(C)] #[derive(Copy, Clone)] pub struct Client { + // FIXME(eddyb) use a reference to the `static COUNTERS`, intead 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, } -// FIXME(#53451) public to work around `Cannot create local mono-item` ICE, -// affecting not only the function itself, but also the `BridgeState` `thread_local!`. -pub extern "C" fn __run_expand1( +extern "C" fn run_expand1( mut bridge: Bridge<'_>, f: fn(crate::TokenStream) -> crate::TokenStream, ) -> Buffer { @@ -385,15 +386,13 @@ impl Client crate::TokenStream> { pub const fn expand1(f: fn(crate::TokenStream) -> crate::TokenStream) -> Self { Client { get_handle_counters: HandleCounters::get, - run: __run_expand1, + run: run_expand1, f, } } } -// FIXME(#53451) public to work around `Cannot create local mono-item` ICE, -// affecting not only the function itself, but also the `BridgeState` `thread_local!`. -pub extern "C" fn __run_expand2( +extern "C" fn run_expand2( mut bridge: Bridge<'_>, f: fn(crate::TokenStream, crate::TokenStream) -> crate::TokenStream, ) -> Buffer { @@ -441,7 +440,7 @@ impl Client crate::TokenStream> { ) -> Self { Client { get_handle_counters: HandleCounters::get, - run: __run_expand2, + run: run_expand2, f, } } From da5965f3a6bbc984d97027896fad9c4f551426fd Mon Sep 17 00:00:00 2001 From: Eduard-Mihai Burtescu Date: Mon, 28 Oct 2019 20:21:12 +0200 Subject: [PATCH 2/3] proc_macro: consolidate bridge::client::run_expand{1,2} into one helper. --- src/libproc_macro/bridge/client.rs | 71 ++++++++++-------------------- 1 file changed, 23 insertions(+), 48 deletions(-) diff --git a/src/libproc_macro/bridge/client.rs b/src/libproc_macro/bridge/client.rs index cd33c4bc67e76..b17359a4c8cf7 100644 --- a/src/libproc_macro/bridge/client.rs +++ b/src/libproc_macro/bridge/client.rs @@ -341,9 +341,12 @@ pub struct Client { pub(super) f: F, } -extern "C" fn run_expand1( +/// 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 DecodeMut<'a, 's, ()>, R: Encode<()>>( mut bridge: Bridge<'_>, - f: fn(crate::TokenStream) -> crate::TokenStream, + f: impl FnOnce(A) -> R, ) -> Buffer { // The initial `cached_buffer` contains the input. let mut b = bridge.cached_buffer.take(); @@ -351,12 +354,12 @@ extern "C" fn run_expand1( panic::catch_unwind(panic::AssertUnwindSafe(|| { bridge.enter(|| { let reader = &mut &b[..]; - let input = TokenStream::decode(reader, &mut ()); + let input = A::decode(reader, &mut ()); // Put the `cached_buffer` back in the `Bridge`, for requests. Bridge::with(|bridge| bridge.cached_buffer = b.take()); - let output = f(crate::TokenStream(input)).0; + let output = f(input); // Take the `cached_buffer` back out, for the output value. b = Bridge::with(|bridge| bridge.cached_buffer.take()); @@ -384,63 +387,35 @@ extern "C" fn run_expand1( impl Client crate::TokenStream> { pub const fn expand1(f: fn(crate::TokenStream) -> crate::TokenStream) -> Self { + extern "C" fn run( + bridge: Bridge<'_>, + f: fn(crate::TokenStream) -> crate::TokenStream, + ) -> Buffer { + run_client(bridge, |input| f(crate::TokenStream(input)).0) + } Client { get_handle_counters: HandleCounters::get, - run: run_expand1, + run, f, } } } -extern "C" fn run_expand2( - mut bridge: Bridge<'_>, - f: fn(crate::TokenStream, crate::TokenStream) -> crate::TokenStream, -) -> Buffer { - // The initial `cached_buffer` contains the input. - let mut b = bridge.cached_buffer.take(); - - panic::catch_unwind(panic::AssertUnwindSafe(|| { - bridge.enter(|| { - let reader = &mut &b[..]; - let input = TokenStream::decode(reader, &mut ()); - let input2 = TokenStream::decode(reader, &mut ()); - - // Put the `cached_buffer` back in the `Bridge`, for requests. - Bridge::with(|bridge| bridge.cached_buffer = b.take()); - - let output = f(crate::TokenStream(input), crate::TokenStream(input2)).0; - - // Take the `cached_buffer` back out, for the output value. - b = Bridge::with(|bridge| bridge.cached_buffer.take()); - - // HACK(eddyb) Separate encoding a success value (`Ok(output)`) - // from encoding a panic (`Err(e: PanicMessage)`) to avoid - // having handles outside the `bridge.enter(|| ...)` scope, and - // to catch panics that could happen while encoding the success. - // - // Note that panics should be impossible beyond this point, but - // this is defensively trying to avoid any accidental panicking - // reaching the `extern "C"` (which should `abort` but may not - // at the moment, so this is also potentially preventing UB). - b.clear(); - Ok::<_, ()>(output).encode(&mut b, &mut ()); - }) - })) - .map_err(PanicMessage::from) - .unwrap_or_else(|e| { - b.clear(); - Err::<(), _>(e).encode(&mut b, &mut ()); - }); - b -} - impl Client crate::TokenStream> { pub const fn expand2( f: fn(crate::TokenStream, crate::TokenStream) -> crate::TokenStream ) -> Self { + extern "C" fn run( + bridge: Bridge<'_>, + f: fn(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: run_expand2, + run, f, } } From b58634454f2a6def88f3197261a458e208f77f49 Mon Sep 17 00:00:00 2001 From: Eduard-Mihai Burtescu Date: Mon, 28 Oct 2019 20:34:59 +0200 Subject: [PATCH 3/3] proc_macro: don't use Rust ABI fn pointers in a C ABI fn signature. --- src/libproc_macro/bridge/client.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libproc_macro/bridge/client.rs b/src/libproc_macro/bridge/client.rs index b17359a4c8cf7..9643dba997aa4 100644 --- a/src/libproc_macro/bridge/client.rs +++ b/src/libproc_macro/bridge/client.rs @@ -389,7 +389,7 @@ impl Client crate::TokenStream> { pub const fn expand1(f: fn(crate::TokenStream) -> crate::TokenStream) -> Self { extern "C" fn run( bridge: Bridge<'_>, - f: fn(crate::TokenStream) -> crate::TokenStream, + f: impl FnOnce(crate::TokenStream) -> crate::TokenStream, ) -> Buffer { run_client(bridge, |input| f(crate::TokenStream(input)).0) } @@ -407,7 +407,7 @@ impl Client crate::TokenStream> { ) -> Self { extern "C" fn run( bridge: Bridge<'_>, - f: fn(crate::TokenStream, crate::TokenStream) -> crate::TokenStream, + f: impl FnOnce(crate::TokenStream, crate::TokenStream) -> crate::TokenStream, ) -> Buffer { run_client(bridge, |(input, input2)| { f(crate::TokenStream(input), crate::TokenStream(input2)).0