Stream: git-wasmtime

Topic: wasmtime / PR #7065 Do proper type checking for type hand...


view this post on Zulip 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.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 19 2023 at 21:32):

rylev requested pchickey for a review on PR #7065.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 19 2023 at 21:32):

rylev requested wasmtime-core-reviewers for a review on PR #7065.

view this post on Zulip 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?

view this post on Zulip Wasmtime GitHub notifications bot (Sep 19 2023 at 21:33):

rylev submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 19 2023 at 22:25):

alexcrichton submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 19 2023 at 22:25):

alexcrichton created PR review comment:

s/self/other/

view this post on Zulip Wasmtime GitHub notifications bot (Sep 19 2023 at 22:29):

alexcrichton submitted PR review:

:+1:

view this post on Zulip Wasmtime GitHub notifications bot (Sep 19 2023 at 22:29):

alexcrichton submitted PR review:

:+1:

view this post on Zulip 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 HashMap reordering of fields isn't really implemented anywhere except adapter trampolines, and even that's on its way out sorta.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 19 2023 at 22:29):

alexcrichton created PR review comment:

Same thing about order as above

view this post on Zulip 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

view this post on Zulip 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

view this post on Zulip Wasmtime GitHub notifications bot (Sep 19 2023 at 22:29):

alexcrichton created PR review comment:

Truncated comment?

view this post on Zulip Wasmtime GitHub notifications bot (Sep 20 2023 at 08:00):

rylev updated PR #7065.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 20 2023 at 08:01):

rylev requested alexcrichton for a review on PR #7065.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 20 2023 at 14:34):

alexcrichton submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 20 2023 at 14:34):

alexcrichton submitted PR review.

view this post on Zulip 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

view this post on Zulip Wasmtime GitHub notifications bot (Sep 21 2023 at 08:06):

rylev updated PR #7065.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 21 2023 at 08:06):

rylev submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 21 2023 at 08:06):

rylev created PR review comment:

Gah sorry - made the change too quickly... Fixed.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 21 2023 at 08:06):

rylev requested alexcrichton for a review on PR #7065.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 21 2023 at 17:29):

alexcrichton submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 21 2023 at 18:38):

alexcrichton merged PR #7065.


Last updated: Dec 23 2024 at 12:05 UTC