Skip to content

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

Merged
bors merged 1 commit intorust-lang:masterfrom
eddyb:proc-macro-less-payload
May 28, 2022
Merged

proc_macro: don't pass a client-side function pointer through the server.#97461
bors merged 1 commit intorust-lang:masterfrom
eddyb:proc-macro-less-payload

Conversation

@eddyb
Copy link
Copy Markdown
Member

@eddyb eddyb commented May 27, 2022

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 #97004 (comment)), 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 (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).


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.:

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:

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).


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

cc @mystor @bjorn3

Loading
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merged-by-bors This PR was explicitly merged by bors. perf-regression Performance regression. perf-regression-triaged The performance regression has been triaged. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants