Wasmtime GitHub notifications bot (Sep 19 2023 at 21:32): Wasmtime GitHub notifications bot (Sep 19 2023 at 21:32):rylev opened PR #7065 from rylev:type-check-handle to bytecodealliance:main:
Paired with @alexcrichton
This fixes an issue that makes it impossible to call another component's exported function from the body of wrapped function import (using something like
Instance::func_new) even when that exported function has the same structural signature as the wrapped import.That's because type checking was done assuming that the types in question were coming from the same component (or more specifically the same types arena). This changes to first try to do the quick and inexpensive call of pointer equality followed by a more expensive walk of two types for structural equality.
Wasmtime GitHub notifications bot (Sep 19 2023 at 21:32):rylev requested pchickey for a review on PR #7065.
Wasmtime GitHub notifications bot (Sep 19 2023 at 21:32):rylev requested wasmtime-core-reviewers for a review on PR #7065.
Wasmtime GitHub notifications bot (Sep 19 2023 at 21:33):rylev created PR review comment:
One thing I was unsure of: are flags types with names in different orders the same type?
Wasmtime GitHub notifications bot (Sep 19 2023 at 21:33):rylev submitted PR review.
Wasmtime GitHub notifications bot (Sep 19 2023 at 22:25):alexcrichton submitted PR review.
Wasmtime GitHub notifications bot (Sep 19 2023 at 22:25):alexcrichton created PR review comment:
s/self/other/
Wasmtime GitHub notifications bot (Sep 19 2023 at 22:29):alexcrichton submitted PR review:
:+1:
Wasmtime GitHub notifications bot (Sep 19 2023 at 22:29):alexcrichton submitted PR review:
:+1:
Wasmtime GitHub notifications bot (Sep 19 2023 at 22:29):alexcrichton created PR review comment:
For the purposes of equivalence here in Wasmtime we'll need to require that the fields are in the exact same order. Lifting/lowering relies on this right now since the
HashMapreordering of fields isn't really implemented anywhere except adapter trampolines, and even that's on its way out sorta.
Wasmtime GitHub notifications bot (Sep 19 2023 at 22:29):alexcrichton created PR review comment:
Same thing about order as above
Wasmtime GitHub notifications bot (Sep 19 2023 at 22:29):alexcrichton created PR review comment:
More-or-less yes it's ok to reorder but for the component model MVP we're going to require exact equivalence
Wasmtime GitHub notifications bot (Sep 19 2023 at 22:29):alexcrichton created PR review comment:
Like above we'll need to check the cases in order
Wasmtime GitHub notifications bot (Sep 19 2023 at 22:29):alexcrichton created PR review comment:
Truncated comment?
Wasmtime GitHub notifications bot (Sep 20 2023 at 08:00):rylev updated PR #7065.
Wasmtime GitHub notifications bot (Sep 20 2023 at 08:01):rylev requested alexcrichton for a review on PR #7065.
Wasmtime GitHub notifications bot (Sep 20 2023 at 14:34):alexcrichton submitted PR review.
Wasmtime GitHub notifications bot (Sep 20 2023 at 14:34):alexcrichton submitted PR review.
Wasmtime GitHub notifications bot (Sep 20 2023 at 14:34):alexcrichton created PR review comment:
oh here we'll still want to check field names as opposed to just types
Wasmtime GitHub notifications bot (Sep 21 2023 at 08:06):rylev updated PR #7065.
Wasmtime GitHub notifications bot (Sep 21 2023 at 08:06):rylev submitted PR review.
Wasmtime GitHub notifications bot (Sep 21 2023 at 08:06):rylev created PR review comment:
Gah sorry - made the change too quickly... Fixed.
Wasmtime GitHub notifications bot (Sep 21 2023 at 08:06):rylev requested alexcrichton for a review on PR #7065.
Wasmtime GitHub notifications bot (Sep 21 2023 at 17:29):alexcrichton submitted PR review.
Wasmtime GitHub notifications bot (Sep 21 2023 at 18:38):alexcrichton merged PR #7065.
Last updated: Oct 23 2024 at 20:03 UTC