Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[wit-bindgen] provide more control over type ownership #6648

Merged
merged 2 commits into from
Jun 27, 2023

Conversation

dicej
Copy link
Collaborator

@dicej dicej commented Jun 27, 2023

This replaces the duplicate_if_necessary parameter to wasmtime::component::bindgen with a new ownership parameter which provides finer-grained control over whether and how generated types own their fields.

The default is Ownership::Owning, which means types own their fields regardless of how they are used in functions. These types are passed by reference when used as parameters to guest-exported functions. Note that this also affects how unnamed types (e.g. list<list<string>>) are passed: using a reference only at the top level (e.g. &[Vec<String>] instead of &[&[&str]], which is more difficult to construct when using non-'static data).

The other option is Ownership::Borrowing, which includes a duplicate_if_necessary field, providing the same code generation strategy as was used prior to this change.

If we're happy with this approach, I'll open another PR in the wit-bindgen repo to match.

This also fixes a bug that caused named types to be considered owned and/or borrowed when they shouldn't have been due to having fields with unnamed types which were owned and/or borrowed in unrelated interfaces.

This replaces the `duplicate_if_necessary` parameter to
`wasmtime::component::bindgen` with a new `ownership` parameter which provides
finer-grained control over whether and how generated types own their fields.

The default is `Ownership::Owning`, which means types own their fields
regardless of how they are used in functions.  These types passed by reference
when used as parameters to guest-exported functions.  Note that this also
affects how unnamed types (e.g. `list<list<string>>`) are passed: using a
reference only at the top level (e.g. `&[Vec<String>]` instead of `&[&[&str]]`,
which is more difficult to construct when using non-`'static` data).

The other option is `Ownership::Borrowing`, which includes a
`duplicate_if_necessary` field, providing the same code generation strategy as
was used prior to this change.

If we're happy with this approach, I'll open another PR in the `wit-bindgen`
repo to match.

This also fixes a bug that caused named types to be considered owned and/or
borrowed when they shouldn't have been due to having fields with unnamed types
which were owned and/or borrowed in unrelated interfaces.

Signed-off-by: Joel Dice <joel.dice@fermyon.com>
@dicej dicej requested a review from a team as a code owner June 27, 2023 16:20
@dicej dicej requested review from alexcrichton and removed request for a team June 27, 2023 16:20
Signed-off-by: Joel Dice <joel.dice@fermyon.com>
Copy link
Member

@alexcrichton alexcrichton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, thanks!

@alexcrichton alexcrichton added this pull request to the merge queue Jun 27, 2023
Merged via the queue into bytecodealliance:main with commit 537214f Jun 27, 2023
19 checks passed
dicej added a commit to dicej/wit-bindgen that referenced this pull request Jun 27, 2023
This is the guest version of bytecodealliance/wasmtime#6648.

This replaces the duplicate_if_necessary parameter to the Rust binding generator
with a new ownership parameter which provides finer-grained control over whether
and how generated types own their fields.

The default is `Ownership::Owning`, which means types own their fields
regardless of how they are used in functions. These types are passed by
reference when used as parameters to guest-exported functions. Note that this
also affects how unnamed types (e.g. `list<list<string>>`) are passed: using a
reference only at the top level (e.g. `&[Vec<String>]` instead of `&[&[&str]]`,
which is more difficult to construct when using non-'static data).

The other option is `Ownership::Borrowing`, which includes a
`duplicate_if_necessary` field, providing the same code generation strategy as
was used prior to this change.

Signed-off-by: Joel Dice <joel.dice@fermyon.com>
alexcrichton pushed a commit to bytecodealliance/wit-bindgen that referenced this pull request Jun 28, 2023
* provide more control over type ownership

This is the guest version of bytecodealliance/wasmtime#6648.

This replaces the duplicate_if_necessary parameter to the Rust binding generator
with a new ownership parameter which provides finer-grained control over whether
and how generated types own their fields.

The default is `Ownership::Owning`, which means types own their fields
regardless of how they are used in functions. These types are passed by
reference when used as parameters to guest-exported functions. Note that this
also affects how unnamed types (e.g. `list<list<string>>`) are passed: using a
reference only at the top level (e.g. `&[Vec<String>]` instead of `&[&[&str]]`,
which is more difficult to construct when using non-'static data).

The other option is `Ownership::Borrowing`, which includes a
`duplicate_if_necessary` field, providing the same code generation strategy as
was used prior to this change.

Signed-off-by: Joel Dice <joel.dice@fermyon.com>

* fix tests

Signed-off-by: Joel Dice <joel.dice@fermyon.com>

* fix tests, part 2

Signed-off-by: Joel Dice <joel.dice@fermyon.com>

---------

Signed-off-by: Joel Dice <joel.dice@fermyon.com>
rvolosatovs pushed a commit to bytecodealliance/wrpc that referenced this pull request May 23, 2024
* provide more control over type ownership

This is the guest version of bytecodealliance/wasmtime#6648.

This replaces the duplicate_if_necessary parameter to the Rust binding generator
with a new ownership parameter which provides finer-grained control over whether
and how generated types own their fields.

The default is `Ownership::Owning`, which means types own their fields
regardless of how they are used in functions. These types are passed by
reference when used as parameters to guest-exported functions. Note that this
also affects how unnamed types (e.g. `list<list<string>>`) are passed: using a
reference only at the top level (e.g. `&[Vec<String>]` instead of `&[&[&str]]`,
which is more difficult to construct when using non-'static data).

The other option is `Ownership::Borrowing`, which includes a
`duplicate_if_necessary` field, providing the same code generation strategy as
was used prior to this change.

Signed-off-by: Joel Dice <joel.dice@fermyon.com>

* fix tests

Signed-off-by: Joel Dice <joel.dice@fermyon.com>

* fix tests, part 2

Signed-off-by: Joel Dice <joel.dice@fermyon.com>

---------

Signed-off-by: Joel Dice <joel.dice@fermyon.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants