Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ACP: Move std::io::Borrowed{Buf,Cursor} into core::io #290

Closed
jmillikin opened this issue Nov 2, 2023 · 6 comments
Closed

ACP: Move std::io::Borrowed{Buf,Cursor} into core::io #290

jmillikin opened this issue Nov 2, 2023 · 6 comments
Labels
ACP-accepted API Change Proposal is accepted (seconded with no objections) api-change-proposal A proposal to add or alter unstable APIs in the standard libraries T-libs-api

Comments

@jmillikin
Copy link

Proposal

Problem statement

I would like to do I/O in a no_std context, and it would be nice if I could use the standard library's existing safe abstractions for reading into an uninitialized buffer.

Motivating examples or use cases

First case: compiling to WASM with no_std to avoid the large size and incomplete implementation of std for wasm32. I want to use BorrowedBuf / BorrowedCursor as part of the interface between the WASM module and the host.

Second case: binaries for the Linux early boot environment (initrd), which has tight size constraints and therefore works best with no_std binaries that do I/O via raw syscall. I want to use BorrowedBuf / BorrowedCursor in the API that wraps SYS_read / SYS_readv.

Solution sketch

Just move the existing implementation from library/std/src/io/readbuf.rs into library/core/src/io/. Maybe give it a different tracking feature like core_io_borrowed_buf so it could be stabilized separately from feature(read_buf).

It's not a big diff:

$ git diff --stat
 library/{std/src/io/readbuf.rs => core/src/io/borrowed_buf.rs}    | 13 -------------
 library/{std/src/io/readbuf => core/src/io/borrowed_buf}/tests.rs |  0
 library/core/src/io/mod.rs                                        |  6 ++++++
 library/core/src/lib.rs                                           |  2 ++
 library/std/src/io/impls.rs                                       | 13 +++++++++++++
 library/std/src/io/mod.rs                                         |  3 +--
 library/std/src/lib.rs                                            |  1 +
 7 files changed, 23 insertions(+), 15 deletions(-)

Alternatives

Do nothing?

Links and related work

rust-lang/rust#78485: Tracking Issue for RFC 2930 (feature read_buf)

What happens now?

This issue contains an API change proposal (or ACP) and is part of the libs-api team feature lifecycle. Once this issue is filed, the libs-api team will review open proposals as capability becomes available. Current response times do not have a clear estimate, but may be up to several months.

Possible responses

The libs team may respond in various different ways. First, the team will consider the problem (this doesn't require any concrete solution or alternatives to have been proposed):

  • We think this problem seems worth solving, and the standard library might be the right place to solve it.
  • We think that this probably doesn't belong in the standard library.

Second, if there's a concrete solution:

  • We think this specific solution looks roughly right, approved, you or someone else should implement this. (Further review will still happen on the subsequent implementation PR.)
  • We're not sure this is the right solution, and the alternatives or other materials don't give us enough information to be sure about that. Here are some questions we have that aren't answered, or rough ideas about alternatives we'd want to see discussed.
@jmillikin jmillikin added api-change-proposal A proposal to add or alter unstable APIs in the standard libraries T-libs-api labels Nov 2, 2023
@m-ou-se
Copy link
Member

m-ou-se commented Nov 7, 2023

We discussed this in the libs-api meeting. We have no objections! Feel free to open a tracking issue and open a PR to rust-lang/rust to add it as an unstable feature.

@the8472
Copy link
Member

the8472 commented Nov 7, 2023

Note that there's another accepted ACP that proposes to add vectored IO support for unitialized buffers: #104
Depending on how that gets implemented that might entail also moving iovec to core. You may want to coordinate with the author.

@jmillikin
Copy link
Author

Thanks! Filed rust-lang/rust#117693 for tracking, and sent PR rust-lang/rust#117694 for review.

@jmillikin
Copy link
Author

Note that there's another accepted ACP that proposes to add vectored IO support for unitialized buffers: #104 Depending on how that gets implemented that might entail also moving iovec to core. You may want to coordinate with the author.

Vectored IO support and IoSlice / struct iovec are both OS-specific, so I think migrating them to core (even partially) would be infeasible absent some deep magic that could unify IoSlice and &[u8].

@the8472
Copy link
Member

the8472 commented Nov 8, 2023

I think it was in the context of moving io::Error to core the team decided that it's ok to have target-specific type declarations in core. So iovec would then also have to exist in core if that would be needed directly on BorrowedBuf. If separate types are needed anyway then it can stay in std.

@ChrisDenton
Copy link
Member

It was pointed out we already do have OS-specific type declarations in core::ffi at least.

GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue Nov 8, 2023
…m-ou-se

Move `BorrowedBuf` and `BorrowedCursor` from `std:io` to `core::io`

Tracking issue: rust-lang#117693

ACP: rust-lang/libs-team#290
TaKO8Ki added a commit to TaKO8Ki/rust that referenced this issue Nov 9, 2023
…m-ou-se

Move `BorrowedBuf` and `BorrowedCursor` from `std:io` to `core::io`

Tracking issue: rust-lang#117693

ACP: rust-lang/libs-team#290
TaKO8Ki added a commit to TaKO8Ki/rust that referenced this issue Nov 9, 2023
…m-ou-se

Move `BorrowedBuf` and `BorrowedCursor` from `std:io` to `core::io`

Tracking issue: rust-lang#117693

ACP: rust-lang/libs-team#290
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Nov 9, 2023
Rollup merge of rust-lang#117694 - jmillikin:core-io-borrowed-buf, r=m-ou-se

Move `BorrowedBuf` and `BorrowedCursor` from `std:io` to `core::io`

Tracking issue: rust-lang#117693

ACP: rust-lang/libs-team#290
@dtolnay dtolnay added the ACP-accepted API Change Proposal is accepted (seconded with no objections) label Nov 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ACP-accepted API Change Proposal is accepted (seconded with no objections) api-change-proposal A proposal to add or alter unstable APIs in the standard libraries T-libs-api
Projects
None yet
Development

No branches or pull requests

5 participants