0xMiden / 0xMiden/miden-vm

Remove blanket impl of Host for all impls of SyncHost

Open
#3,421 0 comments 1 reaction 1 assignee Claimed by @bitwalker View on GitHub
processor
Dominant language
Rust
Stars
772
Forks
352
Avg merge
1d 7h
Merged PRs (30d)
84

Description

The processor currently defines a blanket impl for `Host`:

```rust
impl Host for H {
# ...snip
}
```

This implementation makes it impossible for types which wrap a host to provide implementations of both the sync and async interfaces, so that the appropriate interface is used based on the capabilities of the host.

Concretely, the debugger has a host wrapper type `DapHostWrapper`, which wraps a host to provide certain functionality necessary for implementing debugging via Debug Adapter Protocol. The actual implementation regardless of whether the host is sync or async is sync, but because of the blanket impl, we are forced to use the async API internally when stepping execution, using a `poll_immediately` function, which is just pure overhead/unnecessary complexity.

The blanket impl makes it impossible to implement both `Host` and `SyncHost` for `DapHostWrapper` based on what trait `H` implements, without specialization (and thus using a nightly toolchain). Marker traits don't work here, because the relationship between the traits would require negative trait bounds to avoid the coherence conflict (i.e. `impl Host for DapHostWrapper { ... }`, meaning we only implement `Host` when `H` does _not_ implement `SyncHost`).

Since the `Host` impl for any `SyncHost` is trivial, I would suggest we just punt it to the concrete implementations to define how they implement `Host`. They can easily do the exact same thing the blanket impl does, and by not having the blanket impl we gain flexibility for use cases such as `DapHostWrapper`.

Contributor guide

Open the contributing guide

Research direction

Look for the blanket impl of Host for SyncHost in the processor module. Examine the DapHostWrapper type in the debugger to understand the conflict. The task is to remove the blanket impl and add equivalent Host implementations for each concrete type that currently relies on it. Check for any existing tests related to Host/SyncHost traits to ensure the refactor doesn't break functionality.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
backend
Issue type
Refactor
Difficulty
3/5
Estimated time
1-2 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.