Refactor the internals of Func to remove layers of indirection#1363
Conversation
At this point `Func` has evolved quite a bit since inception and the `WrappedCallable` trait I don't believe is needed any longer. This should help clean up a few entry points by having fewer traits in play.
This commit removes the `wasmtime::Callable` trait, changing the signature of `Func::new` to take an appropriately typed `Fn`. Additionally the function now always takes `&Caller` like `Func::wrap` optionally can, to empower `Func::new` to have the same capabilities of `Func::wrap`.
github-actions
Bot
added
the
wasmtime:api
Subscribe to Label ActionThis issue or pull request has been labeled: "wasmtime:api" Users Subscribed to "wasmtime:api"To subscribe or unsubscribe from this label, edit the |
github-actions
Bot
added
fuzzing
Subscribe to Label ActionThis issue or pull request has been labeled: "fuzzing", "wasmtime:c-api" Users Subscribed to "fuzzing"Users Subscribed to "wasmtime:c-api"To subscribe or unsubscribe from this label, edit the |
sunfishcode
left a comment
sunfishcode
left a comment
There was a problem hiding this comment.
This looks like a nice simplification!
| // Create our actual trampoline function which translates from a bunch | ||
| // of bit patterns on the stack to actual instances of `Val` being | ||
| // passed to the given function. | ||
| let func = Box::new(move |caller_vmctx, values_vec: *mut i128| { |
There was a problem hiding this comment.
We use u128 for values in a lot of places, Is there a reason to prefer i128 here?
Random idea, don't feel obliged to do this here, but we could also introduce a new type, like VMValue or so, which has the size and alignment of a u128, that we could use instead of u128 everywhere, to help document the intent.
There was a problem hiding this comment.
Nah this was just carried over from the original implementation, I'll switch it all to u128
alexcrichton
force-pushed
the
less-traits
branch
from
0b8ed35 to
2ed2164
Compare
2 participants
This commit simplifies
Funcand its API through a few avenues:callablemodule is removed in favor of always calling functions through their trampoline (now that we always have one)Callabletrait is removed in favor of just using animpl Fn()argumentFunc::newmethod is now generic and no longer requires an explicitRcallocationFunc::newclosure now takes a leading&Callerargument to empower it with the same capabilities ofFunc::wrap