Stream: git-wasmtime

Topic: wasmtime / PR #14128 feat: implement PartialEq/Eq/Hash fo...

Wasmtime GitHub notifications bot (Aug 13 2026 at 17:00):

smarcd edited PR #14128.

Wasmtime GitHub notifications bot (Aug 13 2026 at 17:00):

smarcd updated PR #14128.

Wasmtime GitHub notifications bot (Aug 13 2026 at 17:04):

smarcd commented on PR #14128:

Done: title renamed, doc comment trimmed, test moved to tests/all/func.rs and simplified to fetch-multiple-ways equality (name, exports(), table).

One flag: Eq doesn't hold across an import/export boundary between two instances (each instance copies its own VMFunctionImport record, so the pointer differs even though it calls the same code). Left the test scoped to one instance. Let me know if cross-instance identity was actually expected (can compare through to the underlying code/vmctx pointer instead if so).

Wasmtime GitHub notifications bot (Aug 13 2026 at 17:28):

cfallin commented on PR #14128:

Hmm, that's actually somewhat surprising API behavior, IMHO -- equality (and hashing) should hold when a Func refers to the same function within a store, regardless how it's reached.

I think this means we need to take a Store borrow so we can safely reach inside the raw pointers. That means we can't implement the Eq and Hash traits literally -- we need to provide separate methods on the Func for this. I'd imagine something like Func::is_same(&store, &f1, &f2) or similar. cc @alexcrichton for thoughts on this as well since it's a fairly conspicuous API surface...

Wasmtime GitHub notifications bot (Aug 13 2026 at 17:42):

smarcd updated PR #14128.

Wasmtime GitHub notifications bot (Aug 13 2026 at 17:43):

smarcd commented on PR #14128:

Agreed, and that tracks with what I found — pushed Func::is_same(store, a, b) and Func::identity_key(store) in place of the trait impls.

Both take a store borrow and dereference into the VMFuncRef's own identity (vmctx, wasm_call, array_call) rather than comparing the wrapper pointer's own address, so they see through the import/export copy: the underlying entry points and vmctx are identical across instances even though each instance holds its own VMFunctionImport record at a different address. identity_key returns an opaque Hash + Eq value for HashMap use, consistent with is_same.

Extended the test to cover the case that broke last time: a function imported into a second instance and re-exported from there now correctly reports is_same and equal identity_key.

Wasmtime GitHub notifications bot (Aug 13 2026 at 21:00):

alexcrichton commented on PR #14128:

This seems reasonable to me, yeah, thanks! I'd probably go ahead and throw the type index into the hashed key as well to be safe, although it's probably not strictly necessary either

Wasmtime GitHub notifications bot (Aug 13 2026 at 21:39):

:memo: cfallin submitted PR review:

Thanks @smarcd -- another nit below, and see Alex's point. If you can address those and get the CI green, I'm happy to approve and merge.

Wasmtime GitHub notifications bot (Aug 13 2026 at 21:39):

:speech_balloon: cfallin created PR review comment:

It's probably sufficient to return just the wasm_call field here: each exported function will have the two entry points but either one will uniquely identify the function.

Wasmtime GitHub notifications bot (Aug 14 2026 at 05:09):

smarcd updated PR #14128.

Wasmtime GitHub notifications bot (Aug 14 2026 at 05:14):

smarcd commented on PR #14128:

Checked wasm_call before committing to it: it's None for a host Func::wrap until that Func is later paired with a module that has a matching trampoline, and it's filled in in place at that point (confirmed with a throwaway probe: None before instantiation, Some(..) after).

That's two problems for using it alone as the key:

• Two never-imported host functions both read wasm_call = None, so they'd collide.
• Even a single Func's key would change over the store's lifetime as it flips None → Some, which breaks HashMap use outright, not just an occasional collision.

array_call doesn't have either issue (always present, never mutates), so I kept (vmctx, array_call) and folded in type_index per Alex - dropped wasm_call instead of keeping only it.

Also fixed the regression test along the way: it wrapped two different || {} closures to check that distinct host functions aren't is_same, but two separate closure expressions are two distinct Rust types, so that assertion happened to pass for the wrong reason (different array_call, not different vmctx). Now wraps the same closure value twice, so array_call is shared and only vmctx discriminates, plus a new test asserting identity_key is stable across the wasm_call transition.

Wasmtime GitHub notifications bot (Aug 14 2026 at 05:18):

smarcd updated PR #14128.

Wasmtime GitHub notifications bot (Aug 14 2026 at 16:57):

:speech_balloon: cfallin created PR review comment:

No need for the narrative comment here (which bits are exported, etc) -- that's also something we can easily change if needed.

Better to explain why we choose these keys. We include vmctx to disambiguate statically-same functions from different instances of its module; we include the array-call pointer because it's always present and there is one per separate function; and we include the type-index as insurance to ensure that we disambiguate functions based on their Wasm-level signatures.

Wasmtime GitHub notifications bot (Aug 14 2026 at 16:57):

:thumbs_up: cfallin submitted PR review:

Thanks -- just one nit then I'm happy to merge.

Wasmtime GitHub notifications bot (Aug 14 2026 at 18:36):

smarcd updated PR #14128.

Wasmtime GitHub notifications bot (Aug 14 2026 at 18:36):

smarcd commented on PR #14128:

Done.

Wasmtime GitHub notifications bot (Aug 14 2026 at 18:39):

:thumbs_up: cfallin submitted PR review.

Wasmtime GitHub notifications bot (Aug 14 2026 at 18:39):

cfallin has enabled auto merge for PR #14128.

Wasmtime GitHub notifications bot (Aug 14 2026 at 18:45):

cfallin added PR #14128 feat: implement PartialEq/Eq/Hash for Func to the merge queue.

Wasmtime GitHub notifications bot (Aug 14 2026 at 19:11):

github-merge-queue[bot] removed PR #14128 feat: implement PartialEq/Eq/Hash for Func from the merge queue.

Last updated: Aug 30 2026 at 09:07 UTC