portability-wg icon indicating copy to clipboard operation
portability-wg copied to clipboard

Refactor `std` to improve ease of porting

Open jethrogb opened this issue 6 years ago • 20 comments

Currently, std contains platform-specific code throughout. This makes both in-tree and out-of-tree ports harder. In-tree ports are harder because it's not obvious what all needs to be implemented. Out-of-tree ports are harder because they require frequent laborous rebasing due to an unclear interface. It's not possible to port just a little bit to get your platform started, because succesful compilation requires “everything” to be there.

The idea is to add a platform abstraction layer (PAL) to std to separate out platform-specific code from generic code. The exact shape of the desired interface is unclear. The refactoring will likely need to be an incremental process where the PAL interface gets more cleary defined step-by-step.

As part of this work, it should be possible to create a "mock" platform that can be used to test the interface and as a starting point for implementing new platforms.

Prior work:

jethrogb avatar Mar 07 '18 16:03 jethrogb

Experimentation by @panicbit has shown that the current API can't really be captured in traits.

That is quite dated by now. I've tried a new approach which is a lot more straight forward and less idealistic. It will allow for a "mock" platform in the shape of the abstract_platform crate (which is more or less a copy of libstd but generic over the platform impl).

The current hurdles for a PAL in the form of traits are:

  1. lack of GATs, which would allow for a sane abstraction of generic types
  2. lack of statics and generic consts (can be worked around using functions and unlikely to be implemented in Rust)
  3. An ICE related to traits and DST layouts (e.g. with Path)

panicbit avatar Mar 07 '18 19:03 panicbit

As I mentioned a bit on IRC, I would propose these 3 parallel first steps:

  • [ ] (https://github.com/rust-lang/rfcs/issues/2262) core::io, which is generally useful, and especially for the PAL which cannot access std

    1. core::io::Read and core::io::Write
      • "blanket impls" to derive std::io::{Read, Write} from core::io::{Read, Write}
    2. #12: core::io:Error solution
      • @panicbit proposed (on IRC) enum with prefix of variants of std::io::Error, so conversion should optimize away
      • I proposed (in https://github.com/rust-lang-nursery/portability-wg/issues/12#issuecomment-371922047) no core::io::Error, but associated error type for core::io::{Read, Write} so as to better limit per-impl what errors are possible
    3. Associated "combinators types" like core::io::Chain and core::io::Cursor
  • [ ] pure alloc crate (edit pulled out into https://github.com/rust-lang-nursery/portability-wg/issues/20)

  • [ ] https://github.com/rust-lang/rust/pull/51846 alloc::Hashmap. That PR uses lang items. My original ideas for quick fix below:

    1. Just put up with the boilerplate of a manual newtype in std to get the ball rolling.
    2. Work with lang team on something to cut down on boilerplate. I propose permenantly-unstable auto-newtype feature (could maybe be viewed either as a special abstract type or struct wrapper).

(Besides this issue, I think we can also also get started on https://github.com/rust-lang-nursery/portability-wg/issues/5 immediately, and in total parallel to this.)

Ericson2314 avatar Mar 16 '18 17:03 Ericson2314

I actually think these are all orthogonal to this issue. The things you bring up make it easier to use things from std without using std. They don't make it easier to port the platform-dependent stuff of std.

jethrogb avatar Mar 16 '18 18:03 jethrogb

@jethrogb Well, the abstract_platform approach is all about being able to use things from std without using std, so the suggested changes would reduce the amount of boilerplate in the abstract_platform.

panicbit avatar Mar 16 '18 19:03 panicbit

@panicbit beat me to it :). HashMap probably isn't needed in the platform-dependent layers, but Read and Write traits are very useful.

Ericson2314 avatar Mar 16 '18 19:03 Ericson2314

@Ericson2314 Indeed, the only platform-dependent use I could find was in a test in sys_common/net.rs

panicbit avatar Mar 16 '18 19:03 panicbit

As to fallible allocation, I can sort of squint at to make a concern for us. On certain platforms, fallible allocation is the only one that makes sense, so its sort of a policy-portability concern. Then, insofar that std, or as much of std as possible, ought to be ported to to those platforms, the platform-dependent layers should be able to use it.

I moved it below core::io::{Read, Write} so the 3 bullets are in order from most relevant to our goals to least relevant.

Ericson2314 avatar Mar 16 '18 19:03 Ericson2314

I see, these changes are necessary to deal with the dependency inversion problem.

jethrogb avatar Mar 16 '18 19:03 jethrogb

IMHO trait aliases make something like core::io possible; instead of hard-coding the error type you make it an associated type, and then make std::io versions trait aliases which explicitly set the error type.

clarfonthey avatar Mar 16 '18 20:03 clarfonthey

@clarcharr That would be really cool, but Read::read_to_end by accumulating a Vec makes its not possible: that method must be in a std or alloc trait. I think there'd also be more coherence/back-compat issues maybe? Like it would be cool to also make

impl<T: core::io::Write<Err = !>> core::hash::Hasher for T { .. }

which wouldn't work if both were trait aliases (but maybe it's if just one is!).

[Side note, in my previous attempt at this, I also changed some signatures to better reflect how some of the methods treat "end of file" as an error, and some return Ok(0): https://github.com/QuiltOS/core-io/blob/master/src/lib.rs#L112. Not sure whether we allow ourselves to "change" the method signatures like that, but if so that would also require a new trait of course.]

Ericson2314 avatar Mar 16 '18 21:03 Ericson2314

@ericson2314 That might be possible with some version of rust-lang/rfcs#2315, or if std is one crate and the method is marked with #[cfg(alloc)] or something similar.

clarfonthey avatar Mar 16 '18 21:03 clarfonthey

So we reverse the dependencies? std -> pal-> OS|Bare-metal|(WASM minimal layer) (Don't depends on Libc) std -> libm(https://github.com/japaric/m) relibc -> OS|Bare-Metal|(WASM minimal layer) relibc -> (libm) relibc [a posix implementation], we could not avoid libc cause so much existing libs are based on that.

lygstate avatar Mar 24 '18 18:03 lygstate

@japaric

lygstate avatar Mar 24 '18 18:03 lygstate

@jethrogb So what's next? Do we formally decide on a plan and if so how?

Ericson2314 avatar Apr 03 '18 02:04 Ericson2314

Oh uh I liked your ideas in https://github.com/rust-lang-nursery/portability-wg/issues/1#issuecomment-373790342 and am not seeing any negative thoughts about it here, so I'd say we go for that. Unless you think we need a more formal process?

jethrogb avatar Apr 03 '18 02:04 jethrogb

@jethrogb glad you like it!

Repeating what I belated replied with on IRC:

jethrogb: Ericson2314: I see this WG more as keeping track of work that needs to be done and thinking about solutions and less about decision-making jethrogb: but I'm happy to take input on how to run this! :) jethrogb: my thoughts are that there's plenty of decision-making in the rust RFC/PR process

Ericson2314: jethrogb: sorry didn't see that Ericson2314: jethrogb: I'm just guessing, but I thought the point was to sort of to consolidate a plan that then can be ratified through the formal RFC processes Ericson2314: sort of like a bill in committee Ericson2314: so it is useful to winnow down different ideas into a concrete plan the relevant decision making team can trust has been vetted a bit

I'll add that while I suppose the main RFC process would still drill down on the technical merits of the proposed change, the vetting is especially good for making sure the motivation is meaningful and proposed change is palatable to the relevant stakeholders. That could allow the technical merits to be evaluated in isolation without spending lots of time ramping everybody up on the larger context.

But anyways, enough speculating, @aturon what you intend us to do?

Ericson2314 avatar Apr 03 '18 17:04 Ericson2314

I was expecting a lot of changes from the WG to not require RFCs since they don't change the publicly-visible API that rustc/cargo/core/std provide.

jethrogb avatar Apr 03 '18 18:04 jethrogb

@jethrogb Ah good point that's true too. But then might it be even more important we formally plan something, since there's no other decision making in those cases?

In the past a lot of big refactors stalled because it wasn't clear who should merge/review and they bit-rot quick. Getting a head count on people willing to help maintain the PR + some sort of prior commitment on reviewing/merging could be important to keep our momentum going better than in the past.

Ericson2314 avatar Apr 03 '18 22:04 Ericson2314

@chpio We want something like that (see #17)

panicbit avatar Apr 14 '18 11:04 panicbit