alexcrichton requested sunfishcode for a review on PR #1352.
alexcrichton opened PR #1352 from filter-more
to master
:
Added in #1329 it's now possible for multiple parameters to be non-wasm
parameters, so the previousparam_types
method is no longer suitable
for acquiring all wasm-related parameters, rather thenFuncEnvironment
must be consulted. This removes usage ofparam_types()
as a method
from the wasm translation and instead adds a custom method inline for
filtering the parameters based onis_wasm_parameter
.
alexcrichton updated PR #1352 from filter-more
to master
:
Added in #1329 it's now possible for multiple parameters to be non-wasm
parameters, so the previousparam_types
method is no longer suitable
for acquiring all wasm-related parameters, rather thenFuncEnvironment
must be consulted. This removes usage ofparam_types()
as a method
from the wasm translation and instead adds a custom method inline for
filtering the parameters based onis_wasm_parameter
.
alexcrichton updated PR #1352 from filter-more
to master
:
Added in #1329 it's now possible for multiple parameters to be non-wasm
parameters, so the previousparam_types
method is no longer suitable
for acquiring all wasm-related parameters, rather thenFuncEnvironment
must be consulted. This removes usage ofparam_types()
as a method
from the wasm translation and instead adds a custom method inline for
filtering the parameters based onis_wasm_parameter
.
sunfishcode submitted PR Review.
sunfishcode created PR Review Comment:
Can you comment on why this is
mut
andMut
? The ultimate function should be a non-mutating predicate.
alexcrichton updated PR #1352 from filter-more
to master
:
Added in #1329 it's now possible for multiple parameters to be non-wasm
parameters, so the previousparam_types
method is no longer suitable
for acquiring all wasm-related parameters, rather thenFuncEnvironment
must be consulted. This removes usage ofparam_types()
as a method
from the wasm translation and instead adds a custom method inline for
filtering the parameters based onis_wasm_parameter
.
alexcrichton submitted PR Review.
alexcrichton created PR Review Comment:
Oh this is mostly just a Rust idioms thing where you idiomatically take
FnMut
since it's the most flexible. For brevity and simplicity though I've switched this toFn
, sinceFnMut
wasn't actually needed.
alexcrichton updated PR #1352 from filter-more
to master
:
Added in #1329 it's now possible for multiple parameters to be non-wasm
parameters, so the previousparam_types
method is no longer suitable
for acquiring all wasm-related parameters, rather thenFuncEnvironment
must be consulted. This removes usage ofparam_types()
as a method
from the wasm translation and instead adds a custom method inline for
filtering the parameters based onis_wasm_parameter
.
abrown submitted PR Review.
abrown created PR Review Comment:
It's is_wasm_parameter because it isn't counting the number of ArgumentPurpose::Normal parameters.
@sunfishcode, so the above is a default implementation that embedders are supposed to override?
sunfishcode submitted PR Review.
sunfishcode created PR Review Comment:
Yeah, embedders can override it if they wish. We added this hook because Wasmtime specifically wishes to override it.
alexcrichton requested sunfishcode for a review on PR #1352.
sunfishcode submitted PR Review.
sunfishcode merged PR #1352.
Last updated: Dec 23 2024 at 13:07 UTC