bjoernager/rust
bjoernager/rust
1
Fork 0
Code Activity

Auto merge of #97461 - eddyb:proc-macro-less-payload, r=bjorn3

Browse source 
proc_macro: don't pass a client-side function pointer through the server.

Before this PR, `proc_macro::bridge::Client<F>` contained both:
* the C ABI entry-point `run`, that the server can call to start the client
* some "payload" `f: F` passed to that entry-point
  * in practice, this was always a (client-side Rust ABI) `fn` pointer to the actual function the proc macro author wrote, i.e. `#[proc_macro] fn foo(input: TokenStream) -> TokenStream`

In other words, the client was passing one of its (Rust) `fn` pointers to the server, which was passing it back to the client, for the client to call (see later below for why that was ever needed).

I was inspired by `@nnethercote's` attempt to remove the `get_handle_counters` field from `Client` (see https://github.com/rust-lang/rust/pull/97004#issuecomment-1139273301), which combined with removing the `f` ("payload") field, could theoretically allow for a `#[repr(transparent)]` `Client` that mostly just newtypes the C ABI entry-point `fn` pointer <sub>(and in the context of e.g. wasm isolation, that's *all* you want, since you can reason about it from outside the wasm VM, as just a 32-bit "function table index", that you can pass to the wasm VM to call that function)</sub>.

<hr/>

So this PR removes that "payload". But it's not a simple refactor: the reason the field existed in the first place is because monomorphizing over a function type doesn't let you call the function without having a value of that type, because function types don't implement anything like `Default`, i.e.:
```rust
extern "C" fn ffi_wrapper<A, R, F: Fn(A) -> R>(arg: A) -> R {
    let f: F = ???; // no way to get a value of `F`
    f(arg)
}
```
That could be solved with something like this, if it was allowed:
```rust
extern "C" fn ffi_wrapper<
    A, R,
    F: Fn(A) -> R,
    const f: F // not allowed because the type is a generic param
>(arg: A) -> R {
    f(arg)
}
```

Instead, this PR contains a workaround in `proc_macro::bridge::selfless_reify` (see its module-level comment for more details) that can provide something similar to the `ffi_wrapper` example above, but limited to `F` being `Copy` and ZST (and requiring an `F` value to prove the caller actually can create values of `F` and it's not uninhabited or some other unsound situation).

<hr/>

Hopefully this time we don't have a performance regression, and this has a chance to land.

cc `@mystor` `@bjorn3`
This commit is contained in:
bors 2022-05-28 16:49:52 +00:00
commit 116201eefe
8 changed files with 217 additions and 134 deletions
Download patch file Download diff file Expand all files Collapse all files

71
compiler/rustc_builtin_macros/src/proc_macro_harness.rs
View file

@ -294,50 +294,57 @@ fn mk_decls(cx: &mut ExtCtxt<'_>, macros: &[ProcMacro]) -> P<ast::Item> {
// 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::<Vec<_>>(),
cd.attrs.iter().map(|&s| cx.expr_str(span, s)).collect::<Vec<_>>(),
),
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(