Move std::io::Error into core

[bushrat011899]

View all comments

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

Description

Moves std::io::Error into core, deferring Box-adjacent methods to incoherent implementations in alloc, and RawOsError methods to std. This requires some substantial changes to the internals of Error, but none of them are breaking changes or externally visible.

Notably, I've replaced usage of Box with a wrapper around a pointer and an appropriate drop function. This requires the addition of quite a few lines of unsafe, but is required to work around Box only being accessible from alloc. Additionally, an atomic pointer to a VTable is used for working with RawOsError in core, since we cannot know the required implementations without std.

As mention in this comment, there may be concern around having a static AtomicPtr in core for certain users. I've added a configuration option no_io_statics which (similar to no_sync/etc. in alloc) can be used to prevent their inclusion in core. When active, the fallback default implementation will always be used.

---

Notes

• This PR adopts the VTable technique from Move OS-dependent parts of std::io to alloc #152918
• This PR builds on my previous PR Move std::io::RawOsError to core::io #155574
• No AI tooling of any kind was used during the creation of this PR.

[programmerjake]

example of how to link from core's docs to std: https://doc.rust-lang.org/1.95.0/src/core/str/mod.rs.html#200

[bushrat011899]

example of how to link from core's docs to std: https://doc.rust-lang.org/1.95.0/src/core/str/mod.rs.html#200

That's a clever trick I never knew about! Looks like it's also required for linking to incoherent implementations even within the file that creates them too.

[programmerjake]

label -O-unix
See

maybe just leave it until you get the PR to work, that way there's less comment spam.

[programmerjake]

fn is_interrupted(_code: RawOsError) -> bool {
    cfg_select! {
        target_os = "solid_asp3" => {
            _code == -2004
        }
        any(
            target_os = "hermit",
            all(target_vendor = "fortanix", target_env = "sgx"),
            target_os = "teeos",
            target_family = "unix",
            target_os = "wasi",
        ) => {
            _code == 4

not all unixes have EINTR = 4:
https://github.com/search?q=repo%3Arust-lang%2Flibc+%2Fconst+EINTR%2F&type=code

[bushrat011899]

not all unixes have EINTR = 4: https://github.com/search?q=repo%3Arust-lang%2Flibc+%2Fconst+EINTR%2F&type=code

That's exactly the kind of thing I was worried about. I've updated that particular commit to use the correct value on WASI. All other configurations should be correct. I've left the cfg_select! statement unchanged to ensure the implicit selection behaviour doesn't change.

[rustbot]

Reminder, once the PR becomes ready for a review, use @rustbot ready.

[rustbot]

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

[bushrat011899]

@rustbot ready

I don't believe the OsFunctions static can be reduced in this PR without substantially increasing the risk for a change in behaviour. Even the relatively simple is_interrupted function requires considering many different targets for exceptions. Without access to libc in core, that's very difficult to keep in sync. If we want to go down that path, I think it should be in its own PR so it can be given the focus I think it needs.

[Amanieu]

This was discussed in the libs meeting last week. We're happy to use atomics as a workaround for now, but ideally we would want this to be resolved using the compiler's EII support.

@jdonszelmann Are EIIs with defaults in a sufficiently usable state right? We essentially need callbacks from core into std for io::Error, but we want to have an unreachable!() default when std is not linked in to avoid linker errors. This is completely internal to the standard library and not user-exposed.

[clarfonthey]

Also just resurfacing the comments on it since I can't find the appropriate review thread and it was kind of hidden in the meeting notes: as far as we're aware, the Rust for Linux folks would rather not have any static writable data in core+alloc and this would technically change that, even if it's in unstable stuff that might get LTO'd out. This isn't necessarily an API guarantee, but an implementation detail they're trying to avoid, and so the impression was that we'd use Externally Implementable Items on targets that support it, which would satisfy this for now in the cases RFL wants, and then longer-term we could maybe gate stabilisation on EII being available more broadly (at least unstably) so that we can try and retain this property longer term.

I feel like this is maybe the kind of thing that longer-term should be tracked using a dedicated lint, maybe, but at least for now this is the impression I have of the situation and why EII is desirable currently. Feel free to correct me depending on how accurate this is.

[programmerjake]

for the linux kernel, since that won't support std, we could follow my earlier suggestion of only having the EII/atomic on targets that actually have std and hardwire core to use the fallback on targets without std. this would need a cfg.