Skip to content

Conversation

@alexcrichton
Copy link
Member

This is a follow-up to #11325 with a number of cosmetic changes about
the shape of the API and structure of the internals:

  • {Stream,Future}{Reader,Writer}::guard is now an alternative
    constructor to Guard*::new (import fewer types).
  • Internally WithAccessor and DropWithStore are removed in favor of
    direct Drop for Guard* impls.
  • An Option is used to replace ManuallyDrop and unsafe code.
  • {Stream,Future}{Reader,Writer}::close{,_with} now take &mut self
    instead of self to be more composable with &mut self arguments
    during Drop for other structures (e.g. build-your-own
    drop-with-store).
  • The type parameters on Guard* are simplified to just T, the future
    or stream payload, and A: AsAccessor. This helps cut down on the
    complexity of signatures.
  • Guard* types now have into_{stream,future} as an alternative to
    .into() which doesn't require type annotations.

@alexcrichton alexcrichton requested a review from a team as a code owner July 30, 2025 19:58
@alexcrichton alexcrichton requested review from dicej and pchickey and removed request for a team and pchickey July 30, 2025 19:58
@alexcrichton
Copy link
Member Author

alexcrichton commented Jul 30, 2025

I'll note the first commit here is from #11351, so only the second commit is for this PR EDIT: prereqs landed

@alexcrichton alexcrichton requested review from a team as code owners July 30, 2025 21:19
@alexcrichton alexcrichton requested review from fitzgen and removed request for a team July 30, 2025 21:19
Copy link
Member

@fitzgen fitzgen left a comment

Choose a reason for hiding this comment

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

LGTM, although I am not super familiar with this code

This is a follow-up to bytecodealliance#11325 with a number of cosmetic changes about
the shape of the API and structure of the internals:

* `{Stream,Future}{Reader,Writer}::guard` is now an alternative
  constructor to `Guard*::new` (import fewer types).
* Internally `WithAccessor` and `DropWithStore` are removed in favor of
  direct `Drop for Guard*` impls.
* An `Option` is used to replace `ManuallyDrop` and `unsafe` code.
* `{Stream,Future}{Reader,Writer}::close{,_with}` now take `&mut self`
  instead of `self` to be more composable with `&mut self` arguments
  during `Drop` for other structures (e.g. build-your-own
  drop-with-store).
* The type parameters on `Guard*` are simplified to just `T`, the future
  or stream payload, and `A: AsAccessor`. This helps cut down on the
  complexity of signatures.
* `Guard*` types now have `into_{stream,future}` as an alternative to
  `.into()` which doesn't require type annotations.
@github-actions github-actions bot added wasi Issues pertaining to WASI wasmtime:api Related to the API of the `wasmtime` crate itself labels Jul 30, 2025
@alexcrichton alexcrichton added this pull request to the merge queue Jul 31, 2025
Merged via the queue into bytecodealliance:main with commit 59d7034 Jul 31, 2025
43 checks passed
@alexcrichton alexcrichton deleted the refactor-guards branch July 31, 2025 14:45
bongjunj pushed a commit to prosyslab/wasmtime that referenced this pull request Oct 20, 2025
This is a follow-up to bytecodealliance#11325 with a number of cosmetic changes about
the shape of the API and structure of the internals:

* `{Stream,Future}{Reader,Writer}::guard` is now an alternative
  constructor to `Guard*::new` (import fewer types).
* Internally `WithAccessor` and `DropWithStore` are removed in favor of
  direct `Drop for Guard*` impls.
* An `Option` is used to replace `ManuallyDrop` and `unsafe` code.
* `{Stream,Future}{Reader,Writer}::close{,_with}` now take `&mut self`
  instead of `self` to be more composable with `&mut self` arguments
  during `Drop` for other structures (e.g. build-your-own
  drop-with-store).
* The type parameters on `Guard*` are simplified to just `T`, the future
  or stream payload, and `A: AsAccessor`. This helps cut down on the
  complexity of signatures.
* `Guard*` types now have `into_{stream,future}` as an alternative to
  `.into()` which doesn't require type annotations.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

wasi Issues pertaining to WASI wasmtime:api Related to the API of the `wasmtime` crate itself

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants