Skip to content

Implement From<Arc<[u8]>> for Bytes - #857

Open
sb123sb123 wants to merge 3 commits into
tokio-rs:masterfrom
sb123sb123:fix/685-arc-to-bytes
Open

sb123sb123 wants to merge 3 commits into
tokio-rs:masterfrom
sb123sb123:fix/685-arc-to-bytes

Conversation

@sb123sb123

Copy link
Copy Markdown

Fixes #685

Problem

The Bytes documentation describes reference-counted backing storage such as Arc<[u8]>, but Bytes had no From<Arc<[u8]>> implementation. The documented conversion therefore failed to compile.

Cause

Bytes::from_owner already supports owning any AsRef<[u8]> + Send + 'static value, but the standard Arc<[u8]> conversion was missing from the From implementations.

Fix

Add a target_has_atomic = "ptr"-gated From<alloc::sync::Arc<[u8]>> for Bytes implementation that delegates to Bytes::from_owner. Add a regression test that drops the original Arc, verifies the bytes remain usable, and checks that the data pointer is preserved.

Tests

  • cargo fmt --all -- --check
  • cargo test --test test_bytes from_arc -- --nocapture
  • cargo test --all-features
  • cargo check --no-default-features
  • cargo clippy --all-features --lib (passes with existing warnings)
  • git diff --check
  • Independent pre-fix reproduction failed with the missing From<Arc<[u8]>> trait implementation; the same reproduction compiles after the fix.

Limitations

  • The checks ran on Windows. Linux/macOS, cross-target, no-atomic-target, nightly/Miri, and the full CI platform matrix were not run locally.
  • cargo clippy --all-features --all-targets -- -D warnings remains red on clean HEAD because of pre-existing diagnostics in unrelated code; the same baseline failure was reproduced before this change.
  • The conversion is gated on pointer-width atomics because alloc::sync::Arc is unavailable on targets without atomic pointers.

AI assistance

This pull request was prepared with AI assistance. AI was used for repository and issue triage, reproduction, implementation, and test planning; the behavior, patch, and test results were independently checked on the remote host.

Comment thread tests/test_bytes.rs
@sb123sb123

sb123sb123 commented Sep 29, 2026 •

Copy link
Copy Markdown
Author

Added the requested #[cfg(target_has_atomic = "ptr")] guard to the new Arc conversion test. cargo test --test test_bytes passes all 120 tests on both the G drive Windows clone and macOS; cargo fmt --all -- --check and git diff --check pass. The CI run for this head is action_required, and my account cannot approve fork PR runs. Could a repository maintainer approve it?

@sb123sb123

Copy link
Copy Markdown
Author

The minrust run exposed that cfg(target_has_atomic = "ptr") is not stable on this crate's declared Rust 1.57 MSRV, including the new test gate. I replaced it with a tiny build.rs that reads Cargo's CARGO_CFG_TARGET_HAS_ATOMIC and emits a bytes_has_atomic_ptr cfg; the library import, impl, and test now share that gate, so the API remains disabled on targets without pointer atomics without raising MSRV.

Validation on Windows with RUSTFLAGS=-Dwarnings: cargo check --no-default-features passes, cargo test --test test_bytes passes (120 tests), and cargo fmt --all -- --check plus git diff --check pass. The new upstream CI run is action_required and has not started (run); could you approve it and review the update when convenient?

This branch has not been deployed

No deployments
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.

Confusing documentation around Arc<[u8]> compatibility

2 participants