Skip to content
This repository has been archived by the owner on Sep 12, 2024. It is now read-only.

Move std::io::Error out of std. #11

Open
indolering opened this issue Sep 26, 2020 · 54 comments
Open

Move std::io::Error out of std. #11

indolering opened this issue Sep 26, 2020 · 54 comments

Comments

@indolering
Copy link

indolering commented Sep 26, 2020

Having std::io::Error permeates much of std::io, blocking many crates from being used in no_std environments or re-implementing functionality.

I believe Using std::io::{Read, Write, Cursor} in a nostd environment is the canonical tracking issue.

I'm a Rust newbie 🐣, so my apologies if this ticket is not helpful.

@yaahc
Copy link
Member

yaahc commented Sep 26, 2020

@indolering are you looking to also avoid having it bring in a dependency on alloc? std::io::Error has a lot of API dependencies on Box.

@indolering
Copy link
Author

I am super unqualified to answer that question, but the aforementioned ticket and many of the tickets linking to it address that question. I just know that this is on a lot of wishlists and didn't see it covered in the Zulip chat or other tickets.

@elichai
Copy link

elichai commented Sep 26, 2020

I'd love to see this but there are 2 problems I currently see:

  1. downcast returns a Box.
  2. It means a bunch of the From impls it contain will violate the orphan rule

solving the first one will probably require a backwards incompatible breakage, and solving the second one might not be possible

@sfackler
Copy link
Member

It also has a Box internally...

@elichai
Copy link

elichai commented Sep 26, 2020

It also has a Box internally...

only in the impl of these two though

@sfackler
Copy link
Member

The type io::Error contains a Box inside of it, regardless of what traits it implements: https://github.com/rust-lang/rust/blob/master/library/std/src/io/error.rs#L58-L73

@yaahc
Copy link
Member

yaahc commented Sep 26, 2020

The type io::Error contains a Box inside of it, regardless of what traits it implements: https://github.com/rust-lang/rust/blob/master/library/std/src/io/error.rs#L58-L73

Wondering if this could maybee be worked around the way panic does with BoxMeUp... Seems sketch tho. I don't think I want to focus on this short term over #3 which completely blocks this issue already.

@indo-dev-0
Copy link

Cross referencing:

@lygstate
Copy link

I am trying to move std::io into alloc:io, Error is the most fundenmental peace of code, any prototype design of core::Error are appeared?

@burdges
Copy link

burdges commented Feb 16, 2021

I think pub type Error = TinyBox<dyn Error>; should work without alloc using #20 albeit only for a trimmed Error trait without backtrace.

@ColinFinck
Copy link

I've raised a few questions if something like core::io::Error can ever be incorporated into Rust at all: rust-lang-nursery/portability-wg#12 (comment)
Please reply and let me know if this is the correct working group after all.

@yaahc
Copy link
Member

yaahc commented Mar 2, 2021

This is the right place. And to answer your question, I don't know if core::io::Error could ever be a thing. I could see some hacks like the ones we're trying to add to Backtrace to let it have an API that exists in core with the implementation provided by std working for core::io::Error as well, so I don't think it's for sure something we can't do. We already have some prior art of core types (PanicInfo) secretly using a Box as a raw pointer. But we've not seriously considered this yet because we view moving std::error::Error into core and integrating it with panic!s as a higher priority.

@indolering
Copy link
Author

This is the right place. And to answer your question, I don't know if core::io::Error could ever be a thing

That's unfortunate. For all of the revisions one might wish for in a hypothetical Rust 2.0 ... consistent error handling would be high up on the list.

@indolering indolering changed the title Move std::io::Error to core. Move std::io::Error out of std. Dec 19, 2021
@indolering
Copy link
Author

