Skip to content
This repository was archived by the owner on Sep 8, 2025. It is now read-only.

Commit 2f89beb

Browse files
authored
Merge pull request #221 from alexcrichton/remove-unsafe
Make `Accessor::with_data` safe
2 parents 961a76c + 7091baa commit 2f89beb

File tree

1 file changed

+23
-6
lines changed

1 file changed

+23
-6
lines changed

crates/wasmtime/src/runtime/component/concurrent.rs

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -418,13 +418,30 @@ where
418418

419419
/// Changes this accessor to access `D2` instead of the current type
420420
/// parameter `D`.
421-
//
422-
// TODO: this is technically no longer `unsafe` but still not great to
423-
// call. This creates a second `Accessor` which makes it easy to call `with`
424-
// twice which can easily lead to panics (safe panics though). Needs some
425-
// thought about what exactly `Accessor` is represented as.
421+
///
422+
/// This changes the underlying data access from `T` to `D2::Data<'_>`.
423+
///
424+
/// Note that this is not a public or recommended API because it's easy to
425+
/// cause panics with this by having two `Accessor` values live at the same
426+
/// time. The returned `Accessor` does not refer to this `Accessor` meaning
427+
/// that both can be used. You could, for example, call `Accessor::with`
428+
/// simultaneously on both. That would cause a panic though.
429+
///
430+
/// In short while there's nothing unsafe about this it's a footgun. It's
431+
/// here for bindings generation where the provided accessor is transformed
432+
/// into a new accessor and then this returned accessor is passed to
433+
/// implementations.
434+
///
435+
/// Note that one possible fix for this would be a lifetime parameter on
436+
/// `Accessor` itself so the returned value could borrow from the original
437+
/// value (or this could be `self`-by-value instead of `&mut self`) but in
438+
/// attempting that it was found to be a bit too onerous in terms of
439+
/// plumbing things around without a whole lot of benefit.
440+
///
441+
/// In short, this works, but must be treated with care. The current main
442+
/// user, bindings generation, treats this with care.
426443
#[doc(hidden)]
427-
pub unsafe fn with_data<D2: HasData>(
444+
pub fn with_data<D2: HasData>(
428445
&mut self,
429446
get_data: fn(&mut T) -> D2::Data<'_>,
430447
) -> Accessor<T, D2> {

0 commit comments

Comments
 (0)