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

Proposed refactoring to implement core::io #2262

Open
fluffysquirrels opened this issue Dec 25, 2017 · 12 comments
Open

Proposed refactoring to implement core::io #2262

fluffysquirrels opened this issue Dec 25, 2017 · 12 comments
Labels
A-input-output Proposals relating to std{in, out, err}. A-no_std Proposals relating to #[no_std]. A-traits-libstd Standard library trait related proposals & ideas T-libs-api Relevant to the library API team, which will review and decide on the RFC.

Comments

@fluffysquirrels
Copy link

fluffysquirrels commented Dec 25, 2017

Label: T-libs

There have of course been several efforts to refactor std::io to make it usable without std as core::io or similar.

Some links to prior art from a few hours of searching:

From what I can see all efforts are currently stalled, but I'd like to make progress on using std::io in a no_std environment. My use case is a crate I'm writing that uses std::io::{Read, Write}, and I want it to build with std and when no_std.

As mentioned in https://internals.rust-lang.org/t/refactoring-std-for-ultimate-portability/4301, there are circular dependencies between std::io and the rest of std.

I have an idea that I've not seen mentioned: use conditional compilation in the std::io module to enable use without std. Steps required in more detail:

  1. Move rust/src/libstd/io/*.rs to a new directory rust/src/libio_template
  2. Add a new rust/src/libstd/io/mod.rs with just an include!("../../libio_template/mod.rs").
  3. Add new crate rust/src/libio_nostd, which will also include! libio_template.
  4. Add cfg feature "libio_for_std" to both libstd and libio_nostd, with it enabled and disabled by default respectively.
  5. Patch libio_template to compile in libstd and libio_nostd.
  6. All done. no_std consumers can use libio_nostd, libstd API should be identical, future std::io patches must be correct both in libstd and libio_nostd.

The ideal solution, as proposed in https://internals.rust-lang.org/t/refactoring-std-for-ultimate-portability/4301, is probably a careful refactoring to decouple std, but that seems like a lot of work, which has already stalled once. A templated approach allows quick progress and can be eradicated progressively as more parts of std::io are refactored out of std.

I'm willing to work on a prototype branch or more formal RFC if there is interest from the libs team.

@Centril Centril added the T-libs-api Relevant to the library API team, which will review and decide on the RFC. label Dec 25, 2017
@TimNN
Copy link
Contributor

TimNN commented Dec 28, 2017

One problem I see with the include! approach is that std::io::Read and core::io::Read would be different traits, making APIs using either incompatible with each other.

@fluffysquirrels
Copy link
Author

fluffysquirrels commented Dec 28, 2017 via email

@Ericson2314
Copy link
Contributor

Ericson2314 commented Mar 7, 2018

I like the blanket impl I propose here https://internals.rust-lang.org/t/refactoring-std-for-ultimate-portability/4301/3 to connect the two traits.

@fluffysquirrels
Copy link
Author

fluffysquirrels commented Apr 17, 2018

@Ericson2314 Yes! I Read that thread with great interest.

I agree the platform abstraction layer sounds like a great approach. I didn't feel up to the task of starting that work and was looking for a quicker fix that could be improved later.

Edit: misread your post.

Yes, it should be possible to rewrite std::io as a concrete layer on top of a generic core::io layer with some wrapper structs to convert between the two, and make it all low-cost. I would be interested in prototyping that approach instead of what I proposed here.

@SimonSapin
Copy link
Contributor

There’s also methods like Read::read_to_vec that mention Vec which is defined in the alloc crate.

@Ericson2314
Copy link
Contributor

@fluffysquirrels I think you closed by mistake?

@SimonSapin Indeed, I imagine we'd want an alloc::io, too.

@fluffysquirrels
Copy link
Author

fluffysquirrels commented Apr 17, 2018

@fluffysquirrels I think you closed by mistake?

I was thinking the linked page may be more appropriate, but now I see your comments were on a forum, not an issue tracker. My mistake.

Regarding Read::read_to_vec and similar, perhaps core::io could have a feature flag to toggle features that require alloc.

@fluffysquirrels
Copy link
Author

Relevant GitHub issues raised in the portability working group:

rust-lang-nursery/portability-wg#4
rust-lang-nursery/portability-wg#1
rust-lang-nursery/portability-wg#12

@Ericson2314
Copy link
Contributor

@fluffysquirrels Sure, fine to continue this elsewhere. Thanks again for kicking off the discussion :).

@Ericson2314
Copy link
Contributor

@jonas-schievink jonas-schievink added A-input-output Proposals relating to std{in, out, err}. A-no_std Proposals relating to #[no_std]. A-traits-libstd Standard library trait related proposals & ideas labels Aug 18, 2019
bors bot added a commit to tock/tock that referenced this issue Jan 20, 2020
1548: Add kernel::debug::IoWrite trait and remove unsound str::from_utf8_unchecked (fix #1449) r=ppannuto a=gendx

### Pull Request Overview

This pull request adds a new `IoWrite` trait in the kernel, to write arbitrary bytes instead of UTF-8 strings (fixes #1449). Until now, the `flush` function was using `str::from_utf8_unchecked`, even though the bytes are actually not UTF-8 (even with only UTF-8 input, the underlying ring buffer may wrap-around in the middle on a UTF-8 character).

In practice, the underlying implementations work byte-by-byte to send debug output, so a more general trait to write byte slices can be plugged in.

This `IoWrite` trait is similar to `std::io::Write`, except that the latter doesn't exist in `no_std` (because `std::io::Error` isn't compatible - see rust-lang/rust#68315 and rust-lang/rfcs#2262).


### Testing Strategy

This pull request was tested by Travis.


### TODO or Help Wanted

This pull request still needs support for byte-by-byte writing on the arty-e21 board. I'm not even sure the current `debug!` macro is what we want to do in a panic handler.


### Documentation Updated

- [x] Updated the relevant files in `/docs`, or no updates are required.

### Formatting

- [x] Ran `make formatall`.

Co-authored-by: Guillaume Endignoux <guillaumee@google.com>
@lygstate
Copy link

What's the situation of this?

@mauro-balades

This comment was marked as spam.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-input-output Proposals relating to std{in, out, err}. A-no_std Proposals relating to #[no_std]. A-traits-libstd Standard library trait related proposals & ideas T-libs-api Relevant to the library API team, which will review and decide on the RFC.
Projects
None yet
Development

No branches or pull requests

8 participants