Updated name to include migration to alloc, which is significantly easier while still being a big win (I'm mainly interested in Rust for use in WASM).

Cross referencing Yaahallo's post post on Reddit:

The main thing I worry about at that point is if errorkind would make sense to move as well since I think it's pretty heavily tied to system IO APIs but it is also an abstraction over them so who knows. If anyone wants to hop into the error handling working group to push this, I would be happy to support it, but this isn't currently a priority for me.

@ajguerrer
Copy link

ajguerrer commented Apr 5, 2022

std::io::Error is a bit of a special error in that it standardizes interaction with hardware/OS. Especially for #[no_std] projects, I think that is very desirable to have.

I encountered this situation and plan to roughly copy/paste the library implementation, removing the bits about heap allocation. That lead me to an idea that has probably already been considered, but just wanted to check in...

Currently std::io::Error looks something like:

pub struct Error {
    repr: Repr,
}

enum Repr {
    Os(i32),
    Simple(ErrorKind),
    // &str is a fat pointer, but &&str is a thin pointer.
    SimpleMessage(ErrorKind, &'static &'static str),
    Custom(Box<Custom>),
}

struct Custom {
    kind: ErrorKind,
    error: Box<dyn error::Error + Send + Sync>,
}

Would it be feasible to make a core::io::Error with a reduced API surface and have std wrap the core impl? Such as:

core::io

pub struct Error {
    repr: Repr,
}

enum Repr {
    Os(i32),
    Simple(ErrorKind),
    // &str is a fat pointer, but &&str is a thin pointer.
    SimpleMessage(ErrorKind, &'static &'static str),
}

std::io

pub struct Error {
    repr: Repr,
}

enum Repr {
    Core(core::io::Error),
    Custom(Box<Custom>),
}

This may be naïve on my part, but I think UX would be fairly seamless if there is a conversion from core to std.

impl From<core::io::Error> for Error {
    #[inline]
    fn from(err: core::io::Error) -> Error {
        Error { repr: Repr::Core(err) }
    }
}

@yaahc
Copy link
Member

yaahc commented Apr 5, 2022

I don't think the wrapping you described would work because of the way that the Repr enum is bit packed into io::Error, but I think it might be possible to remove the Box from the Custom variant and use a pointer directly which is just the output of https://doc.rust-lang.org/stable/std/boxed/struct.Box.html#method.into_raw. Then the parts of io::Error that don't reference Box could move into core and the parts that do could move into alloc/std.

cc @thomcc

@burdges
Copy link

burdges commented Apr 5, 2022

In #20 I proposed pub type ErrorType = TinyBox<dyn ErrorTrait>; could provide a suitable core::io::Error option, without allocation @ajguerrer but it'd cost two usize instead of one.

As SimpleMessage is too large, we'd need std::io::Error to be type level, so that dyn Error created a sensible representation, like

pub struct OsError(i32);
impl core::ErrorTrait for OsError { .. }
...
pub struct SimpleMessageError<const K: ErrorKind>(&'static &'static str);
impl<const K: ErrorKind> core::ErrorTrait for SimpleMessageError<K> { .. }

All this runs into rust-lang/rust#46139 (comment) though

@ajguerrer
Copy link

Yeah, I see how Repr as I described could undesirably increase the size of the type. It doesn't really buy that much either now that I think about it. Nice idea! I can see how having one common struct for both std and core could be useful too.

@thomcc
Copy link
Member

thomcc commented Apr 5, 2022

ISTM like there's two issues here:

  1. Inability to use traits from from std::io due to std::io::Error.
  2. Desire to use std::io::Error as the representation for various system errors.

Personally, I feel like the first of these is more useful. The approach I've wanted for a long time is something like:

// Inside mod core::io
pub trait Read {
    pub type Error;
    fn read(&mut self, buf: &mut [u8]) -> Result<usize, Self::Error>;
    // ... And so on
}

And then std::io::Read would either be:

  1. some sort of trait alias for core::io::Read with the std::io::Error type. (type std::io::Read = impl core::io::Read<Error = std::io::Error> although this may not actually be how TAIT works)

  2. Or perhaps it could be an extension/blanket trait, or similar. This one is probably the way it in practice for various reasons (for example IoSlice/IoSliceMut have to be #[repr(transparent)] wrappers around a OS-specific type).

A downside is that would mean core::io functions can't do &mut dyn core::io::Read without specifying an error type. This has upsides though:

  • It is more flexible, allowing use in more cases and not being dependent on an allocator for to support custom user error types. This has some potential performance benefits, since user may want to use core::io::Read in a situation where the Error case is more performance sensitive (for example the infallible case below is a big one).

  • It allows stuff like core::io::SomeTrait<Error = !> for situations where the operation is infallible, or produces some specific error type. For example Vec<u8> (probably cannot change this at this point, but) arguably could be core::io::Write<Error = !> (or perhaps core::io::Write<Error = alloc::alloc::AllocError>).

This last bit brings up a point of open design work (there's certainly more), since some way of using a &mut core::io::Write<Error = Something> in cases where a &mut std::io::Write is required. I think this can be addressed with an adaptor function of some sort, but have not thought through the design for it (it seems possible, though).


For moving std::io::Error... Well, I'm against it. I don't really think that type belongs in libcore. It's also would be pretty janky on its own if it's not part of the IO traits. And honestly, libstd would be giving up the ability to make a number of improvements to it in the future, unless they're system-indepentent, or done carefully. That is, it seems like would be giving up a lot for little gain.

All that said, I'm not set in stone here, and could be convinced by a sufficiently well-reasoned RFC, but... it should have an overall design for both the traits and errors, etc.

EDIT: I just realized that this is the project-error-handling repo. I suppose discusion of the IO traits may be considered off-topic. IMO it's worth considering though, because many of the possible designs for the core::io traits reduce or remove the need for moving std::io::Error to libcore.

@Thomasdezeeuw
Copy link

Just an FYI perhaps the Box can be replaced by something like dyn* Display described here: https://smallcultfollowing.com/babysteps/blog/2022/03/29/dyn-can-we-make-dyn-sized.

@burdges
Copy link

burdges commented Apr 5, 2022

Your read trait is not object safe @thomcc which maybe acceptable using ?, or maybe causes issues with error propagation.

I think Nico's dyn* Trait looks like TinyBox<dyn Trait> in #20 @Thomasdezeeuw although his "views on traits" goes much further. If done without rustc support, it'd employ unsafe code that assumes stability & knowledge of rustc layout of &dyn Trait.

@thomcc
Copy link
Member

thomcc commented Apr 5, 2022

It's object safe for concrete error types. (e.g. &mut dyn core::io::Read does not work, but &mut dyn core::io::Read<Error = SomeError> does). This is what I meant by:

A downside is that would mean core::io functions can't do &mut dyn core::io::Read without specifying an error type.

This is indeed the big downside of the design, but it makes the trait usable in far more situations. Making it object-safe requires picking a concrete error type which IMO would significantly limit the usefulness of general-purpose system-independent read/write traits.

I'm unsure what you mean about the error propagation. It should be fine.

@burdges
Copy link

burdges commented Apr 5, 2022

Yes, you could concertize the trait in some dependency via features. Any truly generic dependencies would then avoid dyn Read provide its own Error type, from which the downstream crate must provide a conversion, yes?

It's actually not so bad I think. Ideally folks would implement both this and TinyBox<dyn Trait>, and then explore replacing ark_std::io with both.

@thomcc
Copy link
Member

thomcc commented Apr 5, 2022

Also, I think the concrete error type is actually a feature -- I've had many cases where something implements Read/Write so that it can be used generically, but std::io::Error will always represent a library error type, rather than a system error type. For example, rusqlite::blob::Blob is this way.

FWIW, I also have had a number of cases where I've wanted to have io traits which could not accept a liballoc dependency, and there's nothing fundamental about the traits that should require allocation, aside from holding custom user error types.

I pretty strongly feel that attempting to address io::Traits in libcore by just copying them (including io::Error) wholesale is... honestly, a bad the way to go about this. While I suppose there are reasons to want a subset of io::Error's functionality available to no_std code, most of the time I've heard folks suggest moving io::Error it's been for the traits, and I think that really needs a more considered design.


This last bit brings up a point of open design work (there's certainly more), since some way of using a &mut core::io::Write<Error = Something> in cases where a &mut std::io::Write is required. I think this can be addressed with an adaptor function of some sort, but have not thought through the design for it (it seems possible, though).

I mocked up what I mean by this: https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=f1ae589b585384c260bd44fe9be2a0a9. You could imagine doing this with Box<dyn std::error::Error>, or dyn* std::error::Error, but probably should be thought about more concretely. Also, note that there's a lot of parts1 of this that still need real thought put into how it should be done. Also it does several things that I don't really know if they're the right choice.

Footnotes

  1. For example, using avoiding io::Error nesting, using a single adapter type, etc... Mostly, the reason it does these is just to show that it's possible to avoid some of the downsides that this design may appear to have at first glance, such as extra nesting of std::io::Errors, needing many adapter types, ... It may not actually be important to solve some of these problems, though.

@fintelia
Copy link

fintelia commented Apr 5, 2022

Could you sketch a bit of how you'd write code that was generic over CoreIoRead<Error = T> for all T? I'm imagining that you'd run into a problem like below, where you'd have to handle a user-provided reader returning errors of any possible type, but also want to return errors of your own. Would this sort of thing be expected to use Box<std::error::Error>?

fn parse<E>(reader: &mut impl CoreIoRead<Error = E>) -> Result<ParsedData, ???> {
    let mut data = [0; 1];
    match reader.read(&mut data[..]) {
        Err(e) => return Err(e),
        Some(0) => return Err(MyError::UnexpectedEndOfInput);
        _ => { ... }
    }
}

@yaahc
Copy link
Member

yaahc commented Apr 5, 2022

Could you sketch a bit of how you'd write code that was generic over CoreIoRead<Error = T> for all T? I'm imagining that you'd run into a problem like below, where you'd have to handle a user-provided reader returning errors of any possible type, but also want to return errors of your own. Would this sort of thing be expected to use Box<std::error::Error>?

My intuition is that in cases like this you'd want to further bound the error type to allow conversions

fn parse<E>(reader: &mut impl CoreIoRead<Error = E>) -> Result<ParsedData, ???>
where
     E: From<MyError>,
{
    let mut data = [0; 1];
    match reader.read(&mut data[..]) {
        Err(e) => return Err(e),
        Some(0) => Err(MyError::UnexpectedEndOfInput)?;
        _ => { ... }
    }
}

@ajguerrer
Copy link

Looks like dyn* or any sort of pointer that doesn't assume a heap allocation could make for a convincing replacement to Box<dyn Error + Send + Sync> within std::io::Error.

@burdges , going off Niko's example of Waker I believe "pointer-sized" refers to the size of a fat pointer which is double width. That might be useful for your own considerations for TinyBox.

@thomcc I am very much interested in having a core representation for system errors. Embedded systems have system errors too! I'm a little curious why you think it would be "janky on its own if it's not part of the IO traits" while also proposing that it be completely abstracted out of the IO traits in the same post. Personally, I do like your idea of making Error an associated type in the IO traits, but at the same time, I worry that the ship has already sailed with the way its defined in std.

To give everyone my concrete use case, I want to make some #[no_std] networking primitives that share the same interfaces as the ones in std::net, similar to smoltcp. Right now, I am struggling to come up with an error type. Like smoltcp, I could create my own error. But, I'm actually leaning towards what embassy does. That's why I'm here. It would be great if there was a core::io::Error that does the things that std::io::Error does (without the allocator).

@ajguerrer
Copy link

The more I think about this, the more uncertain I feel about this issue. std::io::Error covers a ton of ground. I think there are some variants inside ErrorKind that could make sense in core like WriteZero or InvalidInput. But, there are also a ton of variants that sound like they would belong in a hypothetical std::net:Error or std::fs:Error. There is also the entire Os(i32) variant inside Repr that probably doesn't make sense in core either.

Assuming that we can diverge heavily from std::io::Error, does there even exist anything that would be appropriate for a core::io::Error? Would it be useful? Or. does the nature of an io::Error imply something too context specific for core? I guess I will need to stay tuned.

@burdges
Copy link

burdges commented Apr 7, 2022

Yes, dyn* Trait and TinyBox<dyn Trait> both wind up double sized, with the data occupying the data pointer position, and a real vtable pointer. It's almost like the vtable pointer is a discriminant in an open ended enum, which is both good and bad.

Read/Write provide a clean, simple, and rust binary serialization abstraction, in that type methods arguments bound by by Read/Write could serialize data without knowing details like const generics. I'd consider this "core" and unrelated to OS, but Read/Write types have many unrelated error styles, including OS errors.

@Nemo157
Copy link
Member

Nemo157 commented Sep 7, 2022

I have tried the trait Read { type Error; } approach twice in the past, using two different approaches for how fallible utilities work with the error type, and was disappointed in both.


The first approach I attempted in a fork of futures was to require a minimum set of error variants supported by any error type:

pub trait CoreIoError: Sized {
    /// An operation could not be completed because a call to `poll_write`
    /// returned `Ok(Async::Ready(0))`.
    fn write_zero(msg: &'static str) -> Self;

    /// An operation could not be completed because an "end of file" was
    /// reached prematurely.
    fn unexpected_eof(msg: &'static str) -> Self;
}

pub trait CoreAsyncRead {
    type Error: CoreIoError;
}

That makes the builtin utilities like read_exact/write_all Just Work™, but doesn't help for other utilities that don't fit into those categories.


The second approach I attempted in embrio was to simply wrap the error type in the builtin utilities:

#[derive(Debug)]
pub enum Error<T> {
    UnexpectedEof,
    Other(T),
}

pub fn read_exact<R: Read>(reader: R, buf: &mut [u8]) -> Result<(), Error<R::Error>> { ... }

That is consistent with how other utilities would have to be defined, but has composition issues. If you have a function calling both read_exact and write_all on types that have the same underlying error you now need to compose those together into an enum Error<T> { UnexpectedEof, WriteZero, Other(T) }.

@djdisodo
Copy link

djdisodo commented Dec 4, 2022

can we just make it depend on alloc?
and later fix the problem depending on alloc

will making it depend on alloc make any other problems than need of adding alloc to dependency?

later we can make it unable to wrap Box with io::Error if alloc isn't present (if that's the best we can do)

also, while there's many crates that implements similar module as std::io, it's unusable because it doesn't provides compatibility with other crates that use std::io

@burdges
Copy link

burdges commented Mar 29, 2023

Arkworks' wraps core/alloc/std inside its own ark_std crate, which provides io traits fairly cleanly: https://github.com/arkworks-rs/std/tree/master/src/io

This actually works, unlike associated Error types, ala https://github.com/QuiltOS/core-io/

It does however create a walled garden, in which your ecosystem's code compiles cleanly without std, but you need shims for other ecosystem's code. The solution is probably for people who want core::io to semi-standardize around some some std_no_std thing and just not import from std directly.

@RaitoBezarius
Copy link

Are there people interested into me sending a PR to move this outside of std ?

@adityashah1212
Copy link

adityashah1212 commented Jul 31, 2023

I don't know what other non-alloc scenarios we can expect, but embedded development is where it becomes a major issue for me. Even, when I do have an allocator, unless I am dying without it, I am not going to use it. In embedded world, what I have felt is that standard errors defined by POSIX are lacking due all kinds of possible failure reasons. I am not sure what issues @Nemo157 faced, but I am in favor of Error being an associated type for core::io::Read and core::io::Write.

Again I am not sure if there are any other non-alloc use cases, but these are my two cents

@rdrpenguin04
Copy link

A big non-alloc case has already been made for being able to use Cursor in no-std, no-alloc.

@RaitoBezarius, what was your plan for the PR? If I knew your solutions to the two outstaning problems, I could try to make a PR myself.

@RaitoBezarius
Copy link

A big non-alloc case has already been made for being able to use Cursor in no-std, no-alloc.

@RaitoBezarius, what was your plan for the PR? If I knew your solutions to the two outstaning problems, I could try to make a PR myself.

My plan was to move it in alloc first which is reasonable and good enough first, moving completely out of std and outside of alloc seems ambitious to me yet.

@rdrpenguin04
Copy link

What was your plan to remove reliance on sys?

@RaitoBezarius
Copy link

What was your plan to remove reliance on sys?

I imagine that it would be reasonable to have associated types or perform some form of Boxing. I would say the usecase I am interested in are usually pre-OS environments, e.g. bootloaders/UEFI/etc. Those have clearly reasonable system errors semantics.

Obviously, some folks may be interested into more customized mechanisms while having "zero cost" (e.g. as few as possible allocations), I don't think those folks can avoid for the time being rolling their own solutions, especially given that we are in some sort of paralysis analysis and I assume that catering to the group who can benefit from POSIX-style IO errors semantics can already move the needle to look into how to cater to everyone?

@rdrpenguin04
Copy link

"associated types" seems to imply something trait-related. So far, traits are not present in ErrorData at all; the way system errors are handled is with a single i32. I'm still wondering if there's a way to handle that with some form of generics rather than using traits or boxes; what do you think?

@thomcc
Copy link
Member

thomcc commented Aug 2, 2023

Note that we avoid. It's worth keeping in mind that some workloads have a lot of IO errors (consider a C compiler which tries to open a bunch of files along the include path), so pessimizing the error case is pretty undesirable.

@rdrpenguin04
Copy link

Exactly, so Box/dyn seems unnecessary. On the other hand, generics could work, or we could honestly just use i32 like we have been and just abstract the code that actually gets and prints said error. A generic type in a PhantomData<&Os> could possibly allow for us to do Os::print_error(id) or something like that.

@adityashah1212
Copy link

adityashah1212 commented Oct 6, 2023

I don't like the idea of using i32 as error type for Write, Read and Cursor traits, since we are back to C coding then. Plus Error trait gives some really nice things that aren't exactly available with i32/u32 types.

I still think this feels more flexible and elegant

/// core crate

pub mod io {
    pub trait Write {
        type Error: core::error::Error;
        ...
    }

    pub trait Read {
        type Error: core::error::Error;
        ...
    }

    pub trait Cursor {
        type Error: core::error::Error;
        ...
    }
}
/// alloc crate

pub mod io {
    struct Error { // Current Error implementation
    }

    pub type Write = core::io::Write<Error = Error>;
    pub type Read = core::io::Read<Error = Error>;
    pub type Cursor = core::io::Cursor<Error = Error>;
}

This allows us to use existing Read, Write and Cursor implementations as long as we have alloc or implement an Error ourselves that doesn't need Boxing if we don't have alloc

@fintelia
Copy link

fintelia commented Oct 6, 2023

@adityashah1212 in your design, what would the default implementation of Read::read_exact look like? For reference, this is what the current version in the standard library does. Notice that it both looks at what e.kind() was returned by the underlying reader, and also creates an io::Error on its own if it hits an EOF:

fn read_exact(&mut self, buf: &mut [u8]) -> Result<()> {
    while !buf.is_empty() {
        match this.read(buf) {
            Ok(0) => break,
            Ok(n) => {
                let tmp = buf;
                buf = &mut tmp[n..];
            }
            Err(ref e) if e.kind() == ErrorKind::Interrupted => {}
            Err(e) => return Err(e),
        }
    }
    if !buf.is_empty() {
        Err(error::const_io_error!(ErrorKind::UnexpectedEof, "failed to fill whole buffer"))
    } else {
        Ok(())
    }
}

@adityashah1212
Copy link

adityashah1212 commented Oct 12, 2023

How about something like this? Again, I am really not an expert on this, so just trying pitch ideas

/// core crate

pub mod io {
   #[non_exhaustive] // To ensure we can add more errors as the trait implementation changes... Not sure of this one though
    enum ErrorKind {
        // Separate or limited version of current ErrorKind, since a lot of those errors don't make much sense in case of no_std
    }

    pub trait Write {
        type Error: core::error::Error + From<ErrorKind>;
        ...
    }

    pub trait Read {
        type Error: core::error::Error + From<ErrorKind>;
        ...
    }

    pub trait Seek {
        type Error: core::error::Error + From<ErrorKind>;
        ...
    }
}
/// alloc crate

pub mod io {
    struct Error { // Current Error implementation
    }

    impl From<core::io::ErrorKind> for Error {
          ...
    }

    pub type Write = core::io::Write<Error = Error>;
    pub type Read = core::io::Read<Error = Error>;
    pub type Seek = core::io::Seek<Error = Error>;
}

Edit: I am an idiot for confusing std::io::Cursor for std::io::Seek. Fixed it

@fintelia
Copy link

You'd need to be able to pass an error message when creating the Error and have something like TryInto<ErrorKind> so that you could determine whether it was a ErrorKind::Interrupted. Other functionality might require even more of the current io::Error struct's interface. None of this is impossible, but it is a lot of messy and bikeshed-able API surface. Which makes me nervous that going down this path would stall out before reaching consensus.

(Note: I have no authority in the Rust project. This is just my own speculation.)

@programmerjake
Copy link
Member

I nerd-sniped myself into writing a mostly-working implementation of moving std::io::Error to core: rust-lang/rust#116685

I am unlikely to push it through merging as I don't actually have that much time, so whoever wants to can take my code and base their own PR off it.

@adityashah1212
Copy link

@fintelia, Looking at the current code, I think all we need is what you corrected on top of what I suggested. Using type Error: core::error::Error + TryInto<core::io::ErrorKind, Error = ()>. Would love to get some inputs from someone in the working group.

@adityashah1212
Copy link

Let me correct what I said earlier. TryInto won't work as it would consume the error and recreating it would be troublesome. What I am about to propose is probably not as great, but can work

/// core crate

pub mod io {
    enum ErrorKind {
        // Current Error Kind
    }

    pub trait AsErrorKind {
         fn kind(&self) -> ErrorKind;
    }

    pub trait Write {
        type Error: AsErrorKind;
        ...
    }

    pub trait Read {
        type Error: AsErrorKind;
        ...
    }

    pub trait Seek {
        type Error: AsErrorKind;
        ...
    }
}
/// alloc crate

pub mod io {
    struct Error { // Current Error implementation
    }

    impl core::io::AsErrorKind for Error {
          fn kind(&self) -> ErrorKind {
              Error::kind(&self)
          }
    }

    pub type Write = core::io::Write<Error = Error>;
    pub type Read = core::io::Read<Error = Error>;
    pub type Seek = core::io::Seek<Error = Error>;
}

@djdisodo
Copy link

i don't think there's reason to limit error type to std::io::Error type, which has ability to handle os errors and some other types of errors
and sometimes there's no possibility that such errors happen(for example &[u8] implements Read but most of errors the type has isn't even possible to happen here), you may want to avoid using these error types then
utilizing Associated Types can solve these error i think

trait Read {
    type Error;
    fn read(...) -> Result<usize, Self::Error>;
}

//slice for example

impl Read for &[u8] {
    type Error: EofError;
    ...
}

struct SomeRandomOsIoStream;

impl Read for SomeRandomOsIoStream {
    type Error = OsError;
    ...
}

//some wrapping reader example

enum ValidationError<T = Infallible> {
    Invalid,
    InnerError(T)
}

struct SomeValidationReader<T: Read, Error=ValidationError> where Error: From<ValidationError> + From<T::Error> (T);

impl Read for SomeValidationReader<T: Read, Error=ValidationErrorM<T::Error> where Error: From<ValidationError::<Infallible>> + From<T::Error> {
    type Error = Error;
    fn read(...) -> Result<usize, Self::Error> {
        if(invalid) {
            Err(ValidationError::Invalid)?;
        }
        Ok(self.0.read(...)?)
    }
}

by using default types we can avoid breaking changes as much as possible,

@aruiz
Copy link

aruiz commented Feb 16, 2024

It feels to me that the right answer here would be to do something similar to what alloc does, all we need is a way to specify an errno provider for our no_std environment and on std we set that to os::sys:errno()

Something along the lines of:

#[os_error_provider]
fn custom_os_error_func() -> i32 {
  //...
}

If you don't use it then trying to use io::Error should throw a compiler error just like with alloc

@gilescope
Copy link

If that's all that's stopping alloc::io::* becoming a thing I'm all in. Making more things wasm is hard without std::io equivalents.

@burdges
Copy link

burdges commented May 15, 2024

rust-lang/rfcs#3632

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests