Skip to content

Generalize IO Traits for Arc<T> where &T: IoTrait#155684

Merged
rust-bors[bot] merged 1 commit intorust-lang:mainfrom
bushrat011899:blanket_io_seek_for_ref
Apr 24, 2026
Merged

Generalize IO Traits for Arc<T> where &T: IoTrait#155684
rust-bors[bot] merged 1 commit intorust-lang:mainfrom
bushrat011899:blanket_io_seek_for_ref

Conversation

@bushrat011899
Copy link
Copy Markdown
Contributor

@bushrat011899 bushrat011899 commented Apr 23, 2026

ACP: rust-lang/libs-team#755
Tracking issue: #154046
Related: #94744

Description

After experimenting with #155625, I noticed Seek and SeekFrom can almost be moved to core::io. Unfortunately, the implementation of Seek for Arc<File> is a blocker for such a move, since Arc is not a fundamental type. This PR attempts to resolve this potential blocker by replacing the implementation with a more general alternative. An internal trait IoHandle has been added which types can implement to opt-in to Read/Write/Seek implementations for Arc<Self> as long as &Self implements said trait. Note that BufRead is excluded as the signature for fill_buf would require returning from a temporary.

Since this "blanket" implementation only applies to a single type which already implements the same traits, I believe this should have no user-facing impact.

If this PR was merged, #134190 could be replaced with a 2 line PR:

impl IoHandle for TcpStream {}
impl IoHandle for UnixStream {}

Likewise for any other types, a table of which can be found here. This is out of scope for this PR to avoid the need for an ACP.


Notes

  • See this comment for further details.
  • No AI tooling of any kind was used during the creation of this PR.

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Apr 23, 2026
@fintelia
Copy link
Copy Markdown
Contributor

This was proposed before in #94744

@programmerjake
Copy link
Copy Markdown
Member

if you have some type that depends on using the shared reference itself (and not just data it points to) to keep track of position, this impl would be incorrect since it doesn't keep the shared reference around, but instead creates a new one from the Arc every time.

it could be made correct by adding a new trait bound SharedSeek that guarantees that the shared reference doesn't need to be updated:

#[unstable(...)]
pub trait SharedSeek
where
    for<'a> &'a Self: Seek,
{
}

impl<T: ?Sized + SharedSeek> Seek for Arc<T>
where
    for<'a> &'a Self: Seek,
{
    ...
}

impl SharedSeek for File {}
// ...

@bushrat011899 bushrat011899 force-pushed the blanket_io_seek_for_ref branch from 2cccf0f to 9008194 Compare April 23, 2026 22:14
@bushrat011899 bushrat011899 changed the title [WIP]: Generalise Seek for Arc<File> for all &T: Seek Generalize IO Traits for Arc<T> where &T: IoTrait Apr 23, 2026
@bushrat011899 bushrat011899 marked this pull request as ready for review April 23, 2026 22:17
@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 23, 2026
@rustbot rustbot removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Apr 23, 2026
@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented Apr 23, 2026

r? @jhpratt

rustbot has assigned @jhpratt.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

  • Owners of files modified in this PR: @ChrisDenton, libs
  • @ChrisDenton, libs expanded to 7 candidates
  • Random selection from Mark-Simulacrum, jhpratt

@bushrat011899
Copy link
Copy Markdown
Contributor Author

I've adjusted the approach here to instead add a marker trait to opt-in to the blanket implementation, which should resolve this concern from #94744. This is still useful as the marker trait can (eventually) be moved into core::io along with the other IO traits but still be implemented for std types like File.

Noting here: I believe these could be replaced with default fn to allow for overriding through specialization, but since there is only one type using these implementations, and their bodies are identical, I don't think it's necessary.

Comment thread library/std/src/io/impls.rs Outdated
Comment thread library/std/src/io/mod.rs Outdated
@bushrat011899 bushrat011899 force-pushed the blanket_io_seek_for_ref branch from 9008194 to 4e14cec Compare April 24, 2026 01:10
Added a marker trait `IoHandle` which can be used by the standard library to opt-in types to a blanket implementation of the various IO traits on `Arc<T>` where `&T: IoTrait` for some `IoTrait`.

The marker is required to avoid types like `Arc<[u8]>`  being included, since they don't have interior mutability and would not give expected results.
@bushrat011899 bushrat011899 force-pushed the blanket_io_seek_for_ref branch from 4e14cec to 7ba9478 Compare April 24, 2026 04:26
@jhpratt
Copy link
Copy Markdown
Member

jhpratt commented Apr 24, 2026

LGTM; this definitely resolves the concern brought up in the previous attempt. Agreed on the name not much mattering, as the documentation makes it clear. "Handle" is a typical term for this sort of thing anyways.

@bors r+

@rust-bors
Copy link
Copy Markdown
Contributor

rust-bors Bot commented Apr 24, 2026

📌 Commit 7ba9478 has been approved by jhpratt

It is now in the queue for this repository.

@rust-bors rust-bors Bot added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 24, 2026
rust-bors Bot pushed a commit that referenced this pull request Apr 24, 2026
Rollup of 9 pull requests

Successful merges:

 - #155684 (Generalize IO Traits for `Arc<T>` where `&T: IoTrait`)
 - #155081 (Move and clean up some ui test)
 - #155379 (Avoid query cycles in DataflowConstProp)
 - #155663 (Eliminate `CrateMetadataRef`.)
 - #155669 (Add `Sender` diagnostic item for `std::sync::mpsc::Sender`)
 - #155698 (Syntactically reject tuple index shorthands in struct patterns to fix a correctness regression)
 - #155703 (Remove myself as a maintainer of `wasm32-wasip1-threads`)
 - #155706 (Remove `AttributeLintKind` variants - part 7)
 - #155712 (Forbid `*-pass` and `*-fail` directives in tests/crashes)
@rust-bors rust-bors Bot merged commit e002c6c into rust-lang:main Apr 24, 2026
11 checks passed
@rustbot rustbot added this to the 1.97.0 milestone Apr 24, 2026
rust-timer added a commit that referenced this pull request Apr 24, 2026
Rollup merge of #155684 - bushrat011899:blanket_io_seek_for_ref, r=jhpratt

Generalize IO Traits for `Arc<T>` where `&T: IoTrait`

ACP: rust-lang/libs-team#755
Tracking issue: #154046
Related: #94744

## Description

After experimenting with #155625, I noticed `Seek` and `SeekFrom` can almost be moved to `core::io`. Unfortunately, the implementation of `Seek` for `Arc<File>` is a blocker for such a move, since `Arc` is not a fundamental type. This PR attempts to resolve this potential blocker by replacing the implementation with a more general alternative. An internal trait `IoHandle` has been added which types can implement to opt-in to `Read`/`Write`/`Seek` implementations for `Arc<Self>` as long as `&Self` implements said trait. Note that `BufRead` is excluded as the signature for `fill_buf` would require returning from a temporary.

Since this "blanket" implementation only applies to a single type which already implements the same traits, I believe this should have no user-facing impact.

If this PR was merged, #134190 could be replaced with a 2 line PR:
```rust
impl IoHandle for TcpStream {}
impl IoHandle for UnixStream {}
```
Likewise for any other types, a table of which can be found [here](rust-lang/libs-team#504 (comment)). This is out of scope for this PR to avoid the need for an ACP.

---

## Notes

* See [this comment](#154046 (comment)) for further details.
* No AI tooling of any kind was used during the creation of this PR.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs Relevant to the library team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